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-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_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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help