Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#14156 closed defect (fixed)

New docbuilder always rebuilds everything

Reported by: jdemeyer Owned by: mvngu
Priority: blocker Milestone: sage-5.8
Component: documentation Keywords:
Cc: hivert, vbraun Merged in: sage-5.8.beta2
Authors: John Palmieri Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jhpalmieri)

When running make doc, the whole documentation is always rebuilt, even if no source file has changed.

Before #6495, only changed files would be rebuilt:

$ make doc  # Initial build
[...]

$ time make doc  # Nothing changed
[...]
real    0m51.266s
user    0m36.840s
sys     0m10.040s

$ touch devel/sage/sage/symbolic/function_factory.py

$ time make doc  # Just one file changed
[...]
building [html]: targets for 1 source files that are out of date
updating environment: 0 added, 1 changed, 0 removed
reading sources... [100%] sage/symbolic/function_factory
[...]
writing output... [ 33%] calculus
writing output... [ 66%] index
writing output... [100%] sage/symbolic/function_factory
[...]
real    1m5.918s
user    0m51.900s
sys     0m10.620s

Apply trac_14156.v2.patch.

Attachments (2)

trac_14156.patch (6.0 KB) - added by jhpalmieri 7 years ago.
trac_14156.v2.patch (4.5 KB) - added by jhpalmieri 7 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 7 years ago by jhpalmieri

  • Cc hivert jhpalmieri sage-combinat novoselt added

Note that running sage --docbuild reference html doesn't rebuild everything, so I guess the problem is that running sage --docbuild reference inventory rebuilds the inventory files no matter what, and then the html build sees that something has changed, so it rebuilds everything.

comment:2 Changed 7 years ago by jhpalmieri

  • Cc jhpalmieri sage-combinat novoselt removed

comment:3 Changed 7 years ago by jhpalmieri

  • Authors set to John Palmieri
  • Cc vbraun added
  • Status changed from new to needs_review

I think there are two problems. First, the inventory builder did not have a working get_outdated_docs method: it was inheriting the method from StandaloneHTMLBuilder, and it was always saying that all docs were outdated: it was comparing the modification time of the appropriate rst file to some nonexistent file. I've put in a new method which is identical to the inherited one, except that it compares to the modification time of objects.inv.

The second problem is that any change to the configuration triggers a rebuild, so passing -D multidoc_first_pass=0 vs. -D multidoc_first_pass=1, or changing the setting of app.config.intersphinx_mapping, turn out to be bad ideas. I've deleted those, and instead filtered out the useless warning message ("search index couldn't be loaded...") during the inventory build.

This now works for me: I can type make doc twice, and it doesn't rebuild, and then I can type sage --docbuild all html or sage --docbuild reference inventory or variations on these and it doesn't rebuild. Touching a single file, running sage -b and then building the docs behaves properly.

Changed 7 years ago by jhpalmieri

comment:4 Changed 7 years ago by vbraun

Doesn't that open us up to races again? The if not (os.path.exists(refpath) and os.path.exists(invpath)): saves us the first time round, but subsequent inventory builds will again read and write simultaneously in the inventory directory. It might be difficult to trigger since you usually don't do that much in subsequent runs...

We could just use an environment variable to sneak information past the command line interface...

comment:5 follow-up: Changed 7 years ago by jhpalmieri

It's easy enough to bypass the multidoc_first_pass argument with an environment variable, but to get around app.config.intersphinx_mapping = {}, I think we will have to rewrite another chunk of Sphinx (intersphinx.py, maybe?).

comment:6 in reply to: ↑ 5 Changed 7 years ago by hivert

Hi there,

I'm sorry not being able to help more: this is my last message from my flat in Paris. I'm moving today and tomorrow to a suburb house and I won't have access to the network until Monday.

Replying to jhpalmieri:

It's easy enough to bypass the multidoc_first_pass argument with an environment variable, but to get around app.config.intersphinx_mapping = {}, I think we will have to rewrite another chunk of Sphinx (intersphinx.py, maybe?).

This is strange. When declaring a sphinx environment variable, there is a way to tell sphinx that changing this variable triggers the rebuild. See Sphinx doc of add_config_value. Maybe I screwed up, but this shouldn't triggers rebuild.

Florent

comment:7 Changed 7 years ago by jhpalmieri

Florent, sorry, I didn't realize that. So I guess the problem is not multidoc_first_pass, as you point out. But setting app.config.intersphinx_mapping definitely triggers a rebuild, so I made this change in conf.py:

  • doc/common/conf.py

    diff --git a/doc/common/conf.py b/doc/common/conf.py
    a b  
    630630    # set to a temporary directory.  We don't want to use intersphinx,
    631631    # etc., when doing introspection.
    632632    if app.srcdir.startswith(SAGE_DOC):
    633         app.add_config_value('intersphinx_mapping', {}, True)
     633        app.add_config_value('intersphinx_mapping', {}, False)
    634634        app.add_config_value('intersphinx_cache_limit', 5, False)
    635635        # We do *not* fully initialize intersphinx since we call it by hand
    636636        # in find_sage_dangling_links.

I'm still adding a get_outdated_docs method to the inventory builder. See the "v2" patch.

comment:8 Changed 7 years ago by jhpalmieri

  • Description modified (diff)

Changed 7 years ago by jhpalmieri

comment:9 Changed 7 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

Looks good to me!

comment:10 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.8.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:11 Changed 7 years ago by jdemeyer

Even with this patch, rebuilding the documentation when nothing needs to be done still takes a very long time.

In sage-5.7:

real    0m48.142s
user    0m37.120s
sys     0m10.110s

In sage-5.8.beta2:

real    5m52.020s
user    4m39.910s
sys     1m5.140s

Do you think this can be improved? See #14204 for a ticket.

Note: See TracTickets for help on using tickets.