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__():
>
> > #
> > # 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_jobs
>
> First 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
