Skip to content

Added hermetic Graphviz support for Sphinx#273

Open
AAmbuj wants to merge 1 commit into
eclipse-score:mainfrom
AAmbuj:amsh_fix_graphviz_use_download_utils_patch
Open

Added hermetic Graphviz support for Sphinx#273
AAmbuj wants to merge 1 commit into
eclipse-score:mainfrom
AAmbuj:amsh_fix_graphviz_use_download_utils_patch

Conversation

@AAmbuj

@AAmbuj AAmbuj commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

-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"

@AAmbuj AAmbuj marked this pull request as draft June 15, 2026 09:12
@AAmbuj AAmbuj force-pushed the amsh_fix_graphviz_use_download_utils_patch branch 2 times, most recently from 5934076 to cc1711a Compare June 15, 2026 10:06
@AAmbuj

AAmbuj commented Jun 17, 2026

Copy link
Copy Markdown
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.

@AAmbuj AAmbuj force-pushed the amsh_fix_graphviz_use_download_utils_patch branch 4 times, most recently from a44ce41 to 515baaa Compare June 18, 2026 04:24
@AAmbuj AAmbuj marked this pull request as ready for review June 18, 2026 04:26

@hoe-jo hoe-jo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 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.
  2. 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.
  3. Fail instead of reaching for host dot. In conf.template.py:218
  4. 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
  5. 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.

@AAmbuj AAmbuj force-pushed the amsh_fix_graphviz_use_download_utils_patch branch 6 times, most recently from deebaac to 2d4f52d Compare June 18, 2026 09:59
-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"
@AAmbuj AAmbuj force-pushed the amsh_fix_graphviz_use_download_utils_patch branch from 2d4f52d to 703abed Compare June 18, 2026 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants