Opened 8 years ago
Closed 8 years ago
#17369 closed defect (fixed)
sphinx-build is broken
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.5 |
Component: | build | Keywords: | |
Cc: | Merged in: | ||
Authors: | Jeroen Demeyer | Reviewers: | Karl-Dieter Crisman |
Report Upstream: | N/A | Work issues: | |
Branch: | 69337a5 (Commits, GitHub, GitLab) | Commit: | 69337a587a9eb7b2fa17fe4f870b987d0434fcdf |
Dependencies: | Stopgaps: |
Description
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
Change History (20)
comment:1 Changed 8 years ago by
comment:2 Changed 8 years ago by
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.
comment:3 Changed 8 years ago by
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.
comment:4 Changed 8 years ago by
- Branch set to u/jdemeyer/ticket/17369
- Created changed from 11/19/14 15:57:37 to 11/19/14 15:57:37
- Modified changed from 11/19/14 16:09:10 to 11/19/14 16:09:10
comment:5 Changed 8 years ago by
- Commit set to 69337a587a9eb7b2fa17fe4f870b987d0434fcdf
- Status changed from new to needs_review
New commits:
69337a5 | Stop patching sphinx-build
|
comment:6 follow-up: ↓ 7 Changed 8 years ago by
?? But how will a brand-new Sage install now get sphinx-build? Is it part of setup.py install
somehow?
comment:7 in reply to: ↑ 6 Changed 8 years ago by
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!
comment:8 follow-up: ↓ 10 Changed 8 years ago by
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.
comment:9 Changed 8 years ago by
For the record, the patching of sphinx-build
was introduced in #4737, which has very little info...
comment:10 in reply to: ↑ 8 Changed 8 years ago by
comment:11 Changed 8 years ago by
I was just about to ask you that question. Otherwise the script setup.py
creates is indeed identical to the one here.
comment:12 Changed 8 years ago by
And I can confirm what you say about #17265. Huh.
comment:13 follow-up: ↓ 14 Changed 8 years ago by
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.
comment:14 in reply to: ↑ 13 ; follow-up: ↓ 15 Changed 8 years ago by
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?
comment:15 in reply to: ↑ 14 Changed 8 years ago by
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.
comment:16 Changed 8 years ago by
- Reviewers set to Karl-Dieter Crisman
- Status changed from needs_review to positive_review
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.
comment:17 Changed 8 years ago by
- Reviewers Karl-Dieter Crisman deleted
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.
comment:18 Changed 8 years ago by
? 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.
comment:19 Changed 8 years ago by
- Reviewers set to Karl-Dieter Crisman
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.
comment:20 Changed 8 years ago by
- Branch changed from u/jdemeyer/ticket/17369 to 69337a587a9eb7b2fa17fe4f870b987d0434fcdf
- Resolution set to fixed
- Status changed from positive_review to closed
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
I guess this could be fixed by
build/pkgs/sphinx/patches/sphinx-build
1.2','console_scripts','sphinx-build'1.2', 'console_scripts', 'sphinx-build')()