Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sage doc does not even start to build with sphinx 7.3.7 #37996

Closed
2 tasks done
kiwifb opened this issue May 12, 2024 · 12 comments · Fixed by #38100
Closed
2 tasks done

sage doc does not even start to build with sphinx 7.3.7 #37996

kiwifb opened this issue May 12, 2024 · 12 comments · Fixed by #38100

Comments

@kiwifb
Copy link
Member

kiwifb commented May 12, 2024

Steps To Reproduce

Found this while trying to build sage-doc in sage-on-gentoo with python 3.12 and sphinx 7.3.7/docutils-0.21.2 (downgrading docutils to a previous working version did not help)

Expected Behavior

It builds successfully

Actual Behavior

make -j8 doc-html 
mkdir -p en/reference/repl
sage -advanced > en/reference/repl/options.txt
make doc-inventory--reference-references
make[1]: Entering directory '/home/portage/sci-mathematics/sage-doc-9999/work/sage-doc-9999/src/doc'
sage --docbuild --no-pdf-links reference/references inventory 
[reference] building [inventory]: targets for 1 source files that are out of date
[reference] updating environment: [new config] 1 added, 0 changed, 0 removed
[reference] Exception occurred:
[reference]   File "/usr/lib/python3.12/site-packages/sphinx/config.py", line 466, in __getstate__
[reference]     self._options[name] = opt = _Opt(*opt)
[reference]                                 ^^^^^^^^^^
[reference] TypeError: _Opt.__init__() missing 1 required positional argument: 'valid_types'
[reference] The full traceback has been saved in /home/portage/sci-mathematics/sage-doc-9999/temp/sphinx-err-lf9_1h84.log, if you want to report the issue to the developers.
[reference] Please also report this if it was a user error, so that a better error message can be provided next time.
[reference] A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
Error building the documentation.
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/usr/lib/python3.12/site-packages/sage_docbuild/__main__.py", line 514, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/lib/python3.12/site-packages/sage_docbuild/__main__.py", line 510, in main
    builder()
  File "/usr/lib/python3.12/site-packages/sage_docbuild/builders.py", line 823, in _wrapper
    getattr(DocBuilder, build_type)(self, *args, **kwds)
  File "/usr/lib/python3.12/site-packages/sage_docbuild/builders.py", line 163, in f
    runsphinx()
  File "/usr/lib/python3.12/site-packages/sage_docbuild/sphinxbuild.py", line 319, in runsphinx
    sys.stderr.raise_errors()
  File "/usr/lib/python3.12/site-packages/sage_docbuild/sphinxbuild.py", line 255, in raise_errors
    raise OSError(self._error)
OSError: Exception occurred:

    Note: incremental documentation builds sometimes cause spurious
    error messages. To be certain that these are real errors, run
    "make doc-clean doc-uninstall" first and try again.
make[1]: *** [Makefile:28: doc-inventory--reference-references] Error 1
make[1]: Leaving directory '/home/portage/sci-mathematics/sage-doc-9999/work/sage-doc-9999/src/doc'
make: *** [Makefile:40: doc-inventory-reference] Error 2

Additional Information

No response

Environment

- gentoo
- sage 10.4.beta5

Checklist

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.
  • I have read the documentation and troubleshoot guide
@collares
Copy link
Contributor

collares commented May 13, 2024

Sphinx 7.3 doesn't like the two lines at https://github.com/sagemath/sage/blob/develop/src/sage_docbuild/conf.py#L840-L841. They conflict with a workaround added in sphinx-doc/sphinx@0c06a50.

The values of nitpicky and nitpick_ignore in upstream sphinx/config.py are _Opt(False, '', ()), and _Opt([], '', frozenset((set, list, tuple))) respectively, by the way. A comment in the same file says that the parameters are

    # 1. Default
    # 2. What needs to be rebuilt if changed
    # 3. Valid types

(I suppose an empty tuple for the last one means "don't validate this type"?)

cc @kwankyu.

@kiwifb
Copy link
Member Author

kiwifb commented May 13, 2024

Thanks for digging, I created a report so I would not forget to do it later :)

@collares
Copy link
Contributor

collares commented May 14, 2024

If I comment the app.connect('builder-inited', nitpick_patch_config) line in conf.py to avoid the problem mentioned above, docbuilding proceeds for a bit, but the second pass fails with

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/nix/store/vd67bwg1v5kmbdi8kw01ws3c5wjg4cnm-python3-3.11.9-env/lib/python3.11/site-packages/sage_docbuild/__main__.py", line 509, in <module>
    sys.exit(main())
             ^^^^^^
  File "/nix/store/vd67bwg1v5kmbdi8kw01ws3c5wjg4cnm-python3-3.11.9-env/lib/python3.11/site-packages/sage_docbuild/__main__.py", line 505, in main
    builder()
  File "/nix/store/vd67bwg1v5kmbdi8kw01ws3c5wjg4cnm-python3-3.11.9-env/lib/python3.11/site-packages/sage_docbuild/builders.py", line 788, in _wrapper
    for module_name in self.get_new_and_updated_modules():
  File "/nix/store/vd67bwg1v5kmbdi8kw01ws3c5wjg4cnm-python3-3.11.9-env/lib/python3.11/site-packages/sage_docbuild/builders.py", line 959, in get_new_and_updated_modules
    env = self.get_sphinx_environment()
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/vd67bwg1v5kmbdi8kw01ws3c5wjg4cnm-python3-3.11.9-env/lib/python3.11/site-packages/sage_docbuild/builders.py", line 857, in get_sphinx_environment
    env.config.values = env.app.config.values
    ^^^^^^^^^^^^^^^^^
AttributeError: property 'values' of 'Config' object has no setter

values was made private in sphinx-doc/sphinx@68af0ac2be, but maybe accessing _options is good enough for now.

@mkoeppe
Copy link
Contributor

mkoeppe commented May 26, 2024

Should we set an upper bound sphinx < 7.3 until this is resolved?

@kiwifb
Copy link
Member Author

kiwifb commented May 26, 2024

I think that's one case where setting an upper limit will not be facing too much opposition. I would hope.

@kwankyu
Copy link
Collaborator

kwankyu commented May 27, 2024

If I comment the app.connect('builder-inited', nitpick_patch_config) line in conf.py to avoid the problem mentioned above, docbuilding proceeds for a bit, but the second pass fails with

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/nix/store/vd67bwg1v5kmbdi8kw01ws3c5wjg4cnm-python3-3.11.9-env/lib/python3.11/site-packages/sage_docbuild/__main__.py", line 509, in <module>
    sys.exit(main())
             ^^^^^^
  File "/nix/store/vd67bwg1v5kmbdi8kw01ws3c5wjg4cnm-python3-3.11.9-env/lib/python3.11/site-packages/sage_docbuild/__main__.py", line 505, in main
    builder()
  File "/nix/store/vd67bwg1v5kmbdi8kw01ws3c5wjg4cnm-python3-3.11.9-env/lib/python3.11/site-packages/sage_docbuild/builders.py", line 788, in _wrapper
    for module_name in self.get_new_and_updated_modules():
  File "/nix/store/vd67bwg1v5kmbdi8kw01ws3c5wjg4cnm-python3-3.11.9-env/lib/python3.11/site-packages/sage_docbuild/builders.py", line 959, in get_new_and_updated_modules
    env = self.get_sphinx_environment()
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/vd67bwg1v5kmbdi8kw01ws3c5wjg4cnm-python3-3.11.9-env/lib/python3.11/site-packages/sage_docbuild/builders.py", line 857, in get_sphinx_environment
    env.config.values = env.app.config.values
    ^^^^^^^^^^^^^^^^^
AttributeError: property 'values' of 'Config' object has no setter

values was made private in sphinx-doc/sphinx@68af0ac2be, but maybe accessing _options is good enough for now.

Removing local/share/doc/sage completely makes it go through. Some leftover files (inventory files) seem to be troublesome. That is, do make doc-clean doc-uninstall and try again (or #38100).

@collares
Copy link
Contributor

collares commented May 27, 2024

@kwankyu Interesting, that's not what I'm seeing here. I'm still using sage --docbuild all html instead of make doc-html to start the build for distro reasons, but hopefully this shouldn't make a difference. While docbuilding on a clean sandbox, make manages to run the following steps successfully (each involving one sage --docbuild run per target):

  • make doc-inventory--reference-references
  • make SAGE_DOCBUILD_OPTS=" --no-prune-empty-dirs" doc-inventory--reference-spkg doc-inventory--reference-manifolds doc-inventory--reference-algebras doc-inventory--reference-polynomial_rings doc-inventory--reference-repl doc-inventory--reference-tensor_free_modules doc-inventory--reference-combinat doc-inventory--reference-dynamics doc-inventory--reference-plot3d doc-inventory--reference-arithgroup doc-inventory--reference-graphs doc-inventory--reference-misc doc-inventory--reference-parallel doc-inventory--reference-topology doc-inventory--reference-arithmetic_curves doc-inventory--reference-asymptotic doc-inventory--reference-calculus doc-inventory--reference-categories doc-inventory--reference-coding doc-inventory--reference-coercion doc-inventory--reference-constants doc-inventory--reference-cpython doc-inventory--reference-cryptography doc-inventory--reference-curves doc-inventory--reference-data_structures doc-inventory--reference-databases doc-inventory--reference-diophantine_approximation doc-inventory--reference-discrete_geometry doc-inventory--reference-doctest doc-inventory--reference-documentation doc-inventory--reference-drinfeld_modules doc-inventory--reference-euclidean_spaces doc-inventory--reference-finite_rings doc-inventory--reference-function_fields doc-inventory--reference-functions doc-inventory--reference-game_theory doc-inventory--reference-games doc-inventory--reference-groups doc-inventory--reference-hecke doc-inventory--reference-history_and_license doc-inventory--reference-homology doc-inventory--reference-hyperbolic_geometry doc-inventory--reference-interfaces doc-inventory--reference-knots doc-inventory--reference-lfunctions doc-inventory--reference-libs doc-inventory--reference-logic doc-inventory--reference-matrices doc-inventory--reference-matroids doc-inventory--reference-modabvar doc-inventory--reference-modfrm doc-inventory--reference-modsym doc-inventory--reference-modules doc-inventory--reference-monoids doc-inventory--reference-noncommutative_polynomial_rings doc-inventory--reference-number_fields doc-inventory--reference-numerical doc-inventory--reference-padics doc-inventory--reference-plotting doc-inventory--reference-power_series doc-inventory--reference-probability doc-inventory--reference-quadratic_forms doc-inventory--reference-quat_algebras doc-inventory--reference-quivers doc-inventory--reference-resolutions doc-inventory--reference-riemannian_geometry doc-inventory--reference-rings doc-inventory--reference-rings_numerical doc-inventory--reference-rings_standard doc-inventory--reference-sat doc-inventory--reference-schemes doc-inventory--reference-semirings doc-inventory--reference-sets doc-inventory--reference-stats doc-inventory--reference-structure doc-inventory--reference-valuations
  • make SAGE_DOCBUILD_OPTS=" --no-prune-empty-dirs" doc-inventory--reference_top

After that, each of the subdirectories inside share/doc/sage/doctrees/en/reference contains an environment.pickle file and a reference.pickle file. It then tries to run make SAGE_DOCBUILD_OPTS=" --no-prune-empty-dirs" doc-html--reference-references, which runs sage --docbuild --no-pdf-links reference/references html --no-prune-empty-dir and fails with the error I pasted in the previous comment.

@kwankyu
Copy link
Collaborator

kwankyu commented May 27, 2024

Sorry. My sage install was actually using Sphinx 7.2.6. So it didn't complain about the code

env.config.values = env.app.config.values

in /sage_docbuild/builders.py, line 857, in get_sphinx_environment. Certainly that line could be problematic with sphinx 7.3.7, where config values are treated differently.

I set #38100 to "needs work" as it is not yet ready for sphinx 7.3.7.

@kwankyu
Copy link
Collaborator

kwankyu commented May 27, 2024

By the way, sage --docbuild all html also works with me, with Sphinx 7.2.6.

@collares
Copy link
Contributor

collares commented May 27, 2024

I just commented out the env.config.values = env.app.config.values (and the corresponding del env.config.values, line 915, when pickling) and, from a cursory glance, the documentation seems to be generated correctly with Sphinx 7.3. Maybe the configuration is now pickle-able? Not sure if this is desired in incremental builds, though.

@kwankyu
Copy link
Collaborator

kwankyu commented May 27, 2024

Yes, I am experimenting along the line similar to yours. Let's see.

@kwankyu
Copy link
Collaborator

kwankyu commented May 28, 2024

#38100 works well with sphinx 7.3.7 and is ready for review.

vbraun pushed a commit to vbraun/sage that referenced this issue May 29, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Fixes sagemath#37996.

We remove this function

```python
def nitpick_patch_config(app):
    """
    Patch the default config for nitpicky
    [...] ensure that nitpicky is not considered as a Sphinx
    environment variable but rather as a Sage environment variable. As a
    consequence, changing it doesn't force the recompilation of the
entire
    documentation.
    """
    app.config.values['nitpicky'] = (False, 'sage')
    app.config.values['nitpick_ignore'] = ([], 'sage')
```
from `src/sage_docbuild/conf.py`.

The original purpose of the function seems to change `"env"` to `"sage"`
for `nitpicky` config variable. But the recent Sphinx is setting `""`
(empty string) instead of `"env"` by default. See
https://github.com/sphinx-doc/sphinx/blob/48cbb43e28efe82b198137042811e0
ade3599ae7/sphinx/config.py#L250C1-L250C41

- Hence  the function is already no-op. But this old and obsolete
function is the cause of sagemath#37996. Hence we simply remove it.

- To prepare for Sphinx 7.3.7, we also remove some code which seems to
have become obsolete when `sage -clone` was removed long time ago.

- Made some docstring up-to-date, to test incremental doc build.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38100
Reported by: Kwankyu Lee
Reviewer(s):
vbraun pushed a commit to vbraun/sage that referenced this issue May 31, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Fixes sagemath#37996.

We remove this function

```python
def nitpick_patch_config(app):
    """
    Patch the default config for nitpicky
    [...] ensure that nitpicky is not considered as a Sphinx
    environment variable but rather as a Sage environment variable. As a
    consequence, changing it doesn't force the recompilation of the
entire
    documentation.
    """
    app.config.values['nitpicky'] = (False, 'sage')
    app.config.values['nitpick_ignore'] = ([], 'sage')
```
from `src/sage_docbuild/conf.py`.

The original purpose of the function seems to change `"env"` to `"sage"`
for `nitpicky` config variable. But the recent Sphinx is setting `""`
(empty string) instead of `"env"` by default. See
https://github.com/sphinx-doc/sphinx/blob/48cbb43e28efe82b198137042811e0
ade3599ae7/sphinx/config.py#L250C1-L250C41

- Hence  the function is already no-op. But this old and obsolete
function is the cause of sagemath#37996. Hence we simply remove it.

- To prepare for Sphinx 7.3.7, we also remove some code which seems to
have become obsolete when `sage -clone` was removed long time ago.

- Made some docstring up-to-date, to test incremental doc build.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38100
Reported by: Kwankyu Lee
Reviewer(s):
@mkoeppe mkoeppe added this to the sage-10.4 milestone Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants