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-09 15:59:45
Also in:
lkml, rust-for-linux
On Tue, Sep 09, 2025 at 08:53:50AM -0600, Jonathan Corbet wrote:
Finally beginning to look at this. I'm working from the pulled version, rather than the commentless patch (please don't do that again :).
Heh, when I had to rebase it, I noticed it was a bad idea to split ;-) I'll merge the commentless patch at the next respin.
A nit from SphinxBuilder::__init__():quoted
# # As we handle number of jobs and quiet in separate, we need to pick # both the same way as sphinx-build would pick, optionally accepts # whitespaces or not. So let's use argparse to handle argument expansion # parser = argparse.ArgumentParser() parser.add_argument('-j', '--jobs', type=int) parser.add_argument('-q', '--quiet', type=int) # # Other sphinx-build arguments go as-is, so place them # at self.sphinxopts, using shell parser # sphinxopts = shlex.split(os.environ.get("SPHINXOPTS", ")) # # Build a list of sphinx args # sphinx_args, self.sphinxopts = parser.parse_known_args(sphinxopts) if sphinx_args.quiet is True: self.verbose = False if sphinx_args.jobs: self.n_jobs = sphinx_args.jobs # # If the command line argument "-j" is used override SPHINXOPTS # self.n_jobs = n_jobsFirst of all, I do wish you would isolate this sort of concern into its own function.
Ok.
But, beyond that, you go to all that effort to parse the --jobs flag, but that last line just throws it all away. What was the real purpose here?
heh, it sounds to be something that got lost during a rebase.
This should be, instead:
if n_jobs:
self.n_jobs = n_jobs # this is parser.parse_args().n_jobs from main()
-
Basically, what happens is that the number of jobs can be on
different places:
1) if called via Makefile, no job arguments are passed at
command line, but SPHINXOPTS may contain "-j" on it.
The code shall use jobserver to get it by default, with:
# Clain all remaining jobs from make jobserver pool
with JobserverExec() as jobserver:
if jobserver.claim:
n_jobs = str(jobserver.claim)
else:
n_jobs = "auto"
# some logic to call sphinx-build with a parallel flag
# After with, claim is returned back to the
# jobserver, to allow other jobs to be executed
# in parallel, if any.
this basically claims all remaining make jobs from GNU jobserver.
So, if the build started with "-j8" and make was called with
other args, the number of available slots could be, for
instance "4".
The above logic will have jobserver.claim = 4, and run:
sphinx-build -j4 <other args>
This is the normal behavior when one does, for instance:
make -j8 drivers/media htmldocs
2) if called with SPHINXOPTS="-j8", it shall ignore jobserver
and call sphinx-build with -j8;
both cases (1) and (2) are handler inside a function
-
Now, when sphinx-build-wrapper is called from command line,
there's no GNU jobserver. So:
3) by default, it uses "-jauto". This can be problematic on
machines with a large number of CPUs but without too much
free memory (with Sphinx 7.x, one needs a really huge amount
of RAM to run sphinx with -j - like 128GB or more with -j24)
4) if "-j" parameter is specified, pass it as-is to sphinx-build;
tools/docs/sphinx-build-wrapper -j16 htmldocs
this calls sphinx-build with -j16.
5) one might still use:
SPHINXOPTS=-j8 tools/docs/sphinx-build-wrapper htmldocs
or, even weirder:
SPHINXOPTS=-j8 tools/docs/sphinx-build-wrapper -j16 htmldocs
The above logic you reviewed is handling (4) and (5). There:
- n_jobs comes from command line;
- this comes from SPHINXOPTS var:
sphinxopts = shlex.split(os.environ.get("SPHINXOPTS", ""))
if both SPHINXOPTS and -j are specified like:
SPHINXOPTS=-j8 tools/docs/sphinx-build-wrapper -j16 htmldocs
IMO it shall pick the latest one (-j16).
Yet, perhaps I should have written the code on a different way,
e.g., like:
if n_jobs:
# Command line argument takes precedence
self.n_jobs = n_jobs
elif sphinx_args.jobs:
# Otherwise, use what it was specified at SPHINXOPTS if
# any
self.n_jobs = sphinx_args.jobs
I'll change it at the next spin and re-test it for all 5 scenarios.
Regards,
Mauro