Thread (67 messages) 67 messages, 5 authors, 2025-09-15

Re: [PATCH v4 08/19] tools/docs: sphinx-build-wrapper: add a wrapper for sphinx-build

From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Date: 2025-09-11 11:38:02
Also in: lkml, rust-for-linux

On Thu, Sep 11, 2025 at 01:23:55PM +0300, Jani Nikula wrote:
On Wed, 10 Sep 2025, Mauro Carvalho Chehab [off-list ref] wrote:
quoted
Em Wed, 10 Sep 2025 13:46:17 +0300
Jani Nikula [off-list ref] escreveu:
quoted
On Thu, 04 Sep 2025, Mauro Carvalho Chehab [off-list ref] wrote:
quoted
There are too much magic inside docs Makefile to properly run
sphinx-build. Create an ancillary script that contains all
kernel-related sphinx-build call logic currently at Makefile.

Such script is designed to work both as an standalone command
and as part of a Makefile. As such, it properly handles POSIX
jobserver used by GNU make.

On a side note, there was a line number increase due to the
conversion:

 Documentation/Makefile          |  131 +++----------
 tools/docs/sphinx-build-wrapper |  293 +++++++++++++++++++++++++++++++
 2 files changed, 323 insertions(+), 101 deletions(-)

This is because some things are more verbosed on Python and because
it requires reading env vars from Makefile. Besides it, this script
has some extra features that don't exist at the Makefile:

- It can be called directly from command line;
- It properly return PDF build errors.

When running the script alone, it will only take handle sphinx-build
targets. On other words, it won't runn make rustdoc after building
htmlfiles, nor it will run the extra check scripts.  
I've always strongly believed we should aim to make it possible to build
the documentation by running sphinx-build directly on the
command-line. Not that it would be the common way to run it, but to not
accumulate things in the Makefile that need to happen before or
after. To promote handling the documentation build in Sphinx. To be able
to debug issues and try new Sphinx versions without all the hacks.
That would be the better, but, unfortunately, this is not possible, for 
several reasons:

1. SPHINXDIRS. It needs a lot of magic to work, both before running
   sphinx-build and after (inside conf.py);
Makes you wonder if that's the right solution to the original
problem. It was added as a kind of hack, and it stuck.
The problem is, IMHO, due to the lack of flexibility of sphinx-build:
It should have a way on it to do partial documentation builds.

On our specific case, given that building docs takes so much time,
SPHINXDIRS is needed, as it allows to quickly check how a change is
output.

I use it a lot at devel time.

Also, media documentation builds depend on it:

    https://linuxtv.org/downloads/v4l-dvb-apis-new/

And probably other subsystems do the same, confining docs into
a subsystem-specific document.
quoted
2. Several extensions require kernel-specific environment variables to
   work. Calling sphinx-build directly breaks them;
The extensions shouldn't be using environment variables for
configuration anyway. Add config options and set them in conf.py like
everything else?
Agreed, but that's a separate problem, and should be addressed outside
this path changeset.

This one give one step on such direction by passing some parameters
via -D instead of env, but still conf.py and scripts are handling
them on a very particular way.
quoted
3. Sphinx itself doesn't build several targets alone. Instead, they create
   a Makefile, and an extra step is needed to finish the build. That's 
   the case for pdf and texinfo, for instance;
