Repository navigation
[BUG] redirect-from sphinx extension is not parallel safe #24058
Description
Summary
CI builds the docs in parallel since #20718.
This doesn't work correctly with redirects: in #24054 it raises
Traceback (most recent call last):
File "/home/circleci/.local/lib/python3.8/site-packages/sphinx/cmd/build.py", line 279, in build_main
app.build(args.force_all, filenames)
File "/home/circleci/.local/lib/python3.8/site-packages/sphinx/application.py", line 350, in build
self.builder.build_update()
File "/home/circleci/.local/lib/python3.8/site-packages/sphinx/builders/__init__.py", line 302, in build_update
self.build(to_build,
File "/home/circleci/.local/lib/python3.8/site-packages/sphinx/builders/__init__.py", line 318, in build
updated_docnames = set(self.read())
File "/home/circleci/.local/lib/python3.8/site-packages/sphinx/builders/__init__.py", line 423, in read
self._read_parallel(docnames, nproc=self.app.parallel)
File "/home/circleci/.local/lib/python3.8/site-packages/sphinx/builders/__init__.py", line 479, in _read_parallel
tasks.join()
File "/home/circleci/.local/lib/python3.8/site-packages/sphinx/util/parallel.py", line 105, in join
if not self._join_one():
File "/home/circleci/.local/lib/python3.8/site-packages/sphinx/util/parallel.py", line 126, in _join_one
raise SphinxParallelError(*result)
sphinx.errors.SphinxParallelError: ValueError: gallery/pyplots/dollar_ticks is already noted as redirecting to gallery/ticks/dollar_ticks
Sphinx parallel build error:
ValueError: gallery/pyplots/dollar_ticks is already noted as redirecting to gallery/ticks/dollar_ticks
(see also #21876)
Proposed fix
Revert afef2e6.
Activity
jklymak commented on Sep 30, 2022
We should probably figure out why we get the error
Also note that since we introduced this mechanism, I think there is a proper package that does this. Whether they handle parallel better or not I'm not sure.
Finally, we don't host on GitPages any longer and could do this with .htaccess now if we chose (I think).
timhoffm commented on Sep 30, 2022
It may be that our redirect directive is not thread-safe. Maybe we have to revert #19686 instead. - Either way, we definitively want parallelism for speed, and need to find a different solution.
StefRe commented on Sep 30, 2022
Please accept my apology. I included the same redirect in two different files. This error didn't show up for non-parallel builds: the second redirect just overwrites the first one. I fixed this mistake and the docs build in parallel without errors.
timhoffm commented on Dec 19, 2022
I'll reopen because the error hit again in a recent CI run.
I rename the issue to state the problem. Simply not building in parallel cannot be a solution because of the performance impact. The error seems rare enough so that the best temporary solution is continuing as is and restarting Circle CI when the error hits.
Mid-term, we should either make redirect-from parallel-safe or find a different solution. We cannot afford a sphinx extension that prevents doc builds in parallel.
jklymak commented on Dec 19, 2022
Do we understand why this directive would not be parallel safe?
oscargus commented on Dec 19, 2022
What was the error in that recent run? I think that the original error is not related to parallel execution, but just re-thrown as a SphinxParallelError.
timhoffm commented on Dec 19, 2022
In the recent run, the error was exactly the same as above.
The error is thrown from here
matplotlib/doc/sphinxext/redirect_from.py
Lines 92 to 103 in 2e64a9e
which should only be run once per .. redirect-from:: directive.
Maybe it has to do with using the domain for data storage and not handling that correctly on our side? At least https://www.sphinx-doc.org/en/master/development/tutorials/todo.html attaches its data todo_all_todos to the environment, not a domain.
Edit: No, sphinx-doc/sphinx#9003 (comment) suggests that attaching data to a domain is actually the correct way.
timhoffm commented on Dec 19, 2022
Btw. it just hit again in a run of #24769:
https://app.circleci.com/pipelines/github/matplotlib/matplotlib/21068/workflows/9b157c38-fd55-4ba7-8cef-a002aa760b38/jobs/73024