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_docbuild: fails when cache cannot be saved #33064

Closed
tornaria opened this issue Dec 21, 2021 · 26 comments
Closed

sage_docbuild: fails when cache cannot be saved #33064

tornaria opened this issue Dec 21, 2021 · 26 comments

Comments

@tornaria
Copy link
Contributor

When doctesting src/sage_docbuild/__init__.py on a read-only location we get a failure which ultimately boils down to:

┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 9.5.beta8, Release Date: 2021-12-12               │
│ Using Python 3.10.1. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
sage: from sage_docbuild import DocBuilder, setup_parser, ReferenceSubBuilder
sage: DocBuilder._options = setup_parser().parse_args([])
sage: import sage_docbuild.sphinxbuild
sage: def raiseBaseException():
....:     raise BaseException("abort pool operation")
....: 
sage: original_runsphinx, sage_docbuild.sphinxbuild.runsphinx = sage_docbuild.sphinxbuild.runsphinx, raiseBaseException
sage: ReferenceSubBuilder("docname", "en")._wrapper("html")
---------------------------------------------------------------------------
PermissionError                           Traceback (most recent call last)
<ipython-input-11-7faec88fcc76> in <module>
----> 1 ReferenceSubBuilder("docname", "en")._wrapper("html")

/usr/lib/sage-9.5.beta8/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sage_docbuild/__init__.py in _wrapper(self, build_type, *args, **kwds)
    797         cache['option_inherited'] = self._options.inherited
    798         cache['option_underscore'] = self._options.underscore
--> 799         self.save_cache()
    800 
    801         # After "sage -clone", refresh the reST file mtimes in

/usr/lib/sage-9.5.beta8/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sage_docbuild/__init__.py in save_cache(self)
    857         """
    858         cache = self.get_cache()
--> 859         with open(self.cache_filename(), 'wb') as file:
    860             pickle.dump(cache, file)
    861         logger.debug("Saved the reference cache: %s", self.cache_filename())

PermissionError: [Errno 13] Permission denied: '/usr/lib/sage-9.5.beta8/local/share/doc/sage/doctrees/en/docname/reference.pickle'

This is due to a save_cache() method trying to save into that file; I believe it would be harmless to ignore this permission error (if the cache is not saved, it will have to be regenerated, so what).

CC: @jhpalmieri

Component: doctest framework

Author: Gonzalo Tornaría

Branch/Commit: e89193f

Reviewer: John Palmieri

Issue created by migration from https://trac.sagemath.org/ticket/33064

@tornaria
Copy link
Contributor Author

Branch: u/tornaria/sage_docbuild-save_cache

@tornaria
Copy link
Contributor Author

New commits:

f767cf1sage_docbuild: do not fail when cache cannot be saved

@tornaria
Copy link
Contributor Author

Commit: f767cf1

@tornaria
Copy link
Contributor Author

Author: Gonzalo Tornaría

@tornaria
Copy link
Contributor Author

comment:2

A related but independent issue is that several methods whose main purpose is to return a path (e.g. DocBuilder._output_dir) also attempt to create the directory even if it won't be used. This issue I can fix by including the required empty directories (although some like .../doc/sage/*/en/docname are just an artifact of running doctests).

I wonder if it would make sense to ignore PermissionError in those calls to sage_makedirs().

@tornaria
Copy link
Contributor Author

comment:3

Replying to @tornaria:

I wonder if it would make sense to ignore PermissionError in those calls to sage_makedirs().

Or else maybe tag these doctests with # optional - dochtml

@mkoeppe
Copy link
Member

mkoeppe commented Dec 21, 2021

comment:4

I'd say these tests should probably not write into the production environment...

@tornaria
Copy link
Contributor Author

comment:6

Replying to @mkoeppe:

I'd say these tests should probably not write into the production environment...

Running tests on a read-write directory creates all of this:

$ find local/share/doc/sage
local/share/doc/sage
local/share/doc/sage/html
local/share/doc/sage/html/en
local/share/doc/sage/html/en/tutorial
local/share/doc/sage/html/en/docname
local/share/doc/sage/html/en/reference
local/share/doc/sage/doctrees
local/share/doc/sage/doctrees/en
local/share/doc/sage/doctrees/en/tutorial
local/share/doc/sage/doctrees/en/docname
local/share/doc/sage/doctrees/en/docname/reference.pickle

Failure to write reference.pickle is no longer a failure after the patch in this ticket. If any of the directories doesn't exist and cannot be created, we get a failure.

Also created on a test run are: local/lib/sage-current-location.txt and logs/pkgs/sqlite.log.

@jhpalmieri
Copy link
Member

comment:7

See #22062 for sqlite.log.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 26, 2021

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a8973f2sage_docbuild: do not fail when cache cannot be saved

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 26, 2021

Changed commit from f767cf1 to a8973f2

@tornaria
Copy link
Contributor Author

comment:9

Followup on #33085 for the task that doctests pass when html documentation is not built/installed.

@jhpalmieri
Copy link
Member

comment:10

The change makes sense to me, but I don't understand the situation in which this will arise. I made the directory local/share/doc/sage unwritable and ran doctests. I got failures, but none appeared to come from save_cache. I think I'm missing something.

@tornaria
Copy link
Contributor Author

tornaria commented Jan 6, 2022

comment:11

Replying to @jhpalmieri:

The change makes sense to me, but I don't understand the situation in which this will arise. I made the directory local/share/doc/sage unwritable and ran doctests. I got failures, but none appeared to come from save_cache. I think I'm missing something.

Make sure the directory local/share/doc/sage/doctrees/en/docname/ exists, otherwise there will be a failure when calling some _output_dir() method from self.cache_filename().

This seems to be a cache of filenames or something... that is computed (or retrieved if cache file present) by self.get_cache() and then saved back to local/share/doc/sage/doctrees/en/docname/reference.pickle by the call to pickle.dump().

For me I think what triggered this is: I build and doctest sagemath before packaging (in that check I have write permission). The package gets installed in a live system at /usr/lib/sagemath... where I don't have write permission. The reference.pickle file mentioned above was left over from the build doctest. Running the sequence in the description of the ticket (which reproduces part of a doctest) gives the failure, possibly given that the reference.pickle file exists.

Note that I'm building with make build so documentation is not built at this stage. I'm aiming for doctests to pass 100% at build time (rw tree) and also pass 100% when installed (ro tree).

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:12

I still can't reproduce this, but as I said, the change makes sense.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 9, 2022

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

e89193fsage_docbuild: do not fail when cache cannot be saved

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 9, 2022

Changed commit from a8973f2 to e89193f

@tornaria
Copy link
Contributor Author

tornaria commented Jan 9, 2022

comment:14

Rebased to 9.5.rc0 without change.

@kiwifb
Copy link
Member

kiwifb commented Jan 11, 2022

comment:15

That doesn't fix anything for me in terms of doctests failure after install in a read only location.

sage -t --long --random-seed=204418260052577960635697341654995479733 /usr/lib/python3.10/site-packages/sage_docbuild/__init__.py
**********************************************************************
File "/usr/lib/python3.10/site-packages/sage_docbuild/__init__.py", line 108, in sage_docbuild.builder_helper
Failed example:
    try:  # optional - sagemath_doc_html
        build_many(build_ref_doc, [("docname", "en", "html", {})])
    except Exception as E:
        "Non-exception during docbuild: abort pool operation" in str(E)
Expected:
    True
Got:
    False
**********************************************************************
File "/usr/lib/python3.10/site-packages/sage_docbuild/__init__.py", line 206, in sage_docbuild.DocBuilder._doctrees_dir
Failed example:
    b._doctrees_dir()             # optional - sagemath_doc_html
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.10/site-packages/sage/doctest/forker.py", line 694, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.10/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage_docbuild.DocBuilder._doctrees_dir[2]>", line 1, in <module>
        b._doctrees_dir()             # optional - sagemath_doc_html
      File "/usr/lib/python3.10/site-packages/sage_docbuild/__init__.py", line 210, in _doctrees_dir
        sage_makedirs(d)
      File "/usr/lib/python3.10/site-packages/sage/misc/misc.py", line 90, in sage_makedirs
        os.makedirs(dirname)
      File "/usr/lib/python3.10/os.py", line 215, in makedirs
        makedirs(head, exist_ok=exist_ok)
      File "/usr/lib/python3.10/os.py", line 215, in makedirs
        makedirs(head, exist_ok=exist_ok)
      File "/usr/lib/python3.10/os.py", line 225, in makedirs
        mkdir(name, mode)
    PermissionError: [Errno 13] Permission denied: '/usr/share/doc/sage-doc-9999/doctrees'
**********************************************************************

Both fail because they try to create /usr/share/doc/sage-doc-9999/doctrees which I don't install because it has no use at runtime. If someone show me a use for it (aside from passing doctests) at runtime, I'd include it in sage-on-gentoo installation.

@jhpalmieri
Copy link
Member

comment:16

Does #33085 help? The fix on this ticket was for a very specific problem.

@kiwifb
Copy link
Member

kiwifb commented Jan 11, 2022

comment:17

Replying to @jhpalmieri:

Does #33085 help? The fix on this ticket was for a very specific problem.

No, I have both included. The label sagemath_doc_html triggers doctesting when html documentation can be found, which is my case. The real issue is that vanilla sage build the documentation in place and that lead people to think that build artifacts like doctrees are part of a normal install when they aren't. The only place in sage outside of sage_docbuild that refers to doctrees is in sage/misc/sphinxify.py and I am not completely sure it should not be relocated.

@mkoeppe
Copy link
Member

mkoeppe commented Jan 11, 2022

comment:18

+1 on investigating whether sage/misc/sphinxify.py really depends on the doctrees

@kiwifb
Copy link
Member

kiwifb commented Jan 12, 2022

comment:19

Replying to @mkoeppe:

+1 on investigating whether sage/misc/sphinxify.py really depends on the doctrees

Short answer: no

Long answer: a "doctrees" folder is passed to the sphinx app, but it is a temporary one. The relevant lines in sphinxify.py are

    srcdir = mkdtemp()
...
    doctreedir = os.path.join(srcdir, 'doctrees')
...
    sphinx_app = Sphinx(srcdir, confdir, outdir, doctreedir, format,
                        confoverrides, None, None, True)

So, a temporary "doctrees" folder is used by sphinxify and it has no relationship with what is normally in $SAGE_LOCAL/share/doc/sage/doctrees.

Grepping sage's source for "doctrees" returned a false positive in that case.

@mkoeppe
Copy link
Member

mkoeppe commented Jan 12, 2022

comment:20

Thanks! I've added this to #29868.

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Jan 28, 2022
@vbraun
Copy link
Member

vbraun commented Feb 12, 2022

Changed branch from u/tornaria/sage_docbuild-save_cache to e89193f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants