sphinx-build is broken

[jdemeyer]

The script $SAGE_LOCAL/bin/sphinx-build (which is copied over from build/pkgs/sphinx/patches/sphinx-build) is broken since it refers to the wrong version.

The Sage documentation build no longer uses the sphinx-build script, so this wasn't noticed immediately. Manually calling this script gives

Traceback (most recent call last):
  File "/usr/local/src/sage-git/local/bin/sphinx-build", line 5, in <module>
    from pkg_resources import load_entry_point
  File "build/bdist.linux-x86_64/egg/pkg_resources.py", line 2817, in <module>
  File "build/bdist.linux-x86_64/egg/pkg_resources.py", line 451, in _build_master
  File "build/bdist.linux-x86_64/egg/pkg_resources.py", line 464, in _build_from_requirements
  File "build/bdist.linux-x86_64/egg/pkg_resources.py", line 639, in resolve
pkg_resources.DistributionNotFound: Sphinx==1.1.2
make: *** [html] Error 1

[kcrisman]

Can you try with a fresh Sage build? I've never had problems using this, and I know others have used sws2rst successfully in the (recent) past, not just the other author on this ticket. Though it's true that the file has

#!/usr/bin/env python
# EASY-INSTALL-ENTRY-SCRIPT: 'Sphinx==1.1.2','console_scripts','sphinx-build'
__requires__ = 'Sphinx==1.1.2'
import sys
from pkg_resources import load_entry_point
import sage.all
sys.exit(
   load_entry_point('Sphinx==1.1.2', 'console_scripts', 'sphinx-build')()
)

I guess this could be fixed by

build/pkgs/sphinx/patches/sphinx-build

@@ -1 +1 @@
 #!/usr/bin/env python
-# EASY-INSTALL-ENTRY-SCRIPT: 'Sphinx==1.1.2','console_scripts','sphinx-build'
+# EASY-INSTALL-ENTRY-SCRIPT: 'Sphinx==1.2.2','console_scripts','sphinx-build'
 __requires__ = 'Sphinx==1.1.2'
 import sys
 from pkg_resources import load_entry_point
 import sage.all
 sys.exit(
-   load_entry_point('Sphinx==1.1.2', 'console_scripts', 'sphinx-build')()
+   load_entry_point('Sphinx==1.2.2', 'console_scripts', 'sphinx-build')()
 )

[kcrisman]

Is there any way to doctest this, or possibly at least fix SPKG.txt to say so?

Haha! It's ALREADY THERE!!!

 * Make sure to update the Sphinx version number in the file
   patches/sphinx-build.

Sigh.

[kcrisman]

Or maybe the script could be modified to grab the number in build/pkgs/sphinx/package-version.txt automatically, before being copied? That would be more future-proof.

[jdemeyer]

New commits:

​69337a5

Stop patching sphinx-build

[kcrisman]

?? But how will a brand-new Sage install now get sphinx-build? Is it part of setup.py install somehow?

[jdemeyer]

Replying to kcrisman:

But how will a brand-new Sage install now get sphinx-build? Is it part of setup.py install somehow?

Yes indeed!

[kcrisman]

Huh.

But the salient features are rather different.

import sys 
from pkg_resources import load_entry_point 
import sage.all 

Will being in sage -sh be enough for that? As opposed to

from sphinx import main
import sys

which is the probable effect of ​sphinx-build? There must have been a reason to have the custom version of the script.

[jdemeyer]

For the record, the patching of sphinx-build was introduced in #4737, which has very little info...

[jdemeyer]

Replying to kcrisman:

There must have been a reason to have the custom version of the script.

I have no idea really. But for #17265 I tested this ticket: sphinx-build seems to work without the import sage.all...

[kcrisman]

I was just about to ask you that question. Otherwise the script setup.py creates is indeed identical to the one here.

[kcrisman]

And I can confirm what you say about #17265. Huh.

[kcrisman]

Can you think of anything else sphinx-build would be used for now? I do know that while building the documentation now, if a given module doesn't exist in Sage (say, some stale files from switching branches) that it complains that it can't find the module and one has to delete the generated .rst by hand. (That is probably a bug that those aren't destroyed after having been auto-generated and then the original file is gone, by the way, but not for this ticket.) So perhaps at some point this functionality is necessary, even if this script is no longer used for building the Sage doc per se. Why does Sphinx have to know that the module exists to build the whole documentation?

If we can resolve that, otherwise this is fine and has a couple other good cleanups.

[jdemeyer]

Replying to kcrisman:

I do know that while building the documentation now, if a given module doesn't exist in Sage (say, some stale files from switching branches) that it complains that it can't find the module and one has to delete the generated .rst by hand.

What makes you think that this has to do with the sphinx-build script?

[kcrisman]

I do know that while building the documentation now, if a given module doesn't exist in Sage (say, some stale files from switching branches) that it complains that it can't find the module and one has to delete the generated .rst by hand.

What makes you think that this has to do with the sphinx-build script?

I don't think it has to do with that, or at least I believe you when you say it doesn't (above). But it does mean that, at least at certain times, Sphinx wants to try to import Sage modules when it builds the documentation, and blows up if it can't. So I'm wondering whether there might be times when that type of behavior, via sphinx-build, would affect whatever remaining uses of sphinx-build there are. Perhaps there is no such behavior, but I'm trying to brainstorm.

[kcrisman]

Okay, here is our src/doc/common/custom-sphinx-build.py

"""
This is Sage's version of the sphinx-build script

Enhancements are:

* import the Sage library to access the docstrings, otherwise doc
  buliding doesn't work.

* redirect stdout to our own logger, and remove some unwanted chatter.
"""

Although this is actually now in builder.py, and in any case the magic place this is needed is there too -

    def get_module_docstring_title(self, module_name):
        """
        Returns the title of the module from its docstring.
        """
        #Try to import the module
        try:
            __import__(module_name)
        except ImportError as err:
            logger.error("Warning: Could not import %s %s", module_name, err)
            return "UNABLE TO IMPORT MODULE"

So as long as we aren't trying to auto-generate stuff with ​autodoc in these we should be cool.

[jhpalmieri]

For what it's worth, this also seems to work with sage --docbuild file=... html. I can't think of any reasons why we would need to import sage.all in this file.

[kcrisman]

? I think the point is that the sphinx-build which is in the patches directory of sphinx is no longer needed because we don't need to import sage.all any more for that, since it's not the one used to build Sage doc. The version in src/doc/common refers to it, but actually the import is in builder.py, and there it is indeed necessary so that autodoc can do its stuff - or if not, maybe to raise an error earlier?

Anyway, if the files in src/doc/common can be modified, let's make that a different ticket.

[jhpalmieri]

I didn't mean to delete you as a reviewer. Sorry about that.

I meant that the changes in this ticket look good to me: there is no need to import sage.all in the sphinx-build script (that's what I meant by "this file"), so we should go back to using the standard version of that script.