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

[Mauro Carvalho Chehab]

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