Declare sphinxext.redirect_from parallel_read_safe #19686
Conversation
|
But is is actually parallel_read_safe as is? Should we not somehow merge the |
|
You're right. I was only thinking about threas-safety, but these run on different processes. Of the top of my head I'm not clear how to do the communication. OTOH, I don't need all the redirect pages in my local docs, so I could as well add a switch that off. CI is single-process anyway. |
|
I think env-merge-info (https://www.sphinx-doc.org/en/master/extdev/appapi.html#sphinx-core-events) is where to merge the different readers' info (which likely means |
|
Untested, but I think you need something like diff --git i/doc/sphinxext/redirect_from.py w/doc/sphinxext/redirect_from.py
index d8aa487b0..f480ced3b 100644
--- i/doc/sphinxext/redirect_from.py
+++ w/doc/sphinxext/redirect_from.py
@@ -48,12 +48,14 @@ HTML_TEMPLATE = """<html>
def setup(app):
RedirectFrom.app = app
app.add_directive("redirect-from", RedirectFrom)
+ app.connect("env-before-read-docs", _setup_redirects)
+ app.connect("env-merge-info", _merge_redirects)
app.connect("build-finished", _generate_redirects)
+ return {"parallel_read_safe": True}
class RedirectFrom(Directive):
required_arguments = 1
- redirects = {}
def run(self):
redirected_doc, = self.arguments
@@ -61,20 +63,32 @@ class RedirectFrom(Directive):
builder = self.app.builder
current_doc = env.path2doc(self.state.document.current_source)
redirected_reldoc, _ = env.relfn2path(redirected_doc, current_doc)
- if redirected_reldoc in self.redirects:
+ if redirected_reldoc in env._redirects:
raise ValueError(
f"{redirected_reldoc} is already noted as redirecting to "
- f"{self.redirects[redirected_reldoc]}")
- self.redirects[redirected_reldoc] = builder.get_relative_uri(
+ f"{env._redirects[redirected_reldoc]}")
+ env._redirects[redirected_reldoc] = builder.get_relative_uri(
redirected_reldoc, current_doc)
return []
+def _setup_redirects(app, env, docnames):
+ if not hasattr(env, "_redirects"):
+ env._redirects = {}
+
+
+def _merge_redirects(app, env, docnames, other):
+ for src, dst in other.items():
+ if src in env._redirects:
+ raise ValueError(f"{src} is already noted as redirecting to {dst}")
+ env._redirects[src] = dst
+
+
def _generate_redirects(app, exception):
builder = app.builder
if builder.name != "html" or exception:
return
- for k, v in RedirectFrom.redirects.items():
+ for k, v in app.env._redirects.items():
p = Path(app.outdir, k + builder.out_suffix)
if p.is_file():
logger.warning(f'A redirect-from directive is trying to create ' |
646053a to
bf93529
Compare
|
It runs, but I still have to check, that it produces all the correct redirects. |
bf93529 to
2900fda
Compare
|
The above proposal adds a private attribute to the sphinx env, which feels very wrong. While that's used in the sphinx example documentation, it's not recommended. New approach: Use a domain as data store for the redirects mapping. |
This uses a domain as data store for the redirects sphinx plugin. That is how the sphinx todo extension handles data and was suggested in sphinx-doc/sphinx#9003 (comment) Limit documenting special members to __call__ Closes matplotlib#20080. Follow up to matplotlib#17151.
2900fda to
4d248b2
Compare
…686-on-v3.4.x Backport PR #19686 on branch v3.4.x (Declare sphinxext.redirect_from parallel_read_safe)
|
When building 3.4.2 docs, no redirects were produced, so there might some bug here. |
|
As pointed out above and in #21876, this doesn't actually produce redirects... |
|
Ok, let's revert this for now. Local built times have already increased to a level that I'm mostly not trying to build locally anymore :(. And CI doesn't use parallel builds AFAIK. @jklymak feel free to revert, I don't have Repo access right now. The local build time needs special investigation. Probably one can just deactivate redirects there. |
|
(OT: For what ever reason parallel builds are far slower for me than non-parallel!) |
PR Summary
The new
redirect_fromplugin prevented sphinx from running in parallel and changed the doc build performance from awfully slow to insanely slow.This change improves the performances again to awfully slow 😄 . It's only relevant for people who arebuilding the docs in parallel (
make html -j4).