Added hermetic Graphviz support for Sphinx#273
Open
AAmbuj wants to merge 1 commit into
Open
Conversation
5934076 to
cc1711a
Compare
Contributor
Author
|
for upstream fix i raised a PR also but pipeline is falling and its showing time out failure. and reviewer also not responded yet. PR: https://gitlab.arm.com/bazel/download_utils/-/merge_requests/23 This PR got merge now updated the PR with upstream changes. |
a44ce41 to
515baaa
Compare
hoe-jo
reviewed
Jun 18, 2026
hoe-jo
left a comment
Contributor
There was a problem hiding this comment.
- Remove the hermeticity hole. In sphinx_module.bzl:267 drop use_default_shell_env = True from the score_html action. The three Graphviz vars are already passed explicitly via env. Then rebuild docs to confirm nothing else (PlantUML/Java) silently relied on host env; if something does, re-introduce it narrowly via env_inherit on the rule with a documented reason rather than the blanket flag.
- Robust execroot resolution. In conf.template.py:39-48 replace the bazel-out string-split (parts[0]) with os.getcwd() captured once at module import (cwd == execroot at action start, before Sphinx chdirs). Keep a Path.cwd() fallback for IDE/non-Bazel runs. This removes the fragile layout assumption while preserving the existing _resolve_execroot_path() consumers.
- Fail instead of reaching for host dot. In conf.template.py:218
- Platform guard. The .deb is a glibc/x86_64-specific compiled artifact. Make the graphviz attr default in sphinx_module.bzl:356 resolve via a select() to @graphviz_deb//:all on @platforms//os:linux + @platforms//cpu:x86_64 and to [] otherwise
- Graphviz render smoke test. Add a minimal sphinx_module fixture under rules_score docs/test area containing a .. graphviz::/digraph directive, plus a build test asserting the produced HTML contains a rendered .svg. Wire it into BUILD following the existing rules_score_doc pattern.
deebaac to
2d4f52d
Compare
-remove use_default_shell_env from HTML action -enforce hermetic graphviz path and library wiring -avoid host dot fallback when graphviz is not configured -add Linux x86_64 platform guard for default graphviz artifact -add graphviz fixture and smoke test for rendered SVG output"
2d4f52d to
703abed
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
-remove use_default_shell_env from HTML action
-enforce hermetic graphviz path and library wiring
-avoid host dot fallback when graphviz is not configured
-add Linux x86_64 platform guard for default graphviz artifact
-add graphviz fixture and smoke test for rendered SVG output"