Let "make doc" always work.

[Thierry Monteil]

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

[Thierry Monteil]

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.

[Jeroen Demeyer]

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

[Thierry Monteil]

So, what should be done ?

[Jeroen Demeyer]

Figure out why the docbuilder sometimes fails and fix that.

[John Palmieri]

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?

[Jeroen Demeyer]

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.

[Jeroen Demeyer]

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

[Thierry Monteil]

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.

[Jeroen Demeyer]

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.

[Jeroen Demeyer]

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)

[Thierry Monteil]

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

[Jeroen Demeyer]

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

[Travis Scrimshaw]

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.

[Sébastien Labbé]

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

[Jeroen Demeyer]

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.

[Thierry Monteil]

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 :(

[John Palmieri]

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.

[Thierry Monteil]

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

[John Palmieri]

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.

[Jeroen Demeyer]

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

[John Palmieri]

No, good point.

[John Palmieri]

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"?

[Kwankyu Lee]

Check this out for alternative solution.

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

[John Palmieri]

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.

[John Palmieri]

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

[Jeroen Demeyer]

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

[John Palmieri]

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

[Jeroen Demeyer]

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?

[John Palmieri]

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.

[Kwankyu Lee]

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

[John Palmieri]

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.

[John Palmieri]

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.

[Jeroen Demeyer]

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.

[John Palmieri]

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.

[John Palmieri]

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

[John Palmieri]

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

[Jeroen Demeyer]

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.

[Jeroen Demeyer]

See #23803

[John Palmieri]

Okay, let's close this.