Skip to content

feat: Output the doctrees directory #2877

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kaycebasques
Copy link

Allows non-hermetic docs builds (e.g. bazelisk run //docs:docs.run) to leverage Sphinx's stateful incremental build system. The key is to expose the doctrees output artifact alongside the built docs, and then use the --doctree-dir flag to notify Sphinx of the existence of the incremental build metadata. This also requires removing the --fresh-env and --write-all. IMO rules_python should not be forcing those options on all users anyways. When docs are built hermetically, Sphinx has to build everything from scratch anyways. There's no need to explicitly set the flags. Users can set those flags downstream in their own projects, via the extra_opts list in their sphinx_docs() target.

@kaycebasques
Copy link
Author

@rickeylev

@rickeylev
Copy link
Collaborator

The docs say https://www.sphinx-doc.org/en/master/man/sphinx-build.html#cmdoption-sphinx-build-d

Normally, these files are put in a directory called .doctrees under the build directory

So I would expect the {name}_build output directory to have the .doctrees directory within it. Are you seeing otherwise? Is it because fresh-env/write-all only apply to the build output directory, and the doctrees, if put elsewhere, are able to persist despite those being set?

I do like that it says the output can be shared between builders -- I wonder if that could be leveraged in a future change.

Remove fresh-env, write-all

I don't recall why I added those. I can't remember if it was just defensive programming, or if ran across a real issue. Does --no-fresh-env or --fresh-env=false work? If so, you could try adding that to extra args. If not (annoying), then we should add a remove_opts attribute to deal with those.

Removing fresh-env is possible if Sphinx is using hash-based change detection. If it's using timestamp detection, then it'll act weird because bazel won't reliably set timestamps on files. I don't see a mention in the docs about what strategy it uses, though. I wonder if a plugin could address this if its using timestamps.

Removing write-all: This might be necessary because (i'm pretty sure) bazel clears the output directory every invocation, but if sphinx thinks a file doesn't need to be written (fresh-env=false), it won't write it. And then the output dir is missing files.

@kaycebasques
Copy link
Author

kaycebasques commented May 14, 2025

Re: the location of the .doctrees dir, I believe it's a sibling of each builder's output dir, not a child within a specific builder's output dir. E.g. you get _build/html and _build/.doctrees, not _build/html/.doctrees. Let me double-check when I get into the office.

If the .doctrees dir is shared among all builders then that location makes sense to me.

@rickeylev
Copy link
Collaborator

what change detection does fresh env use

Timestamp based, unfortunately :( sphinx-doc/sphinx#11556 (comment)

I filed #2879 as an idea to implement a plugin to do content-based change detection.

@kaycebasques
Copy link
Author

Current status is that @rickeylev is helping me investigate whether we can use persistent workers (and a few other things) to get incremental builds working within Bazel's hermetic worldview. Let's leave this PR open for a bit but it may end up getting closed without merge.

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

Successfully merging this pull request may close these issues.

2 participants