fix(sphinxdocs): invalidate cache on branch change#3917
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the Sphinx persistent worker's resilience and correctness during incremental builds and branch switches. It introduces logic to clean up stale source files, clear corrupted doctree caches on failure, and properly invalidate parent documents when referenced files are added or removed. Additionally, a comprehensive integration test suite is added to verify these behaviors across git checkouts. The feedback suggests wrapping the file removal logic in a robust, cross-platform helper to prevent worker crashes, and optimizing the toctree membership check using isdisjoint for better performance.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
kaycebasques
force-pushed
the
git_test
branch
7 times, most recently
from
75d34d0 to
4197ec2
Compare
Per repository policy, new files should not include Bazel copyright headers. Removed headers from newly added test files.
|
temporary artifact ( |
Delete temporary build.log artifact from PR. Move persistent worker integration test to top-level integration_tests/ directory. Replace Git commands in persistent worker test with direct file modifications and rules_bazel_integration_test (BIT). Update rules_go override and distribution filegroups for robust sandboxed testing.
Refine integration test setup and configuration based on review feedback. Added .bazelignore for bazel-* outputs, upgraded rules_bazel_integration_test to 0.37.1, configured local 'self' bazel testing via bazel_from_env wrapper, replaced strict_action_env override with action_env/test_env flags, and refined index.html check path.
Put deleted_packages on separate lines in .bazelrc.deleted_packages, remove redundant fast-tests tag filters from nested workspace .bazelrc, and add explanatory comments for cache disabling flags.
rickeylev
left a comment
rickeylev
left a comment
There was a problem hiding this comment.
I've refactor a bunch to address my comments
- Switched it to rules_bazle_integration_test
- removed using git
etc
Replace wildcard pattern in .bazelignore with explicit bazel-* directory paths (bazel-bin, bazel-out, bazel-testlogs, bazel-sphinxdocs) to avoid InvalidPathException across platforms.
2 participants
We had to disable the
sphinxdocspersistent worker in Pigweed (https://pwbug.dev/493749800) because the cache invalidation logic seemed incomplete with regards to Git branches. E.g. in branchfeatyou create a new docfoo.rst. You switch back to branchmainand try to build Sphinx, but Sphinx warns about needing to ignore unreadable documentfoo.rst. Because Pigweed builds Sphinx with--fail-on-warningthis would break our docs build. This PR adds the bug fix and also starts a suite of tests that will be focused on persistent worker correctness.