Skip to content

Added hermetic Graphviz support for Sphinx - #273

Merged
hoe-jo merged 1 commit into
eclipse-score:mainfrom
AAmbuj:amsh_fix_graphviz_use_download_utils_patch
Jun 22, 2026
Merged

hoe-jo merged 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 7 times, most recently from 2d4f52d to 703abed Compare June 18, 2026 10:21
-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 703abed to 88d2c01 Compare June 22, 2026 11:40
@hoe-jo
hoe-jo merged commit 88418e5 into eclipse-score:main Jun 22, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants