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

sageWithDoc: fix documentation index #189776

Merged
merged 1 commit into from
Sep 6, 2022
Merged

Conversation

collares
Copy link
Member

@collares collares commented Sep 4, 2022

Description of changes

Since basically forever, Sage docbuilding on Nix generates harmless errors of the following form:

Extension error (matplotlib.sphinxext.plot_directive):
Handler <function _copy_css_file at 0x7fd51e53c8b0> for event 'build-finished' threw an exception (exception: [Errno 13] Permission denied: '<dir>/share/doc/sage/html/en/reference/valuations/_static/plot_directive.css')

This happens because the matplotlib Sphinx extension copies a CSS file from the Nix store to reference/SUBDIR/_static using Python's shutil.copy, resulting in a read-only file. Because Sage docbuilding symlinks reference/SUBDIR/_static to reference/_static, the copying effectively happens multiple times, and therefore we get a "Permission denied" error for all but one of the 78 reference/ subdirectories.

Sage builds docs in two passes for parallelism: Subdirectories are built separately, and then a second pass stitches them together by looking at some state files (environment.pickle in particular) created by Sphinx in each of the documentation subdirectories. Sphinx deletes these state files if the build fails, but functions registered as "build-finished" callbacks are an interesting gray area: What happens if they fail? Well, before Sphinx 5 this was ignored, but Sphinx 5 considers this to be a build failure and deletes the relevant state files. Without them, Sage docbuilding can't create the documentation index.

This PR works around the issue by patching Sage's docbuilding to change plot_directive.css's permissions. I have submitted matplotlib/matplotlib#23805 to fix this properly, but since it will probably take a while to reach a release and the bug is almost Sage+Nix-specific, I'd rather get the workaround in Nixpkgs first (it fixes a user-reported problem).

Fixes #189607 (cc @aikrahguzar)

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Sep 4, 2022
@collares
Copy link
Member Author

collares commented Sep 5, 2022

Unfortunately, aarch64 is still completely borked and I have no clue how to debug it (there's more about it on https://trac.sagemath.org/ticket/34297). This is pre-existing, though.

@aikrahguzar
Copy link

Thanks @collares! Is there an easy way I can check this? I don't know anything about nix apart from simple nix-env invocations.

@collares
Copy link
Member Author

collares commented Sep 5, 2022

@aikrahguzar If you don't mind waiting for a while as Sage gets built (on your machine) by Nix, you can run nix-build -I nixpkgs=https://github.com/collares/nixpkgs/archive/382129c3a83f7e58b9fa57c18f516b471bbf6826.tar.gz -E 'with (import <nixpkgs> {}); sage.doc'. This will print a path as the very last line, as well as create a result symlink in your current directory. Both of these should point to the new docs.

+ css_file = os.path.join(app.builder.outdir, '_static', 'plot_directive.css')
+ try:
+ os.chmod(css_file, 0o644)
+ except:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the above try/except is just for future-proofing. All os.chmod calls introduced by my patch succeed as of Sage 9.6.

@aikrahguzar
Copy link

aikrahguzar commented Sep 5, 2022

@aikrahguzar If you don't mind waiting for a while as Sage gets built (on your machine) by Nix, you can run nix-build -I nixpkgs=https://github.com/collares/nixpkgs/archive/382129c3a83f7e58b9fa57c18f516b471bbf6826.tar.gz -E 'with (import <nixpkgs> {}); sage.doc'. This will print a path as the very last line, as well as create a result symlink in your current directory. Both of these should point to the new docs.

I tried this and I can confirm that I get a full index now. Thanks a lot!
Edit: This does solve the issue of generating a docset too, so thanks again!

Copy link
Member

@timokau timokau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @collares, great work as always :)

@timokau timokau merged commit b292bfd into NixOS:master Sep 6, 2022
@collares collares deleted the sage-docindex branch January 19, 2023 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sage docs don't have a complete index
3 participants