Let "make doc" always work.

[tmonteil]

As requested in #18464. Currently make doc can fail when some previous documentation was built.

[tmonteil]

The proposed fix is to just let the doc target depend on doc-clean. Perhaps is there a lighter solution ?

---

New commits:

​131ff97

#19882 : let doc target depend on doc-clean.

[jdemeyer]

No, this would force the doc to be rebuilt everytime you run make.

[tmonteil]

So, what should be done ?

[jdemeyer]

Figure out why the docbuilder sometimes fails and fix that.

[jhpalmieri]

Do we have ideas why? If a .py or .pyx file is present in one branch and not in another, this confuses the docbuilder. What else goes wrong?

[jdemeyer]

Replying to tmonteil:

So, what should be done ?

As quick fix, the following might work:

1. try to build the documentation normally.
2. if that fails, run make doc-clean and rebuild the documentation.

Step 1 is crucial to avoid a lot of needless rebuilds of the documentation.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​80a0a52

#19882 : run doc-clean only if doc-html lead to an error.

[jdemeyer]

I am wondering if the recursive call of $(MAKE) is a good idea (I'm not saying it's bad, I just need to think about it).

[tmonteil]

The current Makefile already calls $(MAKE), see e.g. in #18710:

bdist-clean: clean
        $(MAKE) misc-clean

That said, there is no real recursivity since the doc target depends on doc-html and doc-clean, and none of those depend on doc, so i do not see any loop possibility. I am currently doing a few tests to see how it works in various cases.

[jdemeyer]

It's problematic that doc-html has dependencies which also appear as dependencies of all-sage. So we can end up in a situation where there are two instances of make building the same targets.

[jdemeyer]

Maybe you can solve this with an extra level of indirection:

doc-html: $(DOC_DEPENDENCIES)
    $(MAKE) docbuild-html || ( $(MAKE) doc-clean ; $(MAKE) docbuild-html )

docbuild-html:
    cd ../.. && sage-logger './sage --docbuild --no-pdf-links all html $(SAGE_DOCBUILD_OPTS)' logs/dochtml.log

Ugly, but I guess it would work.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​5b555ae

#19882 add one level of indirection (comment 12)

[tmonteil]

It seems to work. Do someone know a way to reproduce that kind of docbuild error that disappears after a doc-clean ?

[jdemeyer]

Did you try make -q too? I am afraid that will break.

[tscrim]

Replying to tmonteil:

It seems to work. Do someone know a way to reproduce that kind of docbuild error that disappears after a doc-clean ?

I would just create a dummy file with some simple doc¹, build the doc, delete the file, and rebuild the doc (perhaps on a branch based on this one).

Alternatively, you could create a branch where you just remove a file and try to build the doc.

¹ - Don't forget to add an entry in the .rst files.

[slabbe]

#21378 got positive review (duplicate won't fix) saying that this ticket (#19882) is a better solution. But there was no update on #19882 since 8 months...

No later than yesterday, I run make ptestlong overnight to realize the day after that the doc failed and no test at all were ever made:

[dochtml] OSError: [homology ] Exception occurred:
[dochtml] 
[dochtml] 
[dochtml] Note: incremental documentation builds sometimes cause spurious
[dochtml] error messages. To be certain that these are real errors, run
[dochtml] "make doc-clean" first and try again.
Makefile:1061 : la recette pour la cible « doc-html » a échouée

I would like that make ptestlong always work...

[jdemeyer]

Merge conflict...

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​e80667d

#19882 : rebased on 7.6.beta6.

[tmonteil]

Here is a rebased version. Unfortunately, my laptop does not have enough memory anymore to build Sage doc, so that i can not test it seriously :(

[jhpalmieri]

I got the error

***********************************************
Makefile:1093: *** missing separator.  Stop.

with this, until I made the change

build/make/deps

@@ -206 +206 @@
 doc: doc-html
 
 doc-html: $(DOC_DEPENDENCIES)
-    $(MAKE) docbuild-html || ( $(MAKE) doc-clean ; $(MAKE) docbuild-html )
+       $(MAKE) docbuild-html || ( $(MAKE) doc-clean ; $(MAKE) docbuild-html )
 
 docbuild-html:
        $(AM_V_at)cd ../.. && sage-logger -p './sage --docbuild --no-pdf-links all html $(SAGE_DOCBUILD_OPTS)' logs/dochtml.log

(that is, replacing spaces with a tab).

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​83d8cc8

#19882 : replace spaces by tabs.

[tmonteil]

Oops, thanks for noticing, i did a 'hand rebase', without noticing that my editor behave as for python files. Sorry.

[jhpalmieri]

This works. I wish there were a less drastic solution, but let's merge it now and see if someone comes up with something better later.

[jdemeyer]

Did you check that make -q still works as expected?

[jhpalmieri]

No, good point.

[jhpalmieri]

Replying to jdemeyer:

Did you check that make -q still works as expected?

I guess I don't know what you mean, now that I've tested it. With the current develop branch, if I run make successfully, then I see

$ make -q
$ echo $?
1

So it doesn't work (IMHO) with the develop branch. It continues to not work with this branch. Is that working "as expected"?

[klee]

Check this out for alternative solution.

​https://trac.sagemath.org/ticket/22588

[jhpalmieri]

I think that the alternate solution at #22588 and this one both have advantages. For example, if you change your version of Sphinx, you may need to delete the docs and rebuild from scratch (maybe because of the inventory files? I'm not sure). I'm not sure that #22588 will catch that. So why not use both approaches?

That said, my question above about make -q still remains.

[jhpalmieri]

Jeroen: can you explain what you meant in comment:25 about make -q? With or without this branch, make -q leads to echo $? reporting "1", not "0". Do you object to a positive review for this?

(As I said, I also think that there is reason to keep both this and #22588.)

[jdemeyer]

So what should we do here, given #22588? Close as wontfix?

[jhpalmieri]

Will #22588 always work, or will the docs still fail to build some time (I guessed at one scenario in comment:29)?

[jdemeyer]

Replying to jhpalmieri:

Will #22588 always work, or will the docs still fail to build some time

That's something to be seen I guess. The question is: if issues arise with #22588, should we try to fix those issues or should we implement the "nuclear option" of this ticket?

[jhpalmieri]

I don't have strong feelings about it. Since it took so long to get an attempted fix, I'm not that optimistic that future problems will be dealt with, but I don't mind closing this and seeing what happens.

[klee]

Now with #22588, I hope that the doc builder is reliable. I mean: if doc build fails, then the reason is genuine, that is, there is a problem with the doc source, and doc build would fail even after doc-clean. Then the solution of this ticket is actually just to delay the time to recognize the problem of the doc source.

But time will tell whether I am just too optimistic..

[jhpalmieri]

I just did an incremental build from 8.1.beta3 to 8.1.beta4 and got

[dochtml] Building reference manual, first pass.
[dochtml] 
[dochtml] Error building the documentation.
[dochtml] Traceback (most recent call last):
[dochtml]   File "/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/runpy.py", line 174, in _run_module_as_main
[dochtml]     "__main__", fname, loader, pkg_name)
[dochtml]   File "/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/runpy.py", line 72, in _run_code
[dochtml]     exec code in run_globals
[dochtml]   File "/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__main__.py", line 2, in <module>
[dochtml]     main()
[dochtml]   File "/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 1675, in main
[dochtml]     builder()
[dochtml]   File "/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 310, in _wrapper
[dochtml]     getattr(get_builder(document), 'inventory')(*args, **kwds)
[dochtml]   File "/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 505, in _wrapper
[dochtml]     build_many(build_ref_doc, L)
[dochtml]   File "/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 246, in build_many
[dochtml]     ret = x.get(99999)
[dochtml]   File "/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/multiprocessing/pool.py", line 567, in get
[dochtml]     raise self._value
[dochtml] AttributeError: 'module' object has no attribute 'WarningStream'
[dochtml] 
[dochtml] Note: incremental documentation builds sometimes cause spurious
[dochtml] error messages. To be certain that these are real errors, run
[dochtml] "make doc-clean" first and try again.

[jhpalmieri]

And this seems to be repeatable for me: I switched back to 8.1.beta3, did make doc-clean; make, and then when I tried to build 8.1.beta4, same error.

[jdemeyer]

Replying to jhpalmieri:

[dochtml] AttributeError: 'module' object has no attribute 'WarningStream'

This is very likely due to the Sphinx upgrade #23023. It makes sense to require a complete rebuild of the docs when changing Sphinx versions. This should be automated, just like we automatically re-cythonize the Sage library whenever the Cython version changes.

[jhpalmieri]

We could delete the docs in the Sphinx spkg-install script. We used to do this, but then at some point (#11665) we decided to try to keep the old docs.

[jhpalmieri]

For example like this:

build/pkgs/sphinx/spkg-install

@@ -23 +23 @@
 success 'Error installing Sphinx'
 echo
 
+echo "A new version of Sphinx was installed, so deleting old Sage documentation output..."
+echo "Removing \"$SAGE_DOC\"..."
+rm -rf "$SAGE_DOC"
+success 'Error removing the old documentation'
+echo
+
 cd "$CUR"
 echo "Creating grammar pickle..."
 sage-python23 create_grammar_pickle.py

[jhpalmieri]

If this is a good idea, we can do it here or open another ticket.

[jdemeyer]

I like the principle, but not the approach of using the Sphinx installer to remove the docs. It goes against seperation of concerns. Better would be to implement this in the docbuilder: find out the timestamp or version of the installed Sphinx and then take action.

[jdemeyer]

See #23803

[jhpalmieri]

Okay, let's close this.