From 3c68ae0b0671f5a15b924440c8d310663994aaaf Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Wed, 16 Sep 2026 21:27:43 +0200 Subject: [PATCH 1/8] refactor: pass sourcelinks, mounts, and metamodel labels as env vars in the Needs build action The sandboxed Needs build action received these three Bazel-generated files as pre-formatted `--define=...` strings baked into `SPHINX_EXTRA_OPTS`, while the interactive `bazel run` targets already passed the metamodel and mounts manifest as plain environment variables consumed by cli.py. Declare them as typed label attributes on `sphinx_docs` instead, so the action can declare them as sandbox inputs and expose their execroot paths through the same env vars (`SCORE_SOURCELINKS`, `MOUNTS_MANIFEST`, `SCORE_METAMODEL_YAML`) already used elsewhere, removing the duplicated per-caller `$(location ...)` string formatting. --- bzl/needs_rules.bzl | 36 +++++++++++++++++++++------- docs.bzl | 57 ++++++++++++++++++++++++--------------------- 2 files changed, 58 insertions(+), 35 deletions(-) diff --git a/bzl/needs_rules.bzl b/bzl/needs_rules.bzl index 1c6fbd045..dbcca4b33 100644 --- a/bzl/needs_rules.bzl +++ b/bzl/needs_rules.bzl @@ -32,9 +32,10 @@ def _sphinx_docs_impl(ctx): if not bundle.own_source_files.to_list(): fail("Sphinx requires a bundle with direct documentation sources") - # Expand file labels at analysis time, then encode the argument list as - # JSON so spaces, quotes and '=' in Sphinx options survive the environment - # transport unchanged. The launcher adds these after its default options. + # File labels provide execroot-relative paths for this action's sandbox. + # Pass them through the environment variables already consumed by the CLI + # and extensions; reserve the JSON option list for non-path Sphinx overrides. + # Encode that list as JSON so spaces, quotes and '=' survive transport. # ``config`` is transported separately because the launcher derives # Sphinx's ``-c`` directory from its path; it is not just another data file. env = { @@ -43,10 +44,16 @@ def _sphinx_docs_impl(ctx): "OUTPUT_DIRECTORY": output.path, "SPHINX_CONFIG_FILE": ctx.file.config.path, "DATA": "[]", - "SPHINX_EXTRA_OPTS": json.encode([ - ctx.expand_location(option, targets = ctx.attr.tools) - for option in ctx.attr.extra_opts - ]), + "SCORE_SOURCELINKS": ( + ctx.file.score_sourcelinks_json.path if ctx.file.score_sourcelinks_json else "" + ), + "MOUNTS_MANIFEST": ( + ctx.file.mounts_manifest.path if ctx.file.mounts_manifest else "" + ), + "SCORE_METAMODEL_YAML": ( + ctx.file.score_metamodel_yaml.path if ctx.file.score_metamodel_yaml else "" + ), + "SPHINX_EXTRA_OPTS": json.encode(ctx.attr.extra_opts), } # Data and mounted sources must be present at their execution-root paths. @@ -56,7 +63,15 @@ def _sphinx_docs_impl(ctx): executable = ctx.executable.sphinx, env = env, inputs = depset( - [ctx.file.config] + ctx.files.data + ctx.files.tools, + [ctx.file.config] + ctx.files.data + ctx.files.tools + [ + file + for file in [ + ctx.file.score_sourcelinks_json, + ctx.file.mounts_manifest, + ctx.file.score_metamodel_yaml, + ] + if file + ], transitive = [bundle.own_source_files], ), outputs = [output], @@ -73,6 +88,11 @@ sphinx_docs = rule( "bundle": attr.label(providers = [DocsBundleInfo], mandatory = True), "data": attr.label_list(allow_files = True), "tools": attr.label_list(allow_files = True), + # Typed labels let the action pass their execroot paths through the + # environment contract above and still declare sandbox inputs. + "score_sourcelinks_json": attr.label(allow_single_file = True), + "mounts_manifest": attr.label(allow_single_file = True), + "score_metamodel_yaml": attr.label(allow_single_file = True), "extra_opts": attr.string_list(), # The launcher runs on the build host and carries extension runfiles. "sphinx": attr.label(cfg = "exec", executable = True, mandatory = True), diff --git a/docs.bzl b/docs.bzl index dd07da615..315d9850c 100644 --- a/docs.bzl +++ b/docs.bzl @@ -81,10 +81,7 @@ def _needs_sphinx_extra_opts( master_doc, external_needs_source, score_bundle_needs_export, - score_sourcelinks_json, - score_source_code_linker_plain_links, - mounts_manifest, - score_metamodel_yaml): + score_source_code_linker_plain_links): """Return per-target Sphinx configuration defines for a Needs build.""" # The launcher supplies diagnostics shared by every builder. Keep only # target-specific defines here so the action does not receive duplicate @@ -95,10 +92,7 @@ def _needs_sphinx_extra_opts( ("master_doc", master_doc), ("external_needs_source", external_needs_source), ("score_bundle_needs_export", score_bundle_needs_export), - ("score_sourcelinks_json", score_sourcelinks_json), ("score_source_code_linker_plain_links", score_source_code_linker_plain_links), - ("mounts_manifest", mounts_manifest), - ("score_metamodel_yaml", score_metamodel_yaml), ] for option in _sphinx_define(name, value) ] @@ -135,6 +129,14 @@ def _needs_sphinx_docs( sphinx_build_data = [], visibility = None): """Declare a bundle Needs export with the repository-wide Sphinx policy.""" + # These three are consumed as their own typed rule attributes (below), not + # as ordinary tools; still list them here so the caller does not have to + # repeat them when building its own ``tools`` list. + tools = tools + [ + label + for label in [score_sourcelinks_json, mounts_manifest, score_metamodel_yaml] + if label + ] sphinx_build = _declare_sphinx_build_binary( name, sphinx_build_data + [tool for tool in tools if tool not in sphinx_build_data], @@ -152,11 +154,14 @@ def _needs_sphinx_docs( master_doc, external_needs_source, score_bundle_needs_export, - score_sourcelinks_json, score_source_code_linker_plain_links, - mounts_manifest, - score_metamodel_yaml, ), + # Keep these as labels rather than path strings in ``extra_opts``. The + # private rule declares them as action inputs and provides execroot + # paths directly through the environment. + score_sourcelinks_json = score_sourcelinks_json, + mounts_manifest = mounts_manifest, + score_metamodel_yaml = score_metamodel_yaml, sphinx = sphinx_build, tools = tools, visibility = visibility, @@ -354,6 +359,9 @@ def _declare_bundle_local_needs( sphinx_build_deps = _sphinx_runtime_deps(deps) needs_local = _bundle_internal_target(name, "needs_local") + # The generated source-links target stays typed as a label here; the + # private Needs rule owns translating it to an action environment path + # and declaring it as an input. _needs_sphinx_docs( name = needs_local, bundle = ":" + name, @@ -363,9 +371,8 @@ def _declare_bundle_local_needs( master_doc = entry_doc, external_needs_source = "[]", score_bundle_needs_export = "1", - score_sourcelinks_json = "$(location " + str(sourcelinks_json) + ")" if sourcelinks_json else None, + score_sourcelinks_json = sourcelinks_json, score_source_code_linker_plain_links = "1", - tools = [sourcelinks_json] if sourcelinks_json else [], visibility = visibility, ) @@ -547,20 +554,18 @@ def docs( # list-valued attributes such as ``data`` and ``tools``. metamodel_label = [metamodel] if metamodel else [] - mounts_manifest_label = [] + mounts_manifest = None if bundles: mounts_bundle = create_bundle( name = "_docs_mounts", bundles = bundles, visibility = ["//visibility:private"], ) - - mounts_manifest_label = [ - create_mounts_manifest( - name = "_mounts_manifest", - bundle = mounts_bundle, - ), - ] + mounts_manifest = create_mounts_manifest( + name = "_mounts_manifest", + bundle = mounts_bundle, + ) + mounts_manifest_label = [mounts_manifest] if mounts_manifest else [] deps = _sphinx_deps(deps) deps = deps + [ @@ -626,7 +631,7 @@ def docs( "EXTERNAL_NEEDS_FILES": str(external_needs), # `bazel run` starts from a runfiles tree, so this logical path is # resolved by score_mounts through ``RUNFILES_DIR``. - "MOUNTS_MANIFEST": "$(rlocationpath :_mounts_manifest)" if bundles else "", + "MOUNTS_MANIFEST": "$(rlocationpath :_mounts_manifest)" if mounts_manifest else "", "SCORE_SOURCELINKS": "$(location :sourcelinks_json)", } if config_is_generated: @@ -697,13 +702,11 @@ def docs( sphinx_build_deps = deps, sphinx_build_data = data + external_needs + metamodel_label + [":docs_bundle"], external_needs_source = str(data + external_needs), - score_sourcelinks_json = "$(location :sourcelinks_json)", + score_sourcelinks_json = ":sourcelinks_json", score_source_code_linker_plain_links = "1", - # The build action runs in a sandbox, so it needs the action-input path - # rather than the runfiles-relative spelling. - mounts_manifest = "$(location :_mounts_manifest)" if bundles else None, - score_metamodel_yaml = "$(location " + str(metamodel) + ")" if metamodel else None, - tools = external_needs + metamodel_label + [":sourcelinks_json", ":docs_bundle"] + mounts_manifest_label, + mounts_manifest = mounts_manifest, + score_metamodel_yaml = metamodel, + tools = external_needs + [":docs_bundle"], visibility = ["//visibility:public"], ) From 1b952169309511f2e3199bff038ced2388330e01 Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Wed, 16 Sep 2026 21:46:38 +0200 Subject: [PATCH 2/8] refactor: resolve sourcelinks and mounts manifest to Sphinx defines only in cli.py Extensions read these Bazel-provided paths through two parallel channels: a direct os.environ lookup and the app.config value cli.py populates via --define. The env-var path bypassed cli.py's runfiles resolution entirely, so it only worked by coincidence for values that happened to already be cwd-relative. Make cli.py the single place that resolves SCORE_SOURCELINKS (like it already did for SCORE_METAMODEL_YAML, now shared through _resolve_runfiles_relative_path) into a --define, and drop the now-redundant direct env reads in score_source_code_linker, score_mounts, and score_cross_module_compatibility. mounts_manifest's env fallback was already dead code, since cli.py has unconditionally defined it for every invocation; sourcelinks_json's was not, so docs.bzl's interactive SCORE_SOURCELINKS now passes an rlocationpath like its metamodel/mounts siblings, resolved correctly by cli.py instead of read as a bare path by the extension. --- docs.bzl | 2 +- src/docs_cli/cli.py | 36 +++++++++++-------- .../__init__.py | 5 ++- src/extensions/score_mounts/__init__.py | 14 ++++---- .../score_source_code_linker/__init__.py | 18 +++++----- .../tests/test_codelink.py | 20 ++++++----- .../test_repo_source_link_integration.py | 10 +++--- .../test_source_code_link_integration.py | 14 ++++---- 8 files changed, 64 insertions(+), 55 deletions(-) diff --git a/docs.bzl b/docs.bzl index 315d9850c..817370c97 100644 --- a/docs.bzl +++ b/docs.bzl @@ -632,7 +632,7 @@ def docs( # `bazel run` starts from a runfiles tree, so this logical path is # resolved by score_mounts through ``RUNFILES_DIR``. "MOUNTS_MANIFEST": "$(rlocationpath :_mounts_manifest)" if mounts_manifest else "", - "SCORE_SOURCELINKS": "$(location :sourcelinks_json)", + "SCORE_SOURCELINKS": "$(rlocationpath :sourcelinks_json)", } if config_is_generated: # The generated file is named conf.py. Run targets pass its containing diff --git a/src/docs_cli/cli.py b/src/docs_cli/cli.py index b0a69cb43..052abcfba 100644 --- a/src/docs_cli/cli.py +++ b/src/docs_cli/cli.py @@ -138,6 +138,21 @@ def add_watch_dir(path: Path) -> None: return watch_dirs +def _resolve_runfiles_relative_path(config: DocsCliConfig, value: Path) -> Path: + """Resolve a Bazel-provided path that may be runfiles-relative. + + Build actions and direct calls already receive an absolute or + execroot/cwd-relative path. ``bazel run`` instead passes an + rlocationpath, which must be joined with the launcher's own runfiles + directory before use. + """ + if not config.is_bazel_build and not value.is_absolute(): + runfiles_dir = env.optional_path("RUNFILES_DIR") + ws_root = config.ws_root or Path() + value = runfiles_dir / value if runfiles_dir is not None else ws_root / value + return value.absolute() + + def sphinx_arguments( config: DocsCliConfig, ) -> list[str]: @@ -191,22 +206,15 @@ def sphinx_arguments( base_arguments.extend(["-c", str(config_file.parent)]) if metamodel_yaml := env.optional_path("SCORE_METAMODEL_YAML"): - # Under ``bazel run``, this environment variable is runfiles-relative - # and must be resolved through RUNFILES_DIR. A sandboxed Needs action - # instead expands the metamodel label to an execution-root path in - # SPHINX_EXTRA_OPTS; applying runfiles lookup there would escape the - # action's declared inputs. - if not config.is_bazel_build and not metamodel_yaml.is_absolute(): - runfiles_dir = env.optional_path("RUNFILES_DIR") - ws_root = config.ws_root or Path() - metamodel_yaml = ( - runfiles_dir / metamodel_yaml - if runfiles_dir is not None - else ws_root / metamodel_yaml - ) - metamodel_yaml = metamodel_yaml.absolute() + metamodel_yaml = _resolve_runfiles_relative_path(config, metamodel_yaml) base_arguments.append(f"--define=score_metamodel_yaml={metamodel_yaml}") + if sourcelinks_json := env.optional_path("SCORE_SOURCELINKS"): + # The sandboxed Needs action and ``bazel run`` both set this env var; + # only the extension reads ``app.config.score_sourcelinks_json``. + sourcelinks_json = _resolve_runfiles_relative_path(config, sourcelinks_json) + base_arguments.append(f"--define=score_sourcelinks_json={sourcelinks_json}") + if github_repository := env.get("GITHUB_REPOSITORY", ""): # GITHUB_REPOSITORY is expected as "owner/repo"; partition("/") splits # once into (owner, separator, repo), so we can ignore the separator. diff --git a/src/extensions/score_cross_module_compatibility/__init__.py b/src/extensions/score_cross_module_compatibility/__init__.py index 96c6fb075..31ad1c6dd 100644 --- a/src/extensions/score_cross_module_compatibility/__init__.py +++ b/src/extensions/score_cross_module_compatibility/__init__.py @@ -22,11 +22,10 @@ from sphinx.util import logging from sphinx_needs.need_item import NeedItem -from src.helper_lib import Environment, find_ws_root, get_runfiles_dir +from src.helper_lib import find_ws_root, get_runfiles_dir _VERSION_CONDITION = re.compile(r"^\s*version\s*==\s*(\d+)\s*$") logger = logging.getLogger(__name__) -env = Environment() MANDATORY_ATTRIBUTE = "mandatory-attribute" MANDATORY_LINK = "mandatory-link" @@ -220,7 +219,7 @@ def write(self, outdir: str | Path) -> None: def _manifest_path(app: Sphinx) -> Path | None: - raw = getattr(app.config, "mounts_manifest", "") or env.get("MOUNTS_MANIFEST", "") + raw = getattr(app.config, "mounts_manifest", "") if not isinstance(raw, str) or not raw.strip(): return None direct = Path(raw) diff --git a/src/extensions/score_mounts/__init__.py b/src/extensions/score_mounts/__init__.py index 72b1de106..fc211bdc1 100644 --- a/src/extensions/score_mounts/__init__.py +++ b/src/extensions/score_mounts/__init__.py @@ -46,9 +46,7 @@ resolve_source_files, resolve_walk_dir, ) -from src.helper_lib import Environment, find_ws_root, get_runfiles_dir - -env = Environment() +from src.helper_lib import find_ws_root, get_runfiles_dir logger = logging.getLogger(__name__) @@ -56,14 +54,14 @@ def _read_manifest(config: Config): """Locate and load the mounts manifest, or return ``None`` when unset. - The manifest path is passed by Bazel either via the ``mounts_manifest`` config - value or the ``MOUNTS`` env var. Its interpretation depends on the build + ``mounts_manifest`` is set by the docs CLI via ``--define`` from Bazel's + ``MOUNTS_MANIFEST`` env var. Its interpretation depends on the build context: under ``bazel run`` it is a runfiles-relative path (``$(rlocationpath)``) resolved against the runfiles dir; in a sandbox build - it is relative to the exec root (``$(location)``). Resolving the path here - keeps that context branch out of the pure ``_resolver`` module. + it is already relative to the exec root. Resolving the path here keeps + that context branch out of the pure ``_resolver`` module. """ - raw = getattr(config, "mounts_manifest", None) or env.get("MOUNTS_MANIFEST", "") + raw = getattr(config, "mounts_manifest", "") if not raw or not raw.strip() or not isinstance(raw, str): return None diff --git a/src/extensions/score_source_code_linker/__init__.py b/src/extensions/score_source_code_linker/__init__.py index 6cab5c507..bc0e18800 100644 --- a/src/extensions/score_source_code_linker/__init__.py +++ b/src/extensions/score_source_code_linker/__init__.py @@ -59,9 +59,7 @@ construct_and_add_need, run_xml_parser, ) -from src.helper_lib import Environment, find_ws_root - -env = Environment() +from src.helper_lib import find_ws_root LOGGER = get_logger(__name__) # Uncomment this to enable more verbose logging @@ -87,11 +85,11 @@ def build_and_save_combined_file(outdir: Path, app: Sphinx | None = None): Reads the saved partial caches of codelink & testlink Builds the combined JSON cache & saves it """ - source_code_links_path = env.get("SCORE_SOURCELINKS", "") - if not source_code_links_path and app is not None: - source_code_links_path = str( - getattr(app.config, "score_sourcelinks_json", "") or "" - ).strip() + source_code_links_path = ( + str(getattr(app.config, "score_sourcelinks_json", "") or "").strip() + if app is not None + else "" + ) if source_code_links_path: source_code_links_json = Path(source_code_links_path) try: @@ -99,8 +97,8 @@ def build_and_save_combined_file(outdir: Path, app: Sphinx | None = None): except FileNotFoundError as exc: raise FileNotFoundError( "Pre-generated source-code links file does not exist: " - f"{source_code_links_json}. Check SCORE_SOURCELINKS or " - "score_sourcelinks_json." + f"{source_code_links_json}. Check the score_sourcelinks_json " + "Sphinx config value (set from SCORE_SOURCELINKS by the docs CLI)." ) from exc except AssertionError: source_code_links = load_source_code_links_with_metadata_json( diff --git a/src/extensions/score_source_code_linker/tests/test_codelink.py b/src/extensions/score_source_code_linker/tests/test_codelink.py index 8b3d9bf61..1d89fa88b 100644 --- a/src/extensions/score_source_code_linker/tests/test_codelink.py +++ b/src/extensions/score_source_code_linker/tests/test_codelink.py @@ -21,9 +21,11 @@ from collections.abc import Generator from dataclasses import asdict from pathlib import Path -from typing import Any +from types import SimpleNamespace +from typing import Any, cast import pytest +from sphinx.application import Sphinx # S-CORE plugin to allow for properties/attributes in xml # Enables Testlinking @@ -359,11 +361,9 @@ def test_cache_file_operations( def test_combining_without_source_links_continues_with_empty_code_links( - temp_dir: Path, monkeypatch: pytest.MonkeyPatch + temp_dir: Path, ) -> None: - """A build without a pre-generated source-link input must not scan or fail.""" - monkeypatch.delenv("SCORE_SOURCELINKS", raising=False) - + """A build without a configured source-link input must not scan or fail.""" build_and_save_combined_file(temp_dir) grouped_cache = temp_dir / "score_scl_grouped_cache.json" @@ -371,18 +371,20 @@ def test_combining_without_source_links_continues_with_empty_code_links( def test_combining_with_missing_source_links_reports_configured_path( - temp_dir: Path, monkeypatch: pytest.MonkeyPatch + temp_dir: Path, ) -> None: """Report the configured source-link file when it cannot be found.""" missing_file = temp_dir / "missing_source_links.json" - monkeypatch.setenv("SCORE_SOURCELINKS", str(missing_file)) + fake_config = SimpleNamespace(score_sourcelinks_json=str(missing_file)) + app = cast(Sphinx, SimpleNamespace(config=fake_config)) with pytest.raises(FileNotFoundError) as exc_info: - build_and_save_combined_file(temp_dir) + build_and_save_combined_file(temp_dir, app) assert str(exc_info.value) == ( "Pre-generated source-code links file does not exist: " - f"{missing_file}. Check SCORE_SOURCELINKS or score_sourcelinks_json." + f"{missing_file}. Check the score_sourcelinks_json Sphinx config value " + "(set from SCORE_SOURCELINKS by the docs CLI)." ) diff --git a/src/extensions/score_source_code_linker/tests/test_repo_source_link_integration.py b/src/extensions/score_source_code_linker/tests/test_repo_source_link_integration.py index 80f2e9818..cabc7418c 100644 --- a/src/extensions/score_source_code_linker/tests/test_repo_source_link_integration.py +++ b/src/extensions/score_source_code_linker/tests/test_repo_source_link_integration.py @@ -265,10 +265,6 @@ def sphinx_app_setup( git_repo_setup: Path, monkeypatch: pytest.MonkeyPatch, ) -> Callable[[], SphinxTestApp]: - # Source links are generated before Sphinx starts, matching the Bazel build - # contract used by the extension in production. - monkeypatch.setenv("SCORE_SOURCELINKS", str(sphinx_base_dir / "source_links.json")) - def _create_app(): base_dir = sphinx_base_dir docs_dir = base_dir / "docs" @@ -283,6 +279,12 @@ def _create_app(): outdir=sphinx_base_dir / "out", buildername="html", warningiserror=True, + # Source links are generated before Sphinx starts; the docs CLI + # passes the resolved path via ``--define``, matching the Bazel + # build contract used by the extension in production. + confoverrides={ + "score_sourcelinks_json": str(sphinx_base_dir / "source_links.json"), + }, ) return _create_app diff --git a/src/extensions/score_source_code_linker/tests/test_source_code_link_integration.py b/src/extensions/score_source_code_linker/tests/test_source_code_link_integration.py index d2f000fc3..de7383f62 100644 --- a/src/extensions/score_source_code_linker/tests/test_source_code_link_integration.py +++ b/src/extensions/score_source_code_linker/tests/test_source_code_link_integration.py @@ -213,12 +213,6 @@ def sphinx_app_setup( git_repo_setup: Path, monkeypatch: pytest.MonkeyPatch, ) -> Callable[[], SphinxTestApp]: - # Source links are generated before Sphinx starts, matching the Bazel build - # contract used by the extension in production. - monkeypatch.setenv( - "SCORE_SOURCELINKS", str(sphinx_base_dir / ".expected_codelink.json") - ) - def _create_app(): base_dir = sphinx_base_dir docs_dir = base_dir / "docs" @@ -233,6 +227,14 @@ def _create_app(): outdir=sphinx_base_dir / "out", buildername="html", warningiserror=True, + # Source links are generated before Sphinx starts; the docs CLI + # passes the resolved path via ``--define``, matching the Bazel + # build contract used by the extension in production. + confoverrides={ + "score_sourcelinks_json": str( + sphinx_base_dir / ".expected_codelink.json" + ), + }, ) return _create_app From 5b3c43b1ab6450a91cfc348ee0fff48a9ce512d9 Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Wed, 16 Sep 2026 22:10:56 +0200 Subject: [PATCH 3/8] docs: assert cli.py never runs in ExecutionEnvironment.DIRECT cli.py is only ever invoked via `bazel run` or as the sandboxed Needs action's executable (see src/docs_cli/README.md); ExecutionEnvironment.DIRECT exists solely so DocsCliConfig/sphinx_arguments stay unit-testable without a real runfiles tree. Assert this invariant right after building the config so it's readable in the file instead of only discoverable by tracing callers. --- src/docs_cli/cli.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/docs_cli/cli.py b/src/docs_cli/cli.py index 052abcfba..4687976b6 100644 --- a/src/docs_cli/cli.py +++ b/src/docs_cli/cli.py @@ -285,6 +285,12 @@ def main(argv: list[str] | None = None) -> int: debugpy.wait_for_client() config = DocsCliConfig.from_environment(env) + # cli.py is only ever invoked via `bazel run` (a _declare_docs_binary + # target) or as the sandboxed Needs action's executable; see + # src/docs_cli/README.md. ExecutionEnvironment.DIRECT exists so + # DocsCliConfig/sphinx_arguments stay unit-testable without a real + # runfiles tree (see main_test.py) and should never occur here. + assert not config.is_direct, "cli.py must run via bazel run or a Bazel action" ws_root = config.ws_root or Path() package_dir = config.package_dir output_dir = config.output_dir From 733cc47277f4a46891b3237790162d1c384a291c Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Wed, 16 Sep 2026 22:11:26 +0200 Subject: [PATCH 4/8] refactor: require a real Sphinx app in build_and_save_combined_file app was optional and score_sourcelinks_json was read via getattr with a fallback, but the only production caller (setup_combined_linker) always passes a real app, and add_config_value guarantees the config value exists before any event handler can call this function. Make app mandatory and read the config value directly; update the one test that relied on the app=None branch to pass an explicit fake config instead. --- src/extensions/score_source_code_linker/__init__.py | 8 ++------ .../score_source_code_linker/tests/test_codelink.py | 4 +++- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/extensions/score_source_code_linker/__init__.py b/src/extensions/score_source_code_linker/__init__.py index bc0e18800..77a89c94d 100644 --- a/src/extensions/score_source_code_linker/__init__.py +++ b/src/extensions/score_source_code_linker/__init__.py @@ -80,16 +80,12 @@ def get_cache_filename(build_dir: Path, filename: str) -> Path: return build_dir / filename -def build_and_save_combined_file(outdir: Path, app: Sphinx | None = None): +def build_and_save_combined_file(outdir: Path, app: Sphinx): """ Reads the saved partial caches of codelink & testlink Builds the combined JSON cache & saves it """ - source_code_links_path = ( - str(getattr(app.config, "score_sourcelinks_json", "") or "").strip() - if app is not None - else "" - ) + source_code_links_path = app.config.score_sourcelinks_json.strip() if source_code_links_path: source_code_links_json = Path(source_code_links_path) try: diff --git a/src/extensions/score_source_code_linker/tests/test_codelink.py b/src/extensions/score_source_code_linker/tests/test_codelink.py index 1d89fa88b..d2bc67edd 100644 --- a/src/extensions/score_source_code_linker/tests/test_codelink.py +++ b/src/extensions/score_source_code_linker/tests/test_codelink.py @@ -364,7 +364,9 @@ def test_combining_without_source_links_continues_with_empty_code_links( temp_dir: Path, ) -> None: """A build without a configured source-link input must not scan or fail.""" - build_and_save_combined_file(temp_dir) + fake_config = SimpleNamespace(score_sourcelinks_json="") + app = cast(Sphinx, SimpleNamespace(config=fake_config)) + build_and_save_combined_file(temp_dir, app) grouped_cache = temp_dir / "score_scl_grouped_cache.json" assert json.loads(grouped_cache.read_text(encoding="utf-8")) == [] From d953b2b75d4ed3ae2145e507341428a3f211418a Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Wed, 16 Sep 2026 22:34:16 +0200 Subject: [PATCH 5/8] refactor: resolve KNOWN_GOOD_JSON to a Sphinx define only in cli.py xml_parser.py read env.optional_path("KNOWN_GOOD_JSON") directly, bypassing the app.config.KNOWN_GOOD_JSON value score_source_code_linker already registers and cli.py already emits via --define. That define carried an unresolved value, since cli.py never ran it through the runfiles resolution used for SCORE_METAMODEL_YAML/SCORE_SOURCELINKS; it only worked because docs.bzl passed $(location ...), which happens to be cwd-relative under bazel run. Resolve KNOWN_GOOD_JSON through the shared _resolve_runfiles_relative_path helper like its siblings, switch docs.bzl to $(rlocationpath ...), and thread the resolved path through xml_parser.py's call chain (build_test_needs_from_files -> read_test_xml_file -> get_metadata_from_test_path) instead of reading the env var deep inside a plain parsing function that has no Sphinx app access. --- docs.bzl | 2 +- src/docs_cli/cli.py | 1 + src/docs_cli/main_test.py | 2 +- .../tests/test_xml_parser.py | 32 ++++++++++--------- .../score_source_code_linker/xml_parser.py | 19 ++++++----- 5 files changed, 31 insertions(+), 25 deletions(-) diff --git a/docs.bzl b/docs.bzl index 817370c97..e5c8773be 100644 --- a/docs.bzl +++ b/docs.bzl @@ -644,7 +644,7 @@ def docs( docs_env["SCORE_METAMODEL_YAML"] = "$(rlocationpath " + str(metamodel) + ")" if known_good_label: known_good_str = str(known_good_label[0]) - docs_env["KNOWN_GOOD_JSON"] = "$(location " + known_good_str + ")" + docs_env["KNOWN_GOOD_JSON"] = "$(rlocationpath " + known_good_str + ")" docs_data += known_good_label # Generated documentation artifacts may live below ``docs/``. A diff --git a/src/docs_cli/cli.py b/src/docs_cli/cli.py index 4687976b6..834915cab 100644 --- a/src/docs_cli/cli.py +++ b/src/docs_cli/cli.py @@ -229,6 +229,7 @@ def sphinx_arguments( base_arguments.append(f"-A=doc_path={relative_doc_path}") if known_good_json := env.optional_path("KNOWN_GOOD_JSON"): + known_good_json = _resolve_runfiles_relative_path(config, known_good_json) base_arguments.append(f"--define=KNOWN_GOOD_JSON={known_good_json}") return base_arguments diff --git a/src/docs_cli/main_test.py b/src/docs_cli/main_test.py index a63a685c4..59e465e00 100644 --- a/src/docs_cli/main_test.py +++ b/src/docs_cli/main_test.py @@ -214,7 +214,7 @@ def test_bazel_configuration_resolves_runfiles_and_preserves_repo_relative_edit_ "-A=github_user=owner", "-A=github_repo=repo", "-A=doc_path=component/docs", - "--define=KNOWN_GOOD_JSON=baseline.json", + f"--define=KNOWN_GOOD_JSON={workspace}/runfiles/baseline.json", } # Every expected option is present; their relative order is irrelevant here. assert expected_arguments <= set(arguments) diff --git a/src/extensions/score_source_code_linker/tests/test_xml_parser.py b/src/extensions/score_source_code_linker/tests/test_xml_parser.py index bd1d1e5e5..86db75245 100644 --- a/src/extensions/score_source_code_linker/tests/test_xml_parser.py +++ b/src/extensions/score_source_code_linker/tests/test_xml_parser.py @@ -296,7 +296,9 @@ def test_read_test_xml_file( dir1: Path dir2: Path _, dir1, dir2, dir3, dir4 = tmp_xml_dirs() - needs1, no_props1, missing_props1 = xml_parser.read_test_xml_file(dir1 / "test.xml") + needs1, no_props1, missing_props1 = xml_parser.read_test_xml_file( + dir1 / "test.xml", None + ) # Should parse the properties and create a 'valid' testlink assert isinstance(needs1, list) and len(needs1) == 1 tcneed = needs1[0] @@ -311,7 +313,9 @@ def test_read_test_xml_file( assert missing_props1 == [] # No properties at all => Should not be a 'valid' testlink - needs2, no_props2, missing_props2 = xml_parser.read_test_xml_file(dir2 / "test.xml") + needs2, no_props2, missing_props2 = xml_parser.read_test_xml_file( + dir2 / "test.xml", None + ) assert isinstance(needs2, list) and len(needs2) == 1 tcneed2 = needs2[0] assert isinstance(tcneed2, DataOfTestCase) @@ -319,7 +323,9 @@ def test_read_test_xml_file( assert missing_props2 == [] # Extra Properties => Should not cause an error - needs3, no_props3, missing_props3 = xml_parser.read_test_xml_file(dir3 / "test.xml") + needs3, no_props3, missing_props3 = xml_parser.read_test_xml_file( + dir3 / "test.xml", None + ) assert isinstance(needs3, list) and len(needs3) == 1 tcneed3 = needs3[0] assert isinstance(tcneed3, DataOfTestCase) @@ -327,7 +333,9 @@ def test_read_test_xml_file( assert missing_props3 == [] # Missing some properties => Should not be a 'valid' testlink - needs4, no_props4, missing_props4 = xml_parser.read_test_xml_file(dir4 / "test.xml") + needs4, no_props4, missing_props4 = xml_parser.read_test_xml_file( + dir4 / "test.xml", None + ) assert isinstance(needs4, list) and len(needs4) == 1 tcneed4 = needs4[0] assert isinstance(tcneed4, DataOfTestCase) @@ -565,33 +573,27 @@ def test_get_metadata_from_test_path_local(): local_path = Path( "/home/root/docs-as-code/bazel-testlogs/src/extensions/foo/test.xml" ) - md = xml_parser.get_metadata_from_test_path(local_path) + md = xml_parser.get_metadata_from_test_path(local_path, None) assert md["repo_name"] == "local_repo" assert md["hash"] == "" assert md["url"] == "" -def test_get_metadata_from_test_path_combo_with_hash( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch -): +def test_get_metadata_from_test_path_combo_with_hash(tmp_path: Path): """Combo builds with 'hash' in known_good.json populate metadata correctly.""" json_file = tmp_path / "known_good.json" json_file.write_text(json.dumps(_KNOWN_GOOD_WITH_HASH)) - monkeypatch.setenv("KNOWN_GOOD_JSON", str(json_file)) - md = xml_parser.get_metadata_from_test_path(_COMBO_TEST_PATH) + md = xml_parser.get_metadata_from_test_path(_COMBO_TEST_PATH, json_file) assert md["repo_name"] == "score_docs_as_code" assert md["hash"] == "abc123hashvalue" assert md["url"] == "https://github.com/eclipse-score/docs-as-code" -def test_get_metadata_from_test_path_combo_with_version( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch -): +def test_get_metadata_from_test_path_combo_with_version(tmp_path: Path): """Combo builds with 'version' in known_good.json populate metadata correctly.""" json_file = tmp_path / "known_good.json" json_file.write_text(json.dumps(_KNOWN_GOOD_WITH_VERSION)) - monkeypatch.setenv("KNOWN_GOOD_JSON", str(json_file)) - md = xml_parser.get_metadata_from_test_path(_COMBO_TEST_PATH) + md = xml_parser.get_metadata_from_test_path(_COMBO_TEST_PATH, json_file) assert md["repo_name"] == "score_docs_as_code" assert md["hash"] == "v2.1.0" assert md["url"] == "https://github.com/eclipse-score/docs-as-code" diff --git a/src/extensions/score_source_code_linker/xml_parser.py b/src/extensions/score_source_code_linker/xml_parser.py index fde71ee7b..efe367979 100644 --- a/src/extensions/score_source_code_linker/xml_parser.py +++ b/src/extensions/score_source_code_linker/xml_parser.py @@ -49,9 +49,7 @@ store_data_of_test_case_json, store_test_xml_parsed_json, ) -from src.helper_lib import Environment, find_ws_root - -env = Environment() +from src.helper_lib import find_ws_root logger = logging.get_logger(__name__) logger.setLevel("DEBUG") @@ -116,7 +114,9 @@ def clean_test_file_name(raw_filepath: Path) -> Path: ) -def get_metadata_from_test_path(raw_filepath: Path) -> MetaData: +def get_metadata_from_test_path( + raw_filepath: Path, known_good_json: Path | None +) -> MetaData: """ Will parse out the metadata from the testpath. If test is local then the metadata will be: @@ -146,7 +146,6 @@ def get_metadata_from_test_path(raw_filepath: Path) -> MetaData: Removing everything up to and including 'bazel-testlogs' or 'tests-report' """ # print("THIs IS FILEPATH IN GET MD FROm TestPATH: ", raw_filepath) - known_good_json = env.optional_path("KNOWN_GOOD_JSON") clean_filepath = clean_test_file_name(raw_filepath) # print(f"This is the cleaned filepath: {clean_filepath}") repo_name = parse_repo_name_from_path(clean_filepath) @@ -206,7 +205,9 @@ def parse_properties(case_properties: dict[str, Any], properties: Element): def read_test_xml_file( - file: Path, allowed_dirs: list[str] | None = None + file: Path, + known_good_json: Path | None, + allowed_dirs: list[str] | None = None, ) -> tuple[list[DataOfTestCase], list[str], list[str]]: """ Reading & parsing the test.xml files into TestCaseNeeds @@ -222,7 +223,7 @@ def read_test_xml_file( missing_prop_tests: list[str] = [] tree = ET.parse(file) root = tree.getroot() - md = get_metadata_from_test_path(file) + md = get_metadata_from_test_path(file, known_good_json) for testsuite in root.findall("testsuite"): for testcase in testsuite.findall("testcase"): test_file = testcase.get("file") @@ -402,11 +403,13 @@ def build_test_needs_from_files( Returns: - list[TestCaseNeed] """ + known_good_json_str = app.config.KNOWN_GOOD_JSON.strip() + known_good_json = Path(known_good_json_str) if known_good_json_str else None tcns: list[DataOfTestCase] = [] for file in xml_paths: # Last value can be ignored. The 'is_valid' function already prints infos test_cases, tests_missing_all_props, tests_missing_some_props = ( - read_test_xml_file(file, allowed_dirs) + read_test_xml_file(file, known_good_json, allowed_dirs) ) non_prop_tests = ", ".join(n for n in tests_missing_all_props) if non_prop_tests: From 68e00f82d5812d0b466cb81cecc8b54c774f5c3e Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Wed, 16 Sep 2026 22:54:36 +0200 Subject: [PATCH 6/8] refactor: reuse runfiles path resolver for Sphinx config --- src/docs_cli/cli.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/docs_cli/cli.py b/src/docs_cli/cli.py index 834915cab..c953ba1f8 100644 --- a/src/docs_cli/cli.py +++ b/src/docs_cli/cli.py @@ -194,15 +194,7 @@ def sphinx_arguments( base_arguments.extend(["--warning-file", str(output_dir / "warnings.txt")]) if config_file := env.optional_path("SPHINX_CONFIG_FILE"): - # The action receives ctx.file.config.path, which is interpreted from - # the action's execution-root working directory. Resolve it locally - # instead of using runfiles lookup; interactive targets receive a - # runfiles-relative path and need that lookup before Sphinx gets the - # containing directory. - if config.is_bazel_build: - config_file = config_file.absolute() - elif not config_file.is_absolute(): - config_file = get_runfiles_dir() / config_file + config_file = _resolve_runfiles_relative_path(config, config_file) base_arguments.extend(["-c", str(config_file.parent)]) if metamodel_yaml := env.optional_path("SCORE_METAMODEL_YAML"): From 1b9e3867e3d3fb3fecba49757c888f1494e1cee4 Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Wed, 16 Sep 2026 23:04:03 +0200 Subject: [PATCH 7/8] refactor: resolve mounts manifest in docs CLI --- src/docs_cli/cli.py | 17 +++++----- src/docs_cli/main_test.py | 33 +++++++++++++++++-- .../score_cross_module_compatibility/BUILD | 2 +- .../__init__.py | 15 ++------- src/extensions/score_mounts/__init__.py | 13 ++------ 5 files changed, 46 insertions(+), 34 deletions(-) diff --git a/src/docs_cli/cli.py b/src/docs_cli/cli.py index c953ba1f8..e33d051c0 100644 --- a/src/docs_cli/cli.py +++ b/src/docs_cli/cli.py @@ -158,6 +158,10 @@ def sphinx_arguments( ) -> list[str]: """Build Sphinx arguments from the resolved launcher configuration.""" output_dir = config.output_dir + mounts_manifest = env.optional_path("MOUNTS_MANIFEST") + if mounts_manifest: + mounts_manifest = _resolve_runfiles_relative_path(config, mounts_manifest) + base_arguments = [ str(config.source_dir), str(output_dir), @@ -172,7 +176,7 @@ def sphinx_arguments( f"--define=testcase_source_dirs={env.get('TEST_SOURCES', '[]')}", # Path to the Bazel-emitted mounts manifest (empty when no mounts are # configured); consumed by the score_mounts extension. - f"--define=mounts_manifest={env.optional_path('MOUNTS_MANIFEST') or ''}", + f"--define=mounts_manifest={mounts_manifest or ''}", ] if config.is_bazel_build: @@ -232,17 +236,12 @@ def watch_arguments(config: DocsCliConfig) -> list[str]: mounts_manifest = env.optional_path("MOUNTS_MANIFEST") watch_arguments: list[str] = [] if mounts_manifest: - # ``MOUNTS_MANIFEST`` is runfiles-relative under ``bazel run`` and - # an ordinary path for direct invocations, matching score_mounts. - manifest_path = ( - get_runfiles_dir() / mounts_manifest - if config.is_bazel_run - else mounts_manifest - ) + manifest_path = _resolve_runfiles_relative_path(config, mounts_manifest) + runfiles_dir = get_runfiles_dir() if config.is_bazel_run else None for watch_dir in mounted_watch_dirs( manifest_path, config.ws_root, - get_runfiles_dir() if config.is_bazel_run else None, + runfiles_dir, ): watch_arguments.extend(["--watch", watch_dir]) return watch_arguments diff --git a/src/docs_cli/main_test.py b/src/docs_cli/main_test.py index 59e465e00..11311dea6 100644 --- a/src/docs_cli/main_test.py +++ b/src/docs_cli/main_test.py @@ -161,11 +161,11 @@ def test_live_preview_uses_port_and_bundle_watches( ) -> None: # Arrange monkeypatch.setenv("ACTION", "live_preview") - manifest = workspace / "mounts.json" + manifest = workspace / "runfiles/mounts.json" manifest.write_text( '{"mounts": [{"src_root": "extra/docs", "runtime_path": "extra/docs", "mount_at": "extra"}]}' ) - monkeypatch.setenv("MOUNTS_MANIFEST", str(manifest)) + monkeypatch.setenv("MOUNTS_MANIFEST", "mounts.json") autobuild = Mock() monkeypatch.setattr(docs_cli, "sphinx_autobuild_main", autobuild) @@ -180,6 +180,7 @@ def test_live_preview_uses_port_and_bundle_watches( # The requested port and source-linker setting are forwarded unchanged. assert "--port=42424242424" in arguments assert "--define=skip_rescanning_via_source_code_linker=1" in arguments + assert f"--define=mounts_manifest={manifest}" in arguments # Mounted bundle sources are watched in addition to the main docs tree. assert arguments[-2:] == ["--watch", str(workspace / "extra/docs")] # Live preview does not write the successful-build hash. @@ -193,6 +194,7 @@ def test_bazel_configuration_resolves_runfiles_and_preserves_repo_relative_edit_ # Arrange monkeypatch.setenv("SPHINX_CONFIG_FILE", "config/conf.py") monkeypatch.setenv("SCORE_METAMODEL_YAML", "config/metamodel.yaml") + monkeypatch.setenv("MOUNTS_MANIFEST", "mounts.json") monkeypatch.setenv("DATA", '[":bundle"]') monkeypatch.setenv("EXTERNAL_NEEDS_FILES", '["@vendor//:needs"]') monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo") @@ -208,6 +210,7 @@ def test_bazel_configuration_resolves_runfiles_and_preserves_repo_relative_edit_ "-c", str(workspace / "runfiles/config"), f"--define=score_metamodel_yaml={workspace}/runfiles/config/metamodel.yaml", + f"--define=mounts_manifest={workspace}/runfiles/mounts.json", # DATA and EXTERNAL_NEEDS_FILES are passed as one Sphinx define. '--define=external_needs_source=[":bundle", "@vendor//:needs"]', # GitHub metadata must keep edit links repository-relative. @@ -220,6 +223,32 @@ def test_bazel_configuration_resolves_runfiles_and_preserves_repo_relative_edit_ assert expected_arguments <= set(arguments) +def test_bazel_build_resolves_mount_manifest_from_execroot( + workspace: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Arrange + monkeypatch.delenv("BUILD_WORKSPACE_DIRECTORY") + monkeypatch.chdir(workspace) + monkeypatch.setenv("ACTION", "build_needs_json") + monkeypatch.setenv("OUTPUT_DIRECTORY", "outputs/needs") + monkeypatch.setenv( + "MOUNTS_MANIFEST", + "bazel-out/k8-fastbuild/bin/component/mounts.json", + ) + + # Act + config = DocsCliConfig.from_environment() + arguments = sphinx_arguments(config) + + # Assert + assert config.is_bazel_build + assert ( + f"--define=mounts_manifest={workspace}/bazel-out/k8-fastbuild/bin/component/mounts.json" + in arguments + ) + + def test_direct_invocation_resolves_paths_relative_to_cwd( workspace: Path, monkeypatch: pytest.MonkeyPatch, diff --git a/src/extensions/score_cross_module_compatibility/BUILD b/src/extensions/score_cross_module_compatibility/BUILD index afcbae665..3b9f356a3 100644 --- a/src/extensions/score_cross_module_compatibility/BUILD +++ b/src/extensions/score_cross_module_compatibility/BUILD @@ -12,7 +12,7 @@ py_library( srcs = ["__init__.py"], imports = ["."], visibility = ["//visibility:public"], - deps = all_requirements + ["@score_docs_as_code//src/helper_lib"], + deps = all_requirements, ) score_pytest( diff --git a/src/extensions/score_cross_module_compatibility/__init__.py b/src/extensions/score_cross_module_compatibility/__init__.py index 31ad1c6dd..96706f84d 100644 --- a/src/extensions/score_cross_module_compatibility/__init__.py +++ b/src/extensions/score_cross_module_compatibility/__init__.py @@ -22,8 +22,6 @@ from sphinx.util import logging from sphinx_needs.need_item import NeedItem -from src.helper_lib import find_ws_root, get_runfiles_dir - _VERSION_CONDITION = re.compile(r"^\s*version\s*==\s*(\d+)\s*$") logger = logging.getLogger(__name__) @@ -222,16 +220,9 @@ def _manifest_path(app: Sphinx) -> Path | None: raw = getattr(app.config, "mounts_manifest", "") if not isinstance(raw, str) or not raw.strip(): return None - direct = Path(raw) - runfiles = get_runfiles_dir() / raw - # ``mounts_manifest`` may be an execroot path, while the environment value - # passed to ``bazel run`` is runfiles-relative. Prefer an existing path so - # the policy does not depend on the current working directory. - if direct.is_file(): - return direct - if runfiles.is_file(): - return runfiles - return runfiles if find_ws_root() else direct + # The docs CLI resolves Bazel runfiles paths before passing this config + # value to Sphinx. + return Path(raw) def get_reporter(app: Sphinx) -> CompatibilityReporter: diff --git a/src/extensions/score_mounts/__init__.py b/src/extensions/score_mounts/__init__.py index fc211bdc1..c8b192b05 100644 --- a/src/extensions/score_mounts/__init__.py +++ b/src/extensions/score_mounts/__init__.py @@ -55,21 +55,14 @@ def _read_manifest(config: Config): """Locate and load the mounts manifest, or return ``None`` when unset. ``mounts_manifest`` is set by the docs CLI via ``--define`` from Bazel's - ``MOUNTS_MANIFEST`` env var. Its interpretation depends on the build - context: under ``bazel run`` it is a runfiles-relative path - (``$(rlocationpath)``) resolved against the runfiles dir; in a sandbox build - it is already relative to the exec root. Resolving the path here keeps - that context branch out of the pure ``_resolver`` module. + ``MOUNTS_MANIFEST`` env var. The CLI resolves the path for the active + execution context before passing it to Sphinx. """ raw = getattr(config, "mounts_manifest", "") if not raw or not raw.strip() or not isinstance(raw, str): return None - # ``bazel run`` passes an rlocation-relative path; ``sphinx_docs`` in a - # sandbox passes its execroot-relative ``$(location)`` path directly. - manifest_path = get_runfiles_dir() / raw if find_ws_root() else Path(raw) - - return load_mounts_manifest(manifest_path) + return load_mounts_manifest(Path(raw)) def _resolve_data_mounts( From 2ec62ac2ec7d0c93ffcaf9353eab2c2318c4e92a Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Wed, 16 Sep 2026 23:29:31 +0200 Subject: [PATCH 8/8] refactor: centralize external needs runfiles paths --- .../score_metamodel/external_needs.py | 30 ++++++++++-------- .../tests/test_external_needs.py | 31 +++++++++++++++++++ 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/extensions/score_metamodel/external_needs.py b/src/extensions/score_metamodel/external_needs.py index cd89a6566..93320ea45 100644 --- a/src/extensions/score_metamodel/external_needs.py +++ b/src/extensions/score_metamodel/external_needs.py @@ -74,6 +74,18 @@ def _runfiles_module_dir(e: ExternalNeedsSource) -> str: return "_main" if e.is_local else f"{e.bazel_module}+" +def _external_needs_runfiles_path( + runfiles_dir: Path, source: ExternalNeedsSource, *suffix: str +) -> Path: + """Build an external source path without reading the process environment.""" + return ( + runfiles_dir + / _runfiles_module_dir(source) + / source.path_to_target + / Path(*suffix) + ) + + def parse_external_needs_sources_from_DATA(v: str) -> list[ExternalNeedsSource]: if v in ["[]", ""]: return [] @@ -180,15 +192,10 @@ def get_external_needs_source(external_needs_source: str) -> list[ExternalNeedsS def add_external_needs_json(e: ExternalNeedsSource, config: Config): - json_file_raw = ( - Path(_runfiles_module_dir(e)) - / e.path_to_target - / e.target - / "_build/needs/needs.json" - ) - r = get_runfiles_dir() - json_file = r / json_file_raw + json_file = _external_needs_runfiles_path( + r, e, e.target, "_build", "needs", "needs.json" + ) logger.debug(f"External needs.json: {json_file}") try: needs_json_data = json.loads(Path(json_file).read_text(encoding="utf-8")) # pyright: ignore[reportAny] @@ -219,7 +226,7 @@ def add_external_docs_sources(e: ExternalNeedsSource, config: Config): if "ide_support.runfiles" in str(r): logger.error("Combo builds are currently only supported with Bazel.") return - docs_source_path = Path(r) / _runfiles_module_dir(e) / e.path_to_target + docs_source_path = _external_needs_runfiles_path(r, e) # A cross-module root mount keeps its module name as the collection key # (unchanged). Sub-package / same-repo mounts disambiguate via the path. @@ -271,11 +278,8 @@ def connect_external_needs(app: Sphinx, config: Config): def _add_needs_json_file(ext_needs: ExternalNeedsSource, config: Config) -> None: """Resolve a needs_json_file target from runfiles and register it.""" - json_file_raw = ( - Path(_runfiles_module_dir(ext_needs)) / ext_needs.path_to_target / "needs.json" - ) r = get_runfiles_dir() - json_file = r / json_file_raw + json_file = _external_needs_runfiles_path(r, ext_needs, "needs.json") logger.debug(f"External needs_json_file: {json_file}") try: needs_json_data = json.loads( diff --git a/src/extensions/score_metamodel/tests/test_external_needs.py b/src/extensions/score_metamodel/tests/test_external_needs.py index 4e301e160..348c9d82e 100644 --- a/src/extensions/score_metamodel/tests/test_external_needs.py +++ b/src/extensions/score_metamodel/tests/test_external_needs.py @@ -27,6 +27,7 @@ from score_metamodel.external_needs import ( ExternalNeedsSource, _add_needs_json_file, # pyright: ignore[reportPrivateUsage] - white-box unit test + _external_needs_runfiles_path, # pyright: ignore[reportPrivateUsage] - white-box unit test add_external_docs_sources, add_external_needs_json, get_external_needs_source, @@ -79,6 +80,36 @@ def test_empty_list(): assert parse_external_needs_sources_from_DATA("[]") == [] +@pytest.mark.parametrize( + ("source", "suffix", "expected"), + [ + ( + ExternalNeedsSource( + bazel_module="repo", + path_to_target="docs", + target="needs_json", + ), + ("needs_json", "_build", "needs", "needs.json"), + Path("/runfiles/repo+/docs/needs_json/_build/needs/needs.json"), + ), + ( + ExternalNeedsSource( + bazel_module="", + path_to_target="docs", + target="docs_sources", + is_local=True, + ), + (), + Path("/runfiles/_main/docs"), + ), + ], +) +def test_external_needs_runfiles_path_is_environment_independent( + source: ExternalNeedsSource, suffix: tuple[str, ...], expected: Path +) -> None: + assert _external_needs_runfiles_path(Path("/runfiles"), source, *suffix) == expected + + def test_external_str_is_neither_at_nor_slash(): # Labels that start with neither "@" nor "//" are not bazel needs sources. assert get_external_needs_source('["noatrepo/foo/bar:baz"]') == []