New docbuilder always rebuilds everything

[jdemeyer]

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 attachment: trac_14156.v2.patch.

CC: @hivert @vbraun

Component: documentation

Author: John Palmieri

Reviewer: Volker Braun

Merged: sage-5.8.beta2

Issue created by migration from https://trac.sagemath.org/ticket/14156

[jhpalmieri]

comment:1

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.

[jhpalmieri]

Author: John Palmieri

[jhpalmieri]

comment:3

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.

[vbraun]

comment:4

Attachment: trac_14156.patch.gz

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...

[jhpalmieri]

comment:5

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?).

[hivert]

comment:6

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

[jhpalmieri]

comment:7

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:

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

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

[vbraun]

comment:9

Attachment: trac_14156.v2.patch.gz

Looks good to me!

[vbraun]

Reviewer: Volker Braun

[jdemeyer]

Merged: sage-5.8.beta2

[jdemeyer]

comment:11

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.