./configure --disable-doc
Closed this issue · 174 comments
This switch will disable the docbuild and avoid installing its many dependencies (sphinx...).
In contrast to other packages, pplpy's docbuild is enabled unless explicitly disabled by setting SAGE_SPKG_INSTALL_DOCS=no. We move it to a separate package pplpy_doc and make it a dependency of sagemath_doc_html.
Split out from #31396.
We also add argon2_cffi and nbclient to the packages disabled upon --disable-notebook.
Depends on #29039
CC: @tobiasdiez @dimpase @jhpalmieri @sagetrac-tmonteil
Component: build: configure
Author: Matthias Koeppe, John Palmieri
Branch/Commit: 43221d0
Reviewer: John Palmieri, Matthias Koeppe
Issue created by migration from https://trac.sagemath.org/ticket/32759
Commit: 18b4055
Description changed:
---
+++
@@ -1,4 +1,7 @@
This switch will disable the docbuild and installation of its many dependencies (sphinx...)
+
+In contrast to other packages, `ppl`'s docbuild is enabled unless explicitly disabled by setting `SAGE_SPKG_INSTALL_DOCS=no`.
+We conditionalize the dependency of `ppl` on this and also on `sphinx` being in the list of installed packages.
Split out from #31396.
Branch pushed to git repo; I updated commit sha1. New commits:
dfd23fb | build/pkgs/pplpy/dependencies: Fix up SAGE_SPKG_INSTALL_DOCS logic |
Author: Matthias Koeppe
Branch pushed to git repo; I updated commit sha1. New commits:
7307c75 | configure.ac (--disabled-notebook): Also disable nbclient |
Description changed:
---
+++
@@ -5,3 +5,5 @@
Split out from #31396.
+We also add another package to those that are disabled upon `--disable-notebook`.
+If --disable-doc is given, then should doc be removed from the all-start target, so that make doesn't try to build the docs?
Also, after I used ./configure --disable-doc, packages like alabaster and sphinxcontrib-... were built anyway.
Replying to @jhpalmieri:
If
--disable-docis given, then shoulddocbe removed from theall-starttarget, so thatmakedoesn't try to build the docs?
Yes, that's a good idea.
config.log says (for example)
configure:45112: result: sphinx-4.2.0: standard, but disabled using configure option
but maybe the problem is that I ran make instead of make build?
Yes, probably the all-start target pulled in the doc dependencies.
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
d13b8ae | configure.ac (--disable-doc): Disable also 'requests' and its dependencies, and direct sphinx dependencies |
4121cb0 | build/pkgs/pplpy/dependencies: Depend on sphinx only if SAGE_SPKG_INSTALL_DOCS!=no |
8ca1189 | build/pkgs/pplpy/dependencies: Fix up SAGE_SPKG_INSTALL_DOCS logic |
ccf6fb5 | configure.ac (--disabled-notebook): Also disable nbclient |
8e26780 | build/pkgs/sagemath_doc_{html,pdf}: New; delegate to them from targets doc-html etc. |
eeb8777 | m4/sage_spkg_collect.m4: Determine install tree for python3 via new trees.txt mechanism |
c8e19a0 | fixup |
5a4424b | build/make/Makefile.in (all-sage): Do not include packages installed in SAGE_DOCS |
67714b8 | Merge #31356 |
128de23 | configure.ac (--disable-doc): Disable the actual docbuild on 'make' |
Replying to @mkoeppe:
Replying to @jhpalmieri:
If
--disable-docis given, then shoulddocbe removed from theall-starttarget, so thatmakedoesn't try to build the docs?Yes, that's a good idea.
This is implemented now, via #31356
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
eb65e03 | build/pkgs/sagemath_doc_*/dependencies: Add sage_docbuild |
7989d87 | src/doc/Makefile: Handle errors from './sage --docbuild --all-documents' |
d64fb76 | build/make/Makefile.in: Undo reintroduction of SAGE_SKIP_PLOT_DIRECTIVE, SAGE_DOC_MATHJAX settings |
6afc0d5 | build/pkgs/sagemath_doc_html/spkg-install: Move handling of SAGE_DOC_JSMATH environment variable here from src/bin/sage-env |
b2cf40c | build/make/Makefile.in: Undo reintroduction of SAGE_SKIP_PLOT_DIRECTIVE, SAGE_DOC_MATHJAX settings (fixup) |
37a03bd | src/doc/en/developer/packaging.rst: Fix up section label |
fc89a66 | build/pkgs/sagemath_doc_pdf/spkg-install: Set SAGE_DOC_MATHJAX as a workaround |
fd298c4 | build/pkgs/sagemath_doc_pdf/dependencies: Depend on sagemath_doc_html |
b0593ea | Merge #31356 |
8a7027a | configure.ac (--disable-doc): Disable the actual docbuild on 'make' |
Should we expect doctests to pass with ./configure --disable-doc? I get
src/sage/misc/sagedoc.py # 8 doctests failed
src/sage_docbuild/utils.py # 5 doctests failed
src/sage_docbuild/__init__.py # 38 doctests failed
src/sage_docbuild/sphinxbuild.py # 13 doctests failed
src/sage/docs/conf.py # 1 doctest failed
which is not surprising, but should they be tagged so that they are only tested when sagemath_doc_html is built?
git grep 'dochtml' shows that we already seem to have an optional tag for this kind of thing.
We should probably replace it by # optional - sagemath_doc_html and add the missing ones for the failing doctests.
Right, and then in the top-level Makefile, this line needs to be changed:
TESTALL_FLAGS = --optional=sage,dochtml,optional,external,build
and maybe the last line in this block from src/sage/doctest/control.py:
if have_git:
self.files.append(opj(SAGE_SRC, 'sage_setup'))
self.files.append(opj(SAGE_SRC, 'sage_docbuild'))
Perhaps define have_docs appropriately and have a separate if have_docs block. Either that or tag everything in sage_docbuild.
Something like this?
diff --git a/src/sage/doctest/control.py b/src/sage/doctest/control.py
index 858d8cb567..8dbb693e22 100644
--- a/src/sage/doctest/control.py
+++ b/src/sage/doctest/control.py
@@ -727,13 +727,14 @@ class DocTestController(SageObject):
Doctesting ...
"""
opj = os.path.join
- from sage.env import SAGE_SRC, SAGE_DOC_SRC, SAGE_ROOT, SAGE_ROOT_GIT
+ from sage.env import SAGE_SRC, SAGE_DOC_SRC, SAGE_DOC, SAGE_ROOT, SAGE_ROOT_GIT
# SAGE_ROOT_GIT can be None on distributions which typically
# only have the SAGE_LOCAL install tree but not SAGE_ROOT
if SAGE_ROOT_GIT is not None:
have_git = os.path.isdir(SAGE_ROOT_GIT)
else:
have_git = False
+ have_docs = os.path.isdir(opj(SAGE_DOC, 'html', 'en', 'tutorial'))
def all_files():
self.files.append(opj(SAGE_SRC, 'sage'))
@@ -742,7 +743,8 @@ class DocTestController(SageObject):
# don't make sense to run outside a build environment
if have_git:
self.files.append(opj(SAGE_SRC, 'sage_setup'))
- self.files.append(opj(SAGE_SRC, 'sage_docbuild'))
+ if have_docs:
+ self.files.append(opj(SAGE_SRC, 'sage_docbuild'))
self.files.append(SAGE_DOC_SRC)
if self.options.all or (self.options.new and not have_git):I think it would make more sense to just test whether opj(SAGE_SRC, 'sage_docbuild') exists.
Same for sage_setup.
I think we can get rid of the have_git logic
By the way, with make distclean && ./configure --disable-doc && make build, the various sphinx packages are still being built.
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5d962fa | configure.ac (--disable-notebook): Also disable argon2_cffi |
da2f1cc | configure.ac (--disable-doc): New |
4f0fee0 | configure.ac (--disable-doc): Disable also 'requests' and its dependencies, and direct sphinx dependencies |
92d06dc | build/pkgs/pplpy/dependencies: Depend on sphinx only if SAGE_SPKG_INSTALL_DOCS!=no |
3b1f105 | build/pkgs/pplpy/dependencies: Fix up SAGE_SPKG_INSTALL_DOCS logic |
64273d1 | configure.ac (--disabled-notebook): Also disable nbclient |
b6167ca | configure.ac (--disable-doc): Disable the actual docbuild on 'make' |
Replying to @mkoeppe:
I think it would make more sense to just test whether
opj(SAGE_SRC, 'sage_docbuild')exists.Same for
sage_setup.I think we can get rid of the
have_gitlogic
Either that or test whether we can import sage_docbuild. I think the directory will exist (in a source distribution) regardless, but we don't want to test if we haven't at least built sage_docbuild. That seems cleaner than tagging everything in those files optional - sage_docbuild.
I think the doctesting logic is that the source tree (in SAGE_ROOT/src) is preferred over installed sources, which are only used for falling back (for example for use in downstream packaging). But I agree, when falling back to installed sources, then importing sage_docbuild and using its __path__ is the right solution
My point is that the typical doctest in sage_docbuild/* starts with from sage_docbuild import ..., so we shouldn't bother to test (as part of make testall etc.) if we can't import sage_docbuild. So I suggest something like:
diff --git a/src/sage/doctest/control.py b/src/sage/doctest/control.py
index 858d8cb567..2bf0a4beb4 100644
--- a/src/sage/doctest/control.py
+++ b/src/sage/doctest/control.py
@@ -740,9 +740,13 @@ class DocTestController(SageObject):
# Don't run these tests when not in the git repository; they are
# of interest for building sage, but not for runtime behavior and
# don't make sense to run outside a build environment
- if have_git:
+ if os.path.isdir(opj(SAGE_SRC, 'sage_setup')):
self.files.append(opj(SAGE_SRC, 'sage_setup'))
+ try:
+ import sage_docbuild
self.files.append(opj(SAGE_SRC, 'sage_docbuild'))
+ except ImportError:
+ pass
self.files.append(SAGE_DOC_SRC)
if self.options.all or (self.options.new and not have_git):Ah ok, I agree. Same for sage_setup then, I think
Branch pushed to git repo; I updated commit sha1. New commits:
3556181 | git grep -l '#.*optional.*dochtml' | xargs sed -i.bak 's/# optional - dochtml/# optional - sagemath_doc_html/' |
Branch pushed to git repo; I updated commit sha1. New commits:
4c11aab | sed -i.bak 's/dochtml/sagemath_doc_html/' Makefile src/bin/sage-runtests src/doc/en/developer/doctesting.rst |
Is it possible that SAGE_DOC_SRC might be missing? We could do this:
diff --git a/src/sage/doctest/control.py b/src/sage/doctest/control.py
index 858d8cb567..86dbd24d1a 100644
--- a/src/sage/doctest/control.py
+++ b/src/sage/doctest/control.py
@@ -737,13 +737,22 @@ class DocTestController(SageObject):
def all_files():
self.files.append(opj(SAGE_SRC, 'sage'))
- # Don't run these tests when not in the git repository; they are
- # of interest for building sage, but not for runtime behavior and
- # don't make sense to run outside a build environment
- if have_git:
+ # Only test sage_setup and sage_docbuild if the relevant
+ # imports work. They may not work if not in a build
+ # environment or if the documentation build has been
+ # disabled.
+ try:
+ import sage_setup
self.files.append(opj(SAGE_SRC, 'sage_setup'))
+ except ImportError:
+ pass
+ try:
+ import sage_docbuild
self.files.append(opj(SAGE_SRC, 'sage_docbuild'))
- self.files.append(SAGE_DOC_SRC)
+ except ImportError:
+ pass
+ if os.path.isdir(SAGE_DOC_SRC):
+ self.files.append(SAGE_DOC_SRC)
if self.options.all or (self.options.new and not have_git):
self.log("Doctesting entire Sage library.")New commits:
4c11aab | sed -i.bak 's/dochtml/sagemath_doc_html/' Makefile src/bin/sage-runtests src/doc/en/developer/doctesting.rst |
Can we autodetect sagemath_doc_html as an optional tag to add for testing?
Yes, I think it should be autodetected already because sagemath_doc_html is just an installed package
Replying to @jhpalmieri:
Is it possible that
SAGE_DOC_SRCmight be missing?
Yes, I think so, for example in downstream packaging of sage
Replying to @mkoeppe:
Yes, I think it should be autodetected already because
sagemath_doc_htmlis just an installed package
Does that mean we shouldn't manually add it to TESTALL_FLAGS and let it happen automatically?
Yes, that's a good point
Not sure how to implement TESTALL_NODOC_FLAGS then
TESTALL_FLAGS = --optional=sage,sagemath_doc_html,optional,external,build
TESTALL_NODOC_FLAGS = --optional=sage,optional,external,build
Deprecate the nodoc targets and tell people to use ./configure --disable-doc instead?
Or leave it with the two FLAGS being identical? Running make ptest-nodoc runs make build and so shouldn't build sagemath_doc_html, so it shouldn't run tests with that flag, while make ptest runs make and so will build sagemath_doc_html.
Thanks for the analysis, I like the 2nd option
Branch pushed to git repo; I updated commit sha1. New commits:
afa2f80 | Makefile: Get rid of TESTALL_NODOC_FLAGS |
Branch pushed to git repo; I updated commit sha1. New commits:
28cf775 | src/bin/sage-runtests (_get_optional_defaults): No need to include sagemath_doc_html here |
Could you push the change from comment:33 to the ticket please?
Changed branch from u/mkoeppe/__configure___disable_doc to u/jhpalmieri/__configure___disable_doc
Replying to @jhpalmieri:
Should we expect doctests to pass with
./configure --disable-doc? I getsrc/sage/misc/sagedoc.py # 8 doctests failed [...]
Not sure what to do with these ones. What's happening here is that sage.misc.sagedoc.detex has a run-time dependency on Sphinx.
New commits:
10de6fd | trac 32759: doctest sage_setup and sage_docbuild conditionally |
Changed branch from u/jhpalmieri/__configure___disable_doc to u/mkoeppe/__configure___disable_doc
Replying to @mkoeppe:
Yes, I think it should be autodetected already because
sagemath_doc_htmlis just an installed package
Actually, at the moment this is not true because sage.doctest.control only looks at the optional packages.
New commits:
4213f8a | sage.misc.sagedoc: Do not fail if sphinx cannot be imported |
What mechanism adds sage.geometry.polyhedron and sage_spkg to the list of tags?
It's happening in the lines of code in the middle of which I have just inserted a test for sphinx
Branch pushed to git repo; I updated commit sha1. New commits:
7d26e21 | sage.features.sagemath.sagemath_doc_html: New |
Branch pushed to git repo; I updated commit sha1. New commits:
bb1626a | sage.features.sagemath.sagemath_doc_html: Fixup |
Branch pushed to git repo; I updated commit sha1. New commits:
a24c3bd | src/sage/misc/sagedoc.py: Mark some doctests # optional - sphinx |
Branch pushed to git repo; I updated commit sha1. New commits:
3e9770b | sage.features.StaticFile: Accept directories too |
Changed author from Matthias Koeppe to Matthias Koeppe, John Palmieri
By the way, this fails:
make distclean
./configure --disable-doc
make
make doc-html
I think the error boils down to
[reference] intersphinx inventory '/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.5.beta5/local/share/doc/pplpy/objects.inv' not fetchable due to <class 'FileNotFoundError'>: [Errno 2] No such file or directory: '/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.5.beta5/local/share/doc/pplpy/objects.inv'
The problem is that pplpy doesn't build its documentation the first time, and then it doesn't get rebuilt when make doc-html is run. I don't know how to fix this.
With make distclean && ./configure && make ptestlong, I get two doctest failures:
File "src/sage/all.py", line 31, in sage.all
Failed example:
[inspect.getmodule(f).__name__ for f in frames if is_not_allowed(f)]
Expected:
[]
Got:
['pytz', 'importlib.resources']
and
File "src/sage/misc/sagedoc.py", line 31, in sage.misc.sagedoc
Failed example:
"sphinx" in sys.modules
Expected:
False
Got:
True
Replying to @jhpalmieri:
The problem is that
pplpydoesn't build its documentation the first time, and then it doesn't get rebuilt whenmake doc-htmlis run. I don't know how to fix this.
We can introduce a separate package pplpy_doc, something that would actually also make sense for the upstream Python package.
Branch pushed to git repo; I updated commit sha1. New commits:
f681abb | build/bin/sage-spkg: Also source sage-src-env-config so that SAGE_VENV is available |
7438449 | src/bin/sage-env: Do not skip the script if SAGE_ENV_SOURCED was set but SAGE_VENV has changed |
b07c8e3 | build/bin/sage-spkg: Remove workaround of setting of PATH in the generated package scripts |
4af9a24 | Merge #32745 |
810f26f | src/sage/misc/sagedoc.py: Test that sphinx is not imported in a fresh Sage copy |
Branch pushed to git repo; I updated commit sha1. New commits:
7960b90 | src/sage/all.py: Update doctest |
Replying to @jhpalmieri:
With
make distclean && ./configure && make ptestlong, I get two doctest failures:File "src/sage/all.py", line 31, in sage.all Failed example: [inspect.getmodule(f).__name__ for f in frames if is_not_allowed(f)] Expected: [] Got: ['pytz', 'importlib.resources']
I've updated this doctest for the stuff that is pulled in by the doctest framework via Sphinx.
and
File "src/sage/misc/sagedoc.py", line 31, in sage.misc.sagedoc Failed example: "sphinx" in sys.modules Expected: False Got: True
Fixed by starting a fresh interpreter for this test. Had to pull in 2 tickets that fixed issues with recursive invocation of sage
Branch pushed to git repo; I updated commit sha1. New commits:
687e5ba | src/sage/features/sagemath.py: Add doctests |
With --disable-doc, make doc-html goes through but ends with
touch: /var/lib/sage/installed/sagemath_doc_html-none: No such file or directory
make[2]: *** [sagemath_doc_html-SAGE_DOCS-no-deps] Error 1
For reasons I don't understand, my computer is now refusing to build the plots in the documentation — it keeps hanging in references/plotting. After fighting with this for a while, I've given in and am resuming testing using export SAGE_DOCBUILD_OPTS=' --no-plot'.
(Not related to this ticket: this happens with the develop branch, too.)
Branch pushed to git repo; I updated commit sha1. New commits:
40a9760 | build/make/Makefile.in: Raise an error if installation into disabled installation tree is attempted |
Replying to @mkoeppe:
With
--disable-doc,make doc-htmlgoes through but ends withtouch: /var/lib/sage/installed/sagemath_doc_html-none: No such file or directory make[2]: *** [sagemath_doc_html-SAGE_DOCS-no-deps] Error 1
Fixed now by explicitly raising a clear error.
Do we want make doc, make doc-html to work when --disable-doc has been configured?
Replying to @mkoeppe:
Do we want
make doc,make doc-htmlto work when--disable-dochas been configured?
Good question, I don't know. Should the following work? Does it work?
make distclean
./configure --disable-doc
make
./configure --enable-doc # without make distclean or similar
make doc-html
Replying to @jhpalmieri:
Replying to @mkoeppe:
Do we want
make doc,make doc-htmlto work when--disable-dochas been configured?Good question, I don't know.
I think I would be happy if it didn't work but if there were a clear error message.
% make doc
...
Error: Sage build configured with --disable-doc, so building the documentation will not work. To fix this, ...
Replying to @jhpalmieri:
Should the following work? Does it work?
make distclean ./configure --disable-doc make ./configure --enable-doc # without make distclean or similar make doc-html
Yes, I think this works
Branch pushed to git repo; I updated commit sha1. New commits:
51f3002 | build/make/Makefile.in: Better error message when docbuild is attempted but configure --disable-doc has been used |