That's not true for the Makefile currently generated by
sphinx-quickstart. Granted, I haven't used Sphinx much for pdf output.
At the beginning, we were relying on the auto-generated pdf makefiles
only, but this had several issues (I can't recall them anymore). So,
we ended to a more complex proccess.

Yet, still broken, as, no matter if using sphinx quickstart way
or not, one needs to run (directly or indirectly) the produced
Makefile inside Documentation/output/.../latex to generate files,
and it will still always return non-zero, even if all PDFs are
built.

The real fix for it is outside our hands: someone needs to change
the way PDF is produced with a proper PDF builder instead of latex
and integrate at Sphinx tree.

There are some OOT docutils and/or sphinx pdf builders. We tried for
a while rst2pdf, but never worked for the Kernel source.
quoted
4. Man pages generation. Sphinx support to generate it is very poor;
In what way?
Have you ever tried to use it?

    "This builder produces manual pages in the groff format. 
     You have to specify which documents are to be included in 
     which manual pages via the man_pages configuration value."
    (https://www.sphinx-doc.org/en/master/usage/builders/index.html#sphinx.builders.manpage.ManualPageBuilder)

I tried when we were migrating to Sphinx. In summary:
    - Each man page requires an entry on a list;

        man_pages = [
            ('man/func', 'func', 'Func Documentation', [authors], 1),
            ...
        ]

    - Each man page requires a .rst file;
    - Each man page .rst file requires an specific format, like this:

        func(9)
        =======

        NAME
        ----
        func - ...

        SYNOPSIS
        --------
...

        DESCRIPTION
        -----------
...

      E.g. it is basically a troff source "converted" to .rst.
quoted
5. Rust integration adds more complexity to the table;

I'm not seeing sphinx-build supporting the above needs anytime soon,
and, even if we push our needs to Sphinx and it gets accepted there,
we'll still need to wait for quite a while until LTS distros merge
them.
I'm not suggesting to add anything to Sphinx upstream.
Without Sphinx upstream changes, I can't see how we'll get rid of
sphinx-build pre/post processing.
quoted
quoted
This patch moves a bunch of that logic into a Python wrapper, and I feel
like it complicates matters. You can no longer rely on 'make V=1' to get
the build commands, for instance.
Quite the opposite. if you try using "make V=1", it won't show the
command line used to call sphinx-build anymore.

This series restore it.

See, if you build with this series with V=1, you will see exactly
what commands are used on the build:

	$ make V=1 htmldocs
	...
	python3 ./tools/docs/sphinx-build-wrapper htmldocs \
	        --sphinxdirs="." --conf="conf.py" \
        	--builddir="Documentation/output" \
	        --theme= --css= --paper=
	python3 /new_devel/docs/sphinx_latest/bin/sphinx-build -j25 -b html -c /new_devel/docs/Documentation -d /new_devel/docs/Documentation/output/.doctrees -D kerneldoc_bin=scripts/kernel-doc.py -D version=6.17.0-rc1 -D release=6.17.0-rc1+ -D kerneldoc_srctree=. /new_devel/docs/Documentation /new_devel/docs/Documentation/output
	...


quoted
Newer Sphinx versions have the -M option for "make mode". The Makefiles
produced by sphinx-quickstart only have one build target:

# Catch-all target: route all unknown targets to Sphinx using the new
# "make mode" option.  $(O) is meant as a shortcut for $(SPHINXOPTS).
I didn't know about this, but from [1] it sounds it covers just two
targets: "latexpdf" and "info".
sphinx-build -M help gives a list of 24 targets.
Ok, but those are the extra ones. Btw, I'm almost sure we tried it for
latexpdf in the early days. Didn't work well. I guess the problem as
related to returned error codes that are always causing make pdfdocs
to return errors.
 
quoted
The most complex scenario is still not covered: SPHINXDIRS.

[1] https://www.sphinx-doc.org/en/master/man/sphinx-build.html
quoted
%: Makefile
        @$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)

That's all.
Try doing such change on your makefile. it will break:

	- SPHINXDIRS;
	- V=1;
	- rustdoc
I know it does. That's the problem.
The wrapper fixes it by handling the pre/post sphinx-build logic
at the right way.

See, this series doesn't change the need or the implementation logic
for pre/post sphinx-build steps(*). It just moves them to a wrapper,
while getting rid of all small hacky scripts that are needed to make
them work.

We can and should still pursue the goal of simplifying pre/post
steps.

(*) The only change is for PDF: after the series, the return code
    is now zero if all PDF files build, non-zero otherwise.

    This makes easier for humans and for CI to be sure that there 
    are nothing at the .rst files causing PDF builds to break.
quoted
and will still be dependent on variables that are passed via
env from Kernel makefile. So, stil you can't run from command
line. Also, if you call sphinx-build from command line:

	$ sphinx-build -j25 -b html Documentation Documentation/output
	...
	      File "<frozen os>", line 717, in __getitem__
	    KeyError: 'srctree'

It won't work, as several parameters that are required by conf.py and by
Sphinx extensions would be missing (the most important one is srctree, but
there are others in the line too).
quoted
The proposed wrapper duplicates loads of code that's supposed to be
handled by sphinx-build directly.
Once we get the wrapper, we can work to simplify it, but still I
can't see how to get rid of it.
I just don't understand the mentality of first adding something complex,
and then working to simplify it.
It doesn't make it more complex. Quite the opposite:

- it ports what we have as-is to a script;
- it drops hacky glues that were added over time on 4 different files
  to handle sphinxdirs, jobserver, parallelism, second build steps,
  man pages generation;
- it fixes the return code issue with pdf builds.

As all pre/post steps are now in a single place, it makes it easier to
maintain.
Don't make it a Rube Goldberg machine in the first place.
quoted
quoted
Including the target/builder names.
True, but this was a design decision taken lots of years ago: instead
of:
	make html

we're using:

	make htmldocs

This series doesn't change that: either makefile or the script need
to tho the namespace conversion.
In the above Makefile snippet that conversion would be $(@:docs=)
The same could be done at python. Yet, I opted there to use a dict
mainly because:

- we're not consistent about where files will be stored:
  with sphinx-quickstart, html files goes into _build/html. On our
  build system, we dropped "/html";
- we have some specific rules about where the final PDF files will
  be stored;
- we need to map what builder is used, because of the second step.

So, instead of having code checking for those specifics, I opted to
place on a dict, as it makes clearer and easier to maintain.
The clean Makefile way of checking for having Sphinx and the required
versions of Python and dependencies etc. would be a .PHONY target that
just checks, and doesn't do *anything* else. It shouldn't be part of the
sphinx-build rules.

PHONY += check-versions
check-versions:
	sphinx-pre-install --version-check

htmldocs: check-versions
	...

Or something like that.
The problem with that is that we shouldn't run sphinx-pre-install for
cleandocs or non-doc targets.

Anyway, this series doesn't touch sphinx-pre-install call. It is still
inside docs Makefile.

What we can do in the future is to convert sphinx-pre-install code into
a library and then call a check_versions() method from it at the wrapper
script, like:

    SphinxPreInstal().check_versions()     # or something equivalent

And then dropping it from the build system. Yet, this is out of the scope
of this series.
 
-- 
Thanks,
Mauro
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help