Opened 9 months ago

Closed 8 months ago

#33064 closed defect (fixed)

sage_docbuild: fails when cache cannot be saved

Reported by: Gonzalo Tornaría Owned by:
Priority: major Milestone: sage-9.6
Component: doctest framework Keywords:
Cc: John Palmieri Merged in:
Authors: Gonzalo Tornaría Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: e89193f (Commits, GitHub, GitLab) Commit: e89193f4f32246ad9ded6d37f1f2f12a589a81f4
Dependencies: Stopgaps:

Status badges

Description

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).

Change History (22)

comment:1 Changed 9 months ago by Gonzalo Tornaría

Authors: Gonzalo Tornaría
Branch: u/tornaria/sage_docbuild-save_cache
Commit: f767cf1eadfd8ab540a31c5c5ef09d56df09b90e
Status: newneeds_review

New commits:

f767cf1sage_docbuild: do not fail when cache cannot be saved

comment:2 Changed 9 months ago by Gonzalo Tornaría

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().

comment:3 in reply to:  2 Changed 9 months ago by Gonzalo Tornaría

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

comment:4 Changed 9 months ago by Matthias Köppe

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

comment:5 Changed 9 months ago by Matthias Köppe

Cc: John Palmieri added

comment:6 in reply to:  4 Changed 9 months ago by Gonzalo Tornaría

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.

comment:7 Changed 9 months ago by John Palmieri

See #22062 for sqlite.log.

comment:8 Changed 9 months ago by git

Commit: f767cf1eadfd8ab540a31c5c5ef09d56df09b90ea8973f2440f2f51a29cefb6973d5e12a3b2386b3

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

comment:9 Changed 9 months ago by Gonzalo Tornaría

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

comment:10 Changed 9 months ago by John Palmieri

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.

comment:11 in reply to:  10 Changed 9 months ago by Gonzalo Tornaría

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).

comment:12 Changed 9 months ago by John Palmieri

Reviewers: John Palmieri
Status: needs_reviewpositive_review

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

comment:13 Changed 9 months ago by git

Commit: a8973f2440f2f51a29cefb6973d5e12a3b2386b3e89193f4f32246ad9ded6d37f1f2f12a589a81f4
Status: positive_reviewneeds_review

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

comment:14 Changed 9 months ago by Gonzalo Tornaría

Status: needs_reviewpositive_review

Rebased to 9.5.rc0 without change.

comment:15 Changed 9 months ago by François Bissey

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.

comment:16 Changed 9 months ago by John Palmieri

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

comment:17 in reply to:  16 Changed 9 months ago by François Bissey

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.

comment:18 Changed 9 months ago by Matthias Köppe

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

comment:19 in reply to:  18 Changed 9 months ago by François Bissey

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.

comment:20 Changed 9 months ago by Matthias Köppe

Thanks! I've added this to #29868.

comment:21 Changed 8 months ago by Matthias Köppe

Milestone: sage-9.5sage-9.6

comment:22 Changed 8 months ago by Volker Braun

Branch: u/tornaria/sage_docbuild-save_cachee89193f4f32246ad9ded6d37f1f2f12a589a81f4
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.