docs: some cleanup and improvements #33395

gmarull · 2021-03-16T16:29:57Z

Summary:

Current Makefile is just a thin wrapper around the CMake based documentation build. Removing this Makefile makes things more consistent as CMake is used everywhere in Zephyr as of today.
Sphinx supports parallelized build (-j) option. Setting it to auto makes use of all available cores. This option seems to speed up the build on multi-core machines, even though not much when Kconfig is enabled.

Quick comparison on -j auto usage (cpu count=12):

KCONFIG_TURBO_MODE=0

SPHINXOPTS=: ninja -C doc/_build htmldocs 650.51s user 8.50s system 100% cpu 10:55.39 total
SPHINXOPTS=-j auto: ninja -C doc/_build htmldocs 2094.90s user 62.44s system 368% cpu 9:46.23 total

KCONFIG_TURBO_MODE=1:

SPHINXOPTS=: ninja -C doc/_build htmldocs 303.29s user 4.48s system 101% cpu 5:04.46 total
SPHINXOPTS=-j auto: ninja -C doc/_build htmldocs 1060.54s user 21.20s system 555% cpu 3:14.64 total

checkpatch requires a Makefile to be present at the top-level directory. Remove this requirement. Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>

.github/workflows/doc-build.yml

.github/workflows/doc-publish.yml

carlescufi · 2021-03-17T16:30:31Z

doc/CMakeLists.txt

@@ -59,7 +59,7 @@ if(${LATEXMK} STREQUAL LATEXMK-NOTFOUND)
 endif()

 if(NOT DEFINED SPHINXOPTS)
-  set(SPHINXOPTS -q)
+  set(SPHINXOPTS -q -j auto)


carlescufi

LGTM, glad to see that Makefile go and -j auto by default.

carlescufi · 2021-03-18T09:19:59Z

@nashif can you take a look?

nashif

how is having to type:

cmake -GNinja -S doc -Bdoc/_build
ninja -C doc/_build htmldocs

instead of just

make

an improvement?

gmarull · 2021-03-18T10:32:41Z

how is having to type:
cmake -GNinja -S doc -Bdoc/_build
ninja -C doc/_build htmldocs
instead of just
make

in order to rebuild docs one can just do ninja htmldocs everytime when inside the build dir. Actually it's not just make, it's source zephyr-env.sh and make htmldocs...

an improvement?

the question here is: why do we want to maintain a top-level Makefile to build the documentation?

carlescufi · 2021-03-18T11:53:45Z

the question here is: why do we want to maintain a top-level Makefile to build the documentation?

I agree, having a Makefile at the root just to build the doc does not make sense in my opinion.

nashif · 2021-03-18T12:18:52Z

in order to rebuild docs one can just do ninja htmldocs everytime when inside the build dir. Actually it's not just make, it's source zephyr-env.sh and make htmldocs...

lets not pull source zephyr-env.sh into this please, for me this is a given and is always done as the first thing when I go into the repo, building docs right now is really just calling 'make', the default target gets me what I want.

I agree, having a Makefile at the root just to build the doc does not make sense in my opinion.

Then provide a way or a shortcut (script?) to build docs without having to deal with and remember cmake and ninja options like

cmake -GNinja -S doc -Bdoc/_build
ninja -C doc/_build htmldocs

gmarull · 2021-03-18T13:10:00Z

I think that a shortcut script isn't ideal, what if I want to run doxygen target? What if I just want to skip CMake configuration step?

Note that CMake command allows for some more simplicity, e.g. taking Ninja out of the game (doesn't add value here other than offering better experience for Windows users):

# from `doc` folder
cmake -B_build .
cd _build
make htmldocs
# edit files
make htmldocs
...

carlescufi · 2021-03-18T16:04:42Z

Then provide a way or a shortcut (script?) to build docs without having to deal with and remember cmake and ninja options like

We rejected this in-tree back when we switched from make to CMake, asking people to have their own scripts or aliases, if memory serves.
EDIT: @nashif for reference, here is what I am referring to: #5201 (review)

nashif · 2021-03-18T16:55:46Z

EDIT: @nashif for reference, here is what I am referring to: #5201 (review)

right, then leave the Makefile. I am really not sure what problem are we trying to solve here.

gmarull · 2021-03-18T17:03:33Z

EDIT: @nashif for reference, here is what I am referring to: #5201 (review)

right, then leave the Makefile. I am really not sure what problem are we trying to solve here.

IMO it's about tidiness. A top-level Makefile (a wrapper Makefile to be precise) just for docs is odd. Even more if Make is not actually used as a build system... Would it be possible at least to move it down to docs...?

carlescufi · 2021-03-18T17:27:34Z

EDIT: @nashif for reference, here is what I am referring to: #5201 (review)

right, then leave the Makefile. I am really not sure what problem are we trying to solve here.

IMO it's about tidiness. A top-level Makefile (a wrapper Makefile to be precise) just for docs is odd. Even more if Make is not actually used as a build system... Would it be possible at least to move it down to docs...?

I do agree with @gmarull here. That Makefile used to be the project's main entry point to the build system, but now it's just a wrapper around CMake for the docs? it's kind of a very strange reasoning for a Makefile to be there at all to be honest.

carlescufi · 2021-03-19T18:04:58Z

EDIT: @nashif for reference, here is what I am referring to: #5201 (review)

right, then leave the Makefile. I am really not sure what problem are we trying to solve here.

IMO it's about tidiness. A top-level Makefile (a wrapper Makefile to be precise) just for docs is odd. Even more if Make is not actually used as a build system... Would it be possible at least to move it down to docs...?

I do agree with @gmarull here. That Makefile used to be the project's main entry point to the build system, but now it's just a wrapper around CMake for the docs? it's kind of a very strange reasoning for a Makefile to be there at all to be honest.

In fact, I would argue that it's misleading to have that Makefile at all. Everything in Zephyr is CMake-based, but users looking at the codebase for the first time see a Makefile in the root folder, which is a red herring.

carlescufi · 2021-03-23T10:13:50Z

@nashif do you really want to keep this Makefile? I disagree, but we can discuss it.

nashif · 2021-03-25T12:15:21Z

@nashif do you really want to keep this Makefile? I disagree, but we can discuss it.

if the makefile being in the root is the problem, just move it under doc, Is this acceptable?

carlescufi · 2021-03-25T13:01:04Z

@nashif do you really want to keep this Makefile? I disagree, but we can discuss it.

if the makefile being in the root is the problem, just move it under doc, Is this acceptable?

If you insist... but I really think that keeping a Makefile that just wraps a CMake script makes no sense.

carlescufi · 2021-03-25T15:10:20Z

dev-review 2021.03.25:

Move the Makefile to doc/
Long term, create a west doc-build command

Sphinx supports parallelized build (-j) option. Setting it to `auto` makes use of all available cores. This option seems to speed up the build significantly on multi-core machines. Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>

Move the top-level Makefile to the documentation folder as it is only used for documentation. Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>

gmarull · 2021-03-25T19:54:33Z

@nashif should be ready now

mbolivar-nordic

@nashif I remember trying this a couple of years ago and dbkinder told me this lead to nondeterministic error output being caught by the .known-issues checks.

@gmarull have you tried this with different -j levels (not -j auto, but -j 2, -j 4, etc.) to make sure this doesn't happen?

mbolivar-nordic · 2021-03-29T14:59:54Z

lol, too late to the party

gmarull · 2021-03-29T15:02:31Z

@mbolivar-nordic I've only tried -j auto (which is -j 12 on my machine) and I haven't seen any errors so far. Do you mean that known issues filter is masking some real issues?

mbolivar-nordic · 2021-03-29T15:06:28Z

#14983 (comment) is the thread where dbkinder mentioned problems

mbolivar-nordic · 2021-03-29T15:10:28Z

@mbolivar-nordic I've only tried -j auto (which is -j 12 on my machine) and I haven't seen any errors so far. Do you mean that known issues filter is masking some real issues?

I'm reading tea leaves here, but basically I think that we are (or at least at some point, we were) relying on a particular serialization of how the .rst files are processed when masking errors in .known-issues.

For example -- and again I want to emphasize I am just making an educated guess here since it was @dbkinder, the former docs maintainer, who pointed out the error -- if we get a Duplicate C declaration warning from foo.rst using -j1, we might get the same warning from bar.rst using -j3 if bar.rst and foo.rst happen to get processed in different orders in -j3 vs -j1.

gmarull requested review from nashif, carlescufi and galak March 16, 2021 16:29

gmarull added the area: Documentation label Mar 16, 2021

zephyrbot added the area: Continuous Integration label Mar 16, 2021

zephyrbot requested review from jocelyn-li and utzig March 16, 2021 23:40

zephyrbot assigned carlescufi Mar 16, 2021

gmarull force-pushed the feature/docs-cleanup-1 branch 3 times, most recently from 6cd2b60 to 62cc890 Compare March 17, 2021 16:00

scripts: checkpatch: do not require top-level Makefile

719f320

checkpatch requires a Makefile to be present at the top-level directory. Remove this requirement. Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>

gmarull force-pushed the feature/docs-cleanup-1 branch from 62cc890 to 4c27a3f Compare March 17, 2021 16:01

carlescufi reviewed Mar 17, 2021

View reviewed changes

.github/workflows/doc-build.yml Outdated Show resolved Hide resolved

carlescufi reviewed Mar 17, 2021

View reviewed changes

.github/workflows/doc-publish.yml Outdated Show resolved Hide resolved

carlescufi reviewed Mar 17, 2021

View reviewed changes

gmarull force-pushed the feature/docs-cleanup-1 branch from 4c27a3f to bc7cd60 Compare March 17, 2021 16:30

gmarull requested a review from carlescufi March 17, 2021 16:31

carlescufi approved these changes Mar 17, 2021

View reviewed changes

gmarull added the dev-review To be discussed in dev-review meeting label Mar 17, 2021

jocelyn-li approved these changes Mar 18, 2021

View reviewed changes

nashif requested changes Mar 18, 2021

View reviewed changes

gmarull requested a review from nashif March 18, 2021 10:36

doc: enable automatic parallelization

c6cb752

Sphinx supports parallelized build (-j) option. Setting it to `auto` makes use of all available cores. This option seems to speed up the build significantly on multi-core machines. Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>

gmarull force-pushed the feature/docs-cleanup-1 branch 2 times, most recently from ee94d1a to 981d145 Compare March 25, 2021 19:51

doc: move Makefile to doc folder

3892248

Move the top-level Makefile to the documentation folder as it is only used for documentation. Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>

gmarull force-pushed the feature/docs-cleanup-1 branch from 981d145 to 3892248 Compare March 25, 2021 19:52

nashif approved these changes Mar 29, 2021

View reviewed changes

nashif merged commit 32b71cb into zephyrproject-rtos:master Mar 29, 2021

mbolivar-nordic suggested changes Mar 29, 2021

View reviewed changes

gmarull deleted the feature/docs-cleanup-1 branch March 29, 2021 15:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: some cleanup and improvements #33395

docs: some cleanup and improvements #33395

gmarull commented Mar 16, 2021

carlescufi Mar 17, 2021

carlescufi left a comment

carlescufi commented Mar 18, 2021

nashif left a comment •

edited

Loading

gmarull commented Mar 18, 2021 •

edited

Loading

carlescufi commented Mar 18, 2021

nashif commented Mar 18, 2021

gmarull commented Mar 18, 2021 •

edited

Loading

carlescufi commented Mar 18, 2021 •

edited

Loading

nashif commented Mar 18, 2021

gmarull commented Mar 18, 2021 •

edited

Loading

carlescufi commented Mar 18, 2021

carlescufi commented Mar 19, 2021

carlescufi commented Mar 23, 2021

nashif commented Mar 25, 2021

carlescufi commented Mar 25, 2021

carlescufi commented Mar 25, 2021

gmarull commented Mar 25, 2021

mbolivar-nordic left a comment

mbolivar-nordic commented Mar 29, 2021

gmarull commented Mar 29, 2021

mbolivar-nordic commented Mar 29, 2021

mbolivar-nordic commented Mar 29, 2021 •

edited

Loading

docs: some cleanup and improvements #33395

docs: some cleanup and improvements #33395

Conversation

gmarull commented Mar 16, 2021

carlescufi Mar 17, 2021

Choose a reason for hiding this comment

carlescufi left a comment

Choose a reason for hiding this comment

carlescufi commented Mar 18, 2021

nashif left a comment • edited Loading

Choose a reason for hiding this comment

gmarull commented Mar 18, 2021 • edited Loading

carlescufi commented Mar 18, 2021

nashif commented Mar 18, 2021

gmarull commented Mar 18, 2021 • edited Loading

carlescufi commented Mar 18, 2021 • edited Loading

nashif commented Mar 18, 2021

gmarull commented Mar 18, 2021 • edited Loading

carlescufi commented Mar 18, 2021

carlescufi commented Mar 19, 2021

carlescufi commented Mar 23, 2021

nashif commented Mar 25, 2021

carlescufi commented Mar 25, 2021

carlescufi commented Mar 25, 2021

gmarull commented Mar 25, 2021

mbolivar-nordic left a comment

Choose a reason for hiding this comment

mbolivar-nordic commented Mar 29, 2021

gmarull commented Mar 29, 2021

mbolivar-nordic commented Mar 29, 2021

mbolivar-nordic commented Mar 29, 2021 • edited Loading

nashif left a comment •

edited

Loading

gmarull commented Mar 18, 2021 •

edited

Loading

gmarull commented Mar 18, 2021 •

edited

Loading

carlescufi commented Mar 18, 2021 •

edited

Loading

gmarull commented Mar 18, 2021 •

edited

Loading

mbolivar-nordic commented Mar 29, 2021 •

edited

Loading