From e4e602c10f7e447e796a9667434557e073438751 Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Fri, 18 Sep 2026 12:14:15 +0200 Subject: [PATCH 1/3] refactor: transport external needs labels explicitly --- bzl/needs_rules.bzl | 7 +- docs.bzl | 10 +- src/docs_cli/README.md | 2 + src/docs_cli/cli.py | 43 +++- src/docs_cli/main_test.py | 60 ++++- src/extensions/score_metamodel/__init__.py | 7 + .../score_metamodel/external_needs.py | 157 +++++------- .../tests/test_external_needs.py | 227 ++++++++---------- .../score_source_code_linker/xml_parser.py | 2 +- src/helper_lib/env.py | 3 +- src/helper_lib/external_needs.py | 194 +++++++++++++++ 11 files changed, 453 insertions(+), 259 deletions(-) create mode 100644 src/helper_lib/external_needs.py diff --git a/bzl/needs_rules.bzl b/bzl/needs_rules.bzl index dbcca4b33..f0fa72999 100644 --- a/bzl/needs_rules.bzl +++ b/bzl/needs_rules.bzl @@ -33,8 +33,8 @@ def _sphinx_docs_impl(ctx): fail("Sphinx requires a bundle with direct documentation sources") # 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. + # Pass them through the environment variables consumed by the CLI; 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. @@ -43,7 +43,7 @@ def _sphinx_docs_impl(ctx): "SOURCE_DIRECTORY": bundle.source_dir_execroot_path, "OUTPUT_DIRECTORY": output.path, "SPHINX_CONFIG_FILE": ctx.file.config.path, - "DATA": "[]", + "EXTERNAL_NEEDS_LABELS": ctx.attr.external_needs_labels, "SCORE_SOURCELINKS": ( ctx.file.score_sourcelinks_json.path if ctx.file.score_sourcelinks_json else "" ), @@ -93,6 +93,7 @@ sphinx_docs = rule( "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), + "external_needs_labels": attr.string(default = "[]"), "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 e5c8773be..0dc937e55 100644 --- a/docs.bzl +++ b/docs.bzl @@ -79,7 +79,6 @@ def _sphinx_define(name, value): def _needs_sphinx_extra_opts( master_doc, - external_needs_source, score_bundle_needs_export, score_source_code_linker_plain_links): """Return per-target Sphinx configuration defines for a Needs build.""" @@ -90,7 +89,6 @@ def _needs_sphinx_extra_opts( option for name, value in [ ("master_doc", master_doc), - ("external_needs_source", external_needs_source), ("score_bundle_needs_export", score_bundle_needs_export), ("score_source_code_linker_plain_links", score_source_code_linker_plain_links), ] @@ -119,7 +117,7 @@ def _needs_sphinx_docs( sphinx_build_deps, bundle, master_doc = None, - external_needs_source = None, + external_needs_labels = "[]", score_bundle_needs_export = None, score_sourcelinks_json = None, score_source_code_linker_plain_links = None, @@ -152,13 +150,13 @@ def _needs_sphinx_docs( data = sphinx_build_data, extra_opts = _needs_sphinx_extra_opts( master_doc, - external_needs_source, score_bundle_needs_export, score_source_code_linker_plain_links, ), # 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. + external_needs_labels = external_needs_labels, score_sourcelinks_json = score_sourcelinks_json, mounts_manifest = mounts_manifest, score_metamodel_yaml = score_metamodel_yaml, @@ -369,7 +367,7 @@ def _declare_bundle_local_needs( sphinx_build_deps = sphinx_build_deps, sphinx_build_data = data, master_doc = entry_doc, - external_needs_source = "[]", + external_needs_labels = "[]", score_bundle_needs_export = "1", score_sourcelinks_json = sourcelinks_json, score_source_code_linker_plain_links = "1", @@ -701,7 +699,7 @@ def docs( config = sphinx_config, sphinx_build_deps = deps, sphinx_build_data = data + external_needs + metamodel_label + [":docs_bundle"], - external_needs_source = str(data + external_needs), + external_needs_labels = str(data + external_needs), score_sourcelinks_json = ":sourcelinks_json", score_source_code_linker_plain_links = "1", mounts_manifest = mounts_manifest, diff --git a/src/docs_cli/README.md b/src/docs_cli/README.md index 4d83b8ccd..4ce06aa7d 100644 --- a/src/docs_cli/README.md +++ b/src/docs_cli/README.md @@ -49,6 +49,8 @@ linking can traverse this Bazel package boundary. `docs.bzl` provides `SOURCE_DIRECTORY`, `PACKAGE_DIR`, `DATA`, and optional configuration such as `SPHINX_CONFIG_FILE`, `SCORE_METAMODEL_YAML`, `MOUNTS_MANIFEST`, `EXTERNAL_NEEDS_FILES`, `TEST_SOURCES` and `KNOWN_GOOD_JSON`. +The sandboxed Needs action passes its external-needs labels separately through +the internal `EXTERNAL_NEEDS_LABELS` variable. Bazel provides the workspace and runfiles locations. The CLI resolves source and output paths relative to the package containing the `docs()` call; generated configuration is resolved through runfiles. diff --git a/src/docs_cli/cli.py b/src/docs_cli/cli.py index e33d051c0..e98cb7f5c 100644 --- a/src/docs_cli/cli.py +++ b/src/docs_cli/cli.py @@ -15,7 +15,6 @@ import argparse import hashlib -import json import logging import os import shutil @@ -32,6 +31,11 @@ from src.extensions.score_mounts._resolver import load_mounts_manifest, resolve_walk_dir from src.helper_lib import Environment, get_runfiles_dir from src.helper_lib.config import DocsCliConfig +from src.helper_lib.external_needs import ( + parse_external_needs_labels, + resolve_external_needs_sources, + serialize_external_needs_sources, +) logger = logging.getLogger(__name__) @@ -40,15 +44,30 @@ env = Environment() -def _merged_external_needs() -> str: - """Combine DATA and EXTERNAL_NEEDS_FILES into one JSON label list. +def _build_external_needs_source_config() -> str: + """Build the JSON for Sphinx's ``external_needs_source`` configuration. + + ``DATA`` contains all data dependencies of the documentation target, + ``EXTERNAL_NEEDS_FILES`` contains explicitly declared external-needs + dependencies, and the sandboxed Needs action uses + ``EXTERNAL_NEEDS_LABELS`` for its explicitly declared label list. All + variables contain JSON arrays of Bazel labels. - Both env vars hold JSON lists of Bazel labels; the extension parses the - resulting `external_needs_source` define uniformly. + The CLI combines those arrays, discards ordinary data dependencies, turns + supported external-needs labels into descriptors, resolves their paths in + the Bazel runfiles tree, and serializes the descriptors for the Sphinx + extension. If no runfiles directory is available, the descriptors retain + no resolved path and the extension resolves them through its own fallback + (for now). """ - data = env.string_list("DATA") + data = env.string_list("DATA", "[]") external = env.string_list("EXTERNAL_NEEDS_FILES", "[]") - return json.dumps(data + external) + labels = env.string_list("EXTERNAL_NEEDS_LABELS", "[]") + sources = parse_external_needs_labels(data + external + labels) + runfiles_dir = env.optional_path("RUNFILES_DIR") + if runfiles_dir is not None: + sources = resolve_external_needs_sources(sources, runfiles_dir.absolute()) + return serialize_external_needs_sources(sources) def _compute_hash(files: list[Path]) -> str: @@ -159,6 +178,7 @@ def sphinx_arguments( """Build Sphinx arguments from the resolved launcher configuration.""" output_dir = config.output_dir mounts_manifest = env.optional_path("MOUNTS_MANIFEST") + runfiles_dir = env.optional_path("RUNFILES_DIR") if mounts_manifest: mounts_manifest = _resolve_runfiles_relative_path(config, mounts_manifest) @@ -170,13 +190,16 @@ def sphinx_arguments( "-T", # show details in case of errors in extensions "--jobs", "auto", - # Merge DATA (:needs_json / :docs_sources) with EXTERNAL_NEEDS_FILES - # (:needs_json_file) into one define consumed by the Sphinx extensions. - f"--define=external_needs_source={_merged_external_needs()}", + # Convert Bazel data dependencies into descriptors consumed by the + # score_metamodel extension. + f"--define=external_needs_source={_build_external_needs_source_config()}", 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={mounts_manifest or ''}", + # The external-needs extension uses this root to resolve label-based + # inputs without reading the process environment itself. + f"--define=runfiles_dir={runfiles_dir.absolute() if runfiles_dir else ''}", ] if config.is_bazel_build: diff --git a/src/docs_cli/main_test.py b/src/docs_cli/main_test.py index 11311dea6..78180bec0 100644 --- a/src/docs_cli/main_test.py +++ b/src/docs_cli/main_test.py @@ -11,6 +11,7 @@ # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* +import json from pathlib import Path from unittest.mock import Mock @@ -31,6 +32,7 @@ def workspace(fs: FFS, monkeypatch: pytest.MonkeyPatch) -> Path: # optional values left behind by the test runner before setting the basics. ENVIRONMENT_OVERRIDES = ( "EXTERNAL_NEEDS_FILES", + "EXTERNAL_NEEDS_LABELS", "TEST_SOURCES", "MOUNTS_MANIFEST", "SPHINX_CONFIG_FILE", @@ -195,8 +197,8 @@ def test_bazel_configuration_resolves_runfiles_and_preserves_repo_relative_edit_ 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("DATA", '["//:needs_json"]') + monkeypatch.setenv("EXTERNAL_NEEDS_FILES", '["@vendor//:needs_json"]') monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo") monkeypatch.setenv("KNOWN_GOOD_JSON", "baseline.json") monkeypatch.setenv("ACTION", "incremental") @@ -205,14 +207,37 @@ def test_bazel_configuration_resolves_runfiles_and_preserves_repo_relative_edit_ arguments = sphinx_arguments(DocsCliConfig.from_environment()) # Assert + external_needs = json.dumps( + [ + { + "bazel_module": "", + "path_to_target": "", + "target": "needs_json", + "is_local": True, + "resolved_path": str( + workspace / "runfiles/_main/needs_json/_build/needs/needs.json" + ), + }, + { + "bazel_module": "vendor", + "path_to_target": "", + "target": "needs_json", + "is_local": False, + "resolved_path": str( + workspace / "runfiles/vendor+/needs_json/_build/needs/needs.json" + ), + }, + ] + ) expected_arguments = { # Generated configuration and metamodel paths use the runfiles tree. "-c", str(workspace / "runfiles/config"), f"--define=score_metamodel_yaml={workspace}/runfiles/config/metamodel.yaml", f"--define=mounts_manifest={workspace}/runfiles/mounts.json", + f"--define=runfiles_dir={workspace}/runfiles", # DATA and EXTERNAL_NEEDS_FILES are passed as one Sphinx define. - '--define=external_needs_source=[":bundle", "@vendor//:needs"]', + f"--define=external_needs_source={external_needs}", # GitHub metadata must keep edit links repository-relative. "-A=github_user=owner", "-A=github_repo=repo", @@ -249,6 +274,35 @@ def test_bazel_build_resolves_mount_manifest_from_execroot( ) +def test_bazel_needs_action_uses_external_needs_labels_channel( + 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("EXTERNAL_NEEDS_LABELS", '["//:needs_json"]') + + # Act + arguments = sphinx_arguments(DocsCliConfig.from_environment()) + + # Assert + expected_source: dict[str, object] = { + "bazel_module": "", + "path_to_target": "", + "target": "needs_json", + "is_local": True, + "resolved_path": str( + workspace / "runfiles/_main/needs_json/_build/needs/needs.json" + ), + } + assert ( + f"--define=external_needs_source={json.dumps([expected_source])}" in arguments + ) + + def test_direct_invocation_resolves_paths_relative_to_cwd( workspace: Path, monkeypatch: pytest.MonkeyPatch, diff --git a/src/extensions/score_metamodel/__init__.py b/src/extensions/score_metamodel/__init__.py index 8a6a35777..83593785d 100644 --- a/src/extensions/score_metamodel/__init__.py +++ b/src/extensions/score_metamodel/__init__.py @@ -243,6 +243,13 @@ def _clear_needs_defaults(app: Sphinx): def setup(app: Sphinx) -> dict[str, str | bool]: app.add_config_value("external_needs_source", "", rebuild="env") + app.add_config_value( + "runfiles_dir", + "", + rebuild="env", + types=str, + description="Bazel runfiles root supplied by the documentation CLI.", + ) app.add_config_value("score_metamodel_yaml", "", rebuild="env") app.add_config_value("required_in_id", [], rebuild="env") app.add_config_value("score_bundle_needs_export", False, rebuild="env") diff --git a/src/extensions/score_metamodel/external_needs.py b/src/extensions/score_metamodel/external_needs.py index 93320ea45..c3f36de6d 100644 --- a/src/extensions/score_metamodel/external_needs.py +++ b/src/extensions/score_metamodel/external_needs.py @@ -13,7 +13,6 @@ import json import subprocess -from dataclasses import dataclass from pathlib import Path from sphinx.application import Sphinx @@ -22,85 +21,26 @@ from sphinx_needs.needsfile import NeedsList from src.helper_lib import get_runfiles_dir +from src.helper_lib.external_needs import ( + ExternalNeedsSource as ExternalNeedsSource, + external_needs_runfiles_path, + external_needs_source_path as _external_needs_source_path, + parse_bazel_external_need, + parse_external_needs_source_config as _parse_external_needs_source_config, +) logger = logging.getLogger(__name__) +_external_needs_runfiles_path = external_needs_runfiles_path +_parse_bazel_external_need = parse_bazel_external_need -@dataclass -class ExternalNeedsSource: - bazel_module: str - path_to_target: str - target: str - # True for a same-repo mount (`//pkg:needs_json`), whose runfiles live under - # `_main/…`. False for a cross-module mount (`@repo//…:needs_json`), whose - # runfiles live under `{bazel_module}+/…`. - is_local: bool = False - -def _parse_bazel_external_need(s: str) -> ExternalNeedsSource | None: - is_cross_module = s.startswith("@") - is_local = s.startswith("//") - if not is_cross_module and not is_local: - # Local need, not external needs - return None - - if "//" not in s or ":" not in s: - raise ValueError( - f"Unsuported external data dependency: '{s}'. Must contain '//' & ':'" - ) - repo_and_path, target = s.split( - ":", 1 - ) # @score_process//:needs_json => [@score_process//, needs_json] - repo, path_to_target = repo_and_path.split("//", 1) - repo = repo.lstrip("@") # empty for same-repo `//pkg:needs_json` - - if target in ("needs_json", "needs_json_file", "docs_sources"): - return ExternalNeedsSource( - bazel_module=repo, - path_to_target=path_to_target, - target=target, - is_local=is_local, - ) - # Unknown data target. Probably not a needs.json file. - return None - - -def _runfiles_module_dir(e: ExternalNeedsSource) -> str: - """Runfiles top-level directory holding this source's package tree. - - Same-repo mounts are staged under `_main/…`; cross-module mounts under the - module's bzlmod canonical name `{bazel_module}+/…`. - """ - 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 [] - - logger.debug(f"Parsing external needs sources: {v}") - - try: - data = json.loads(v) - except json.JSONDecodeError as e: - logger.error(f"Failed to parse external needs sources from DATA {v}: {e}") - raise SystemExit(1) from e - - res = [res for el in data if (res := _parse_bazel_external_need(el))] - logger.debug(f"Parsed external needs sources: {res}") - return res +def _runfiles_dir(config: Config) -> Path: + """Use the CLI-provided runfiles root, with a direct-invocation fallback.""" + raw = getattr(config, "runfiles_dir", "") + if isinstance(raw, str) and raw.strip(): + return Path(raw) + return get_runfiles_dir() def parse_external_needs_sources_from_bazel_query() -> list[ExternalNeedsSource]: @@ -183,19 +123,25 @@ def temp(self: NeedsList): def get_external_needs_source(external_needs_source: str) -> list[ExternalNeedsSource]: if external_needs_source: - # Path taken for all invocations via `bazel` - return parse_external_needs_sources_from_DATA(external_needs_source) + # The CLI supplies resolved descriptors for Bazel invocations. + try: + return _parse_external_needs_source_config(external_needs_source) + except json.JSONDecodeError as e: + logger.error( + f"Failed to parse external needs sources from " + f"external_needs_source {external_needs_source}: {e}" + ) + raise SystemExit(1) from e else: # This is the path taken for anything that doesn't # run via `bazel` e.g. esbonio or other direct executions return parse_external_needs_sources_from_bazel_query() # pyright: ignore[reportAny] -def add_external_needs_json(e: ExternalNeedsSource, config: Config): - r = get_runfiles_dir() - json_file = _external_needs_runfiles_path( - r, e, e.target, "_build", "needs", "needs.json" - ) +def add_external_needs_json( + e: ExternalNeedsSource, config: Config, runfiles_dir: Path | None +): + json_file = _external_needs_source_path(runfiles_dir, e) 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] @@ -217,16 +163,14 @@ def add_external_needs_json(e: ExternalNeedsSource, config: Config): ) -def add_external_docs_sources(e: ExternalNeedsSource, config: Config): +def add_external_docs_sources( + e: ExternalNeedsSource, config: Config, runfiles_dir: Path | None +): # Note that bazel does NOT write the files under e.target! # The runfiles layout mirrors the original git layout: same-repo mounts live # under `_main/…`, cross-module mounts under `{e.bazel_module}+/…` # (see _runfiles_module_dir). - r = get_runfiles_dir() - if "ide_support.runfiles" in str(r): - logger.error("Combo builds are currently only supported with Bazel.") - return - docs_source_path = _external_needs_runfiles_path(r, e) + docs_source_path = _external_needs_source_path(runfiles_dir, e) # A cross-module root mount keeps its module name as the collection key # (unchanged). Sub-package / same-repo mounts disambiguate via the path. @@ -256,30 +200,37 @@ def connect_external_needs(app: Sphinx, config: Config): export_values={"project_url": ""} if bundle_export else None, ) - # Local external needs from DATA (e.g. :needs_json or :docs_sources) + # External needs descriptors supplied by the documentation CLI. external_needs = get_external_needs_source(app.config.external_needs_source) # this sets the default value - required for the needs-config-writer # setting 'needscfg_exclude_defaults = True' to see the diff config.needs_external_needs = [] - for e in external_needs: - if e.target == "needs_json": - add_external_needs_json(e, app.config) - elif e.target == "needs_json_file": - _add_needs_json_file(e, app.config) - elif e.target == "docs_sources": - add_external_docs_sources(e, app.config) - else: - raise ValueError( - f"Internal Error. Unknown external needs target: {e.target}" - ) + if external_needs: + runfiles_dir = ( + _runfiles_dir(app.config) + if any(source.resolved_path is None for source in external_needs) + else None + ) + for e in external_needs: + if e.target == "needs_json": + add_external_needs_json(e, app.config, runfiles_dir) + elif e.target == "needs_json_file": + _add_needs_json_file(e, app.config, runfiles_dir) + elif e.target == "docs_sources": + add_external_docs_sources(e, app.config, runfiles_dir) + else: + raise ValueError( + f"Internal Error. Unknown external needs target: {e.target}" + ) -def _add_needs_json_file(ext_needs: ExternalNeedsSource, config: Config) -> None: +def _add_needs_json_file( + ext_needs: ExternalNeedsSource, config: Config, runfiles_dir: Path | None +) -> None: """Resolve a needs_json_file target from runfiles and register it.""" - r = get_runfiles_dir() - json_file = _external_needs_runfiles_path(r, ext_needs, "needs.json") + json_file = _external_needs_source_path(runfiles_dir, ext_needs) 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 348c9d82e..f9bff1c80 100644 --- a/src/extensions/score_metamodel/tests/test_external_needs.py +++ b/src/extensions/score_metamodel/tests/test_external_needs.py @@ -28,10 +28,11 @@ ExternalNeedsSource, _add_needs_json_file, # pyright: ignore[reportPrivateUsage] - white-box unit test _external_needs_runfiles_path, # pyright: ignore[reportPrivateUsage] - white-box unit test + _external_needs_source_path, # pyright: ignore[reportPrivateUsage] - white-box unit test + _runfiles_dir, # pyright: ignore[reportPrivateUsage] - white-box unit test add_external_docs_sources, add_external_needs_json, get_external_needs_source, - parse_external_needs_sources_from_DATA, ) from sphinx.config import Config from sphinx_needs.needsfile import NeedsList @@ -76,10 +77,6 @@ def test_extend_needs_json_exporter_can_override_bundle_export_metadata( assert needs_list.needs_list["project_url"] == "" -def test_empty_list(): - assert parse_external_needs_sources_from_DATA("[]") == [] - - @pytest.mark.parametrize( ("source", "suffix", "expected"), [ @@ -110,110 +107,92 @@ def test_external_needs_runfiles_path_is_environment_independent( 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"]') == [] - - -def test_same_repo_entry_with_path(): - # A same-repo `//pkg:needs_json` mount now parses as a local source that - # carries its sub-package path. - result = parse_external_needs_sources_from_DATA('["//foo/bar:needs_json"]') - assert result == [ - ExternalNeedsSource( - bazel_module="", - path_to_target="foo/bar", - target="needs_json", - is_local=True, - ) - ] - - -def test_same_repo_root_entry(): - result = parse_external_needs_sources_from_DATA('["//:needs_json"]') - assert result == [ - ExternalNeedsSource( - bazel_module="", - path_to_target="", - target="needs_json", - is_local=True, - ) - ] - - -def test_cross_module_sub_package_entry(): - # A cross-module sub-package target now parses (previously rejected) and - # keeps its path so the runfiles path can be built correctly. - result = parse_external_needs_sources_from_DATA('["@repo//foo/bar:needs_json"]') - assert result == [ - ExternalNeedsSource( - bazel_module="repo", - path_to_target="foo/bar", - target="needs_json", - is_local=False, - ) - ] - - -def test_single_entry_with_path_non_target(): - # A target that is not needs_json / docs_sources is not reported as an external needs source. - result = parse_external_needs_sources_from_DATA('["@repo//foo/bar:baz"]') - assert result == [] - - -def test_single_entry_no_path(): - result = parse_external_needs_sources_from_DATA('["@repo//:target"]') - # If a target is not named "needs_json", it will not be reported as external needs - assert result == [] - +@pytest.mark.parametrize( + ("source", "expected"), + [ + ( + ExternalNeedsSource( + bazel_module="repo", + path_to_target="docs", + target="needs_json", + ), + Path("/runfiles/repo+/docs/needs_json/_build/needs/needs.json"), + ), + ( + ExternalNeedsSource( + bazel_module="repo", + path_to_target="docs", + target="needs_json_file", + ), + Path("/runfiles/repo+/docs/needs.json"), + ), + ( + ExternalNeedsSource( + bazel_module="", + path_to_target="docs", + target="docs_sources", + is_local=True, + ), + Path("/runfiles/_main/docs"), + ), + ], +) +def test_external_needs_source_path_selects_target_layout( + source: ExternalNeedsSource, expected: Path +) -> None: + assert _external_needs_source_path(Path("/runfiles"), source) == expected -def test_single_entry_json_no_path(): - result = parse_external_needs_sources_from_DATA('["@repo//:needs_json"]') - assert result == [ - ExternalNeedsSource(bazel_module="repo", path_to_target="", target="needs_json") - ] +def test_configured_runfiles_dir_is_used_for_external_sources( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + config = Config() + configured_runfiles = tmp_path / "configured.runfiles" + config.runfiles_dir = str(configured_runfiles) -def test_multiple_entries(): - result = parse_external_needs_sources_from_DATA( - '["@repo1//:needs_json", "@repo2//:needs_json"]' + monkeypatch.setattr( + ext_needs, + "get_runfiles_dir", + lambda: pytest.fail("configured runfiles root should be preferred"), ) - assert result == [ - ExternalNeedsSource( - bazel_module="repo1", path_to_target="", target="needs_json" - ), - ExternalNeedsSource( - bazel_module="repo2", path_to_target="", target="needs_json" - ), - ] - -def test_multiple_entries_2(): - # Both targets are named "needs_json" but one is a sub-package target, so the path is preserved. - result = parse_external_needs_sources_from_DATA( - '["@repo1//:needs_json", "@repo2//path:needs_json"]' + assert _runfiles_dir(config) == configured_runfiles + + +def test_descriptor_config_preserves_resolved_source_path() -> None: + resolved_path = Path("/runfiles/vendor+/needs_json/_build/needs/needs.json") + raw = json.dumps( + [ + { + "bazel_module": "vendor", + "path_to_target": "", + "target": "needs_json", + "is_local": False, + "resolved_path": str(resolved_path), + } + ] ) - assert result == [ - ExternalNeedsSource( - bazel_module="repo1", path_to_target="", target="needs_json" - ), + sources = get_external_needs_source(raw) + + assert sources == [ ExternalNeedsSource( - bazel_module="repo2", - path_to_target="path", + bazel_module="vendor", + path_to_target="", target="needs_json", - is_local=False, - ), + resolved_path=resolved_path, + ) ] + assert _external_needs_source_path(None, sources[0]) == resolved_path -def test_invalid_entry(): - with pytest.raises(ValueError): - _ = parse_external_needs_sources_from_DATA('["@not_a_valid_string"]') +def test_external_needs_source_rejects_label_lists() -> None: + with pytest.raises(ValueError, match="descriptor objects"): + get_external_needs_source('["//:needs_json"]') def test_add_external_needs_json_appends_entry( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path + tmp_path: Path, ) -> None: """add_external_needs_json should append one external needs mapping entry.""" # Arrange @@ -231,9 +210,7 @@ def test_add_external_needs_json_appends_entry( json.dumps({"project_url": "https://example.test/repo"}), encoding="utf-8" ) - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: runfiles_dir) - - add_external_needs_json(e, config) + add_external_needs_json(e, config, runfiles_dir) assert config.needs_external_needs is not None assert len(config.needs_external_needs) == 1 @@ -243,7 +220,7 @@ def test_add_external_needs_json_appends_entry( def test_add_external_needs_json_appends_entry_local( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path + tmp_path: Path, ) -> None: """A same-repo mount resolves under `_main///…`.""" e = ExternalNeedsSource( @@ -265,9 +242,7 @@ def test_add_external_needs_json_appends_entry_local( json.dumps({"project_url": "https://example.test/local"}), encoding="utf-8" ) - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: runfiles_dir) - - add_external_needs_json(e, config) + add_external_needs_json(e, config, runfiles_dir) assert config.needs_external_needs is not None assert len(config.needs_external_needs) == 1 @@ -277,7 +252,7 @@ def test_add_external_needs_json_appends_entry_local( def test_add_needs_json_file_appends_entry( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path + tmp_path: Path, ) -> None: """_add_needs_json_file should load from a :needs_json_file target.""" # Arrange: create the needs.json at the runfiles path @@ -292,8 +267,6 @@ def test_add_needs_json_file_appends_entry( config = Config() config.needs_external_needs = [] - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: runfiles_dir) - # Act e = ExternalNeedsSource( bazel_module="ext_mod", @@ -301,7 +274,7 @@ def test_add_needs_json_file_appends_entry( path_to_target="", is_local=False, ) - _add_needs_json_file(e, config) + _add_needs_json_file(e, config, runfiles_dir) # Assert assert config.needs_external_needs is not None @@ -312,7 +285,7 @@ def test_add_needs_json_file_appends_entry( def test_add_external_needs_json_missing_file_keeps_list_empty( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path + tmp_path: Path, ) -> None: """add_external_needs_json should return gracefully when JSON file is missing.""" # Arrange @@ -322,16 +295,14 @@ def test_add_external_needs_json_missing_file_keeps_list_empty( config = Config() config.needs_external_needs = [] - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: tmp_path) - - add_external_needs_json(e, config) + add_external_needs_json(e, config, tmp_path) # Assert assert config.needs_external_needs == [] def test_add_external_docs_sources_adds_collection( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path + tmp_path: Path, ) -> None: """add_external_docs_sources should add one symlink collection entry.""" e = ExternalNeedsSource( @@ -340,9 +311,7 @@ def test_add_external_docs_sources_adds_collection( config = Config() config.collections = {} - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: tmp_path) - - add_external_docs_sources(e, config) + add_external_docs_sources(e, config, tmp_path) assert config.collections is not None assert "third_party_docs" in config.collections @@ -353,7 +322,7 @@ def test_add_external_docs_sources_adds_collection( def test_add_external_docs_sources_local_sub_package( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path + tmp_path: Path, ) -> None: """A same-repo sub-package `docs_sources` mount resolves under `_main/`. @@ -371,9 +340,7 @@ def test_add_external_docs_sources_local_sub_package( config = Config() config.collections = {} - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: tmp_path) - - add_external_docs_sources(e, config) + add_external_docs_sources(e, config, tmp_path) assert config.collections is not None key = "src/tests/e2e/external_needs/producer" @@ -387,7 +354,7 @@ def test_add_external_docs_sources_local_sub_package( def test_add_external_docs_sources_local_root_key_fallback( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path + tmp_path: Path, ) -> None: """A same-repo root `docs_sources` mount falls back to the `_main` key. @@ -404,9 +371,7 @@ def test_add_external_docs_sources_local_root_key_fallback( config = Config() config.collections = {} - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: tmp_path) - - add_external_docs_sources(e, config) + add_external_docs_sources(e, config, tmp_path) assert config.collections is not None assert "_main" in config.collections @@ -416,20 +381,20 @@ def test_add_external_docs_sources_local_root_key_fallback( assert entry["target"] == "_main" -def test_add_external_docs_sources_ide_support_returns_without_changes( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """add_external_docs_sources should exit early for ide_support.runfiles paths.""" +def test_add_external_docs_sources_ide_support_uses_runfiles_tree() -> None: + """add_external_docs_sources should use the IDE fallback runfiles tree.""" e = ExternalNeedsSource( bazel_module="third_party_docs", target="docs_sources", path_to_target="" ) config = Config() config.collections = {} - monkeypatch.setattr( - ext_needs, "get_runfiles_dir", lambda: Path("/tmp/ide_support.runfiles") - ) - - add_external_docs_sources(e, config) + add_external_docs_sources(e, config, Path("/tmp/ide_support.runfiles")) - assert config.collections == {} + assert config.collections == { + "third_party_docs": { + "driver": "symlink", + "source": "/tmp/ide_support.runfiles/third_party_docs+", + "target": "third_party_docs", + } + } diff --git a/src/extensions/score_source_code_linker/xml_parser.py b/src/extensions/score_source_code_linker/xml_parser.py index efe367979..f20a6006e 100644 --- a/src/extensions/score_source_code_linker/xml_parser.py +++ b/src/extensions/score_source_code_linker/xml_parser.py @@ -60,7 +60,7 @@ def parse_testcase_source_dirs(v: str) -> list[str]: The value arrives as a `str(list)` produced by Starlark (double-quoted, i.e. valid JSON), mirroring how `external_needs_source` is parsed in - `score_metamodel.external_needs.parse_external_needs_sources_from_DATA`. + ` score_metamodel.external_needs.get_external_needs_source`. """ if v in ("[]", ""): return [] diff --git a/src/helper_lib/env.py b/src/helper_lib/env.py index ed3975b9b..2445c762d 100644 --- a/src/helper_lib/env.py +++ b/src/helper_lib/env.py @@ -68,8 +68,7 @@ def json(self, name: str, default: str | None = None) -> object: def string_list(self, name: str, default: str | None = None) -> list[str]: """Read a JSON list and validate that every item is a string.""" raw_value = self.get(name, default) - # DATA was historically allowed to be present but empty. Treat that as - # an empty list while still requiring the environment variable itself. + # Treat an explicitly empty value as an empty list. if not raw_value: return [] value = json.loads(raw_value) diff --git a/src/helper_lib/external_needs.py b/src/helper_lib/external_needs.py new file mode 100644 index 000000000..f14500b0c --- /dev/null +++ b/src/helper_lib/external_needs.py @@ -0,0 +1,194 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* + +"""Shared parsing and path resolution for Bazel external documentation inputs.""" + +import json +from collections.abc import Mapping +from dataclasses import dataclass, replace +from pathlib import Path +from typing import TypeGuard, cast + + +@dataclass +class ExternalNeedsSource: + bazel_module: str + path_to_target: str + target: str + # True for a same-repo mount (`//pkg:needs_json`), whose runfiles live under + # `_main/…`. False for a cross-module mount (`@repo//…:needs_json`), whose + # runfiles live under `{bazel_module}+/…`. + is_local: bool = False + resolved_path: Path | None = None + + +def parse_bazel_external_need(s: str) -> ExternalNeedsSource | None: + is_cross_module = s.startswith("@") + is_local = s.startswith("//") + if not is_cross_module and not is_local: + # Local need, not external needs + return None + + if "//" not in s or ":" not in s: + raise ValueError( + f"Unsuported external data dependency: '{s}'. Must contain '//' & ':'" + ) + repo_and_path, target = s.split(":", 1) + repo, path_to_target = repo_and_path.split("//", 1) + repo = repo.lstrip("@") + + if target in ("needs_json", "needs_json_file", "docs_sources"): + return ExternalNeedsSource( + bazel_module=repo, + path_to_target=path_to_target, + target=target, + is_local=is_local, + ) + return None + + +def parse_external_needs_labels(labels: list[str]) -> list[ExternalNeedsSource]: + """Parse supported external-needs labels and ignore ordinary data labels.""" + return [ + source + for label in labels + if (source := parse_bazel_external_need(label)) is not None + ] + + +def _runfiles_module_dir(source: ExternalNeedsSource) -> str: + return "_main" if source.is_local else f"{source.bazel_module}+" + + +def external_needs_runfiles_path( + runfiles_dir: Path, source: ExternalNeedsSource, *suffix: str +) -> Path: + return ( + runfiles_dir + / _runfiles_module_dir(source) + / source.path_to_target + / Path(*suffix) + ) + + +def external_needs_source_path( + runfiles_dir: Path | None, source: ExternalNeedsSource +) -> Path: + """Return a source's resolved path or derive it from the runfiles root.""" + if source.resolved_path is not None: + return source.resolved_path + if runfiles_dir is None: + raise ValueError( + "An external needs source has no resolved path or runfiles root." + ) + + if source.target == "needs_json": + suffix = (source.target, "_build", "needs", "needs.json") + elif source.target == "needs_json_file": + suffix = ("needs.json",) + elif source.target == "docs_sources": + suffix = () + else: + raise ValueError(f"Unsupported external needs target: {source.target}") + return external_needs_runfiles_path(runfiles_dir, source, *suffix) + + +def resolve_external_needs_sources( + sources: list[ExternalNeedsSource], runfiles_dir: Path +) -> list[ExternalNeedsSource]: + """Attach absolute runfiles paths to parsed external-needs sources.""" + return [ + replace( + source, + resolved_path=external_needs_source_path(runfiles_dir, source), + ) + for source in sources + ] + + +def serialize_external_needs_sources( + sources: list[ExternalNeedsSource], +) -> str: + """Serialize external-needs descriptors for a Sphinx config value.""" + return json.dumps( + [ + { + "bazel_module": source.bazel_module, + "path_to_target": source.path_to_target, + "target": source.target, + "is_local": source.is_local, + "resolved_path": ( + str(source.resolved_path) + if source.resolved_path is not None + else None + ), + } + for source in sources + ] + ) + + +def _deserialize_external_needs_source( + value: Mapping[str, object], +) -> ExternalNeedsSource: + string_fields = ("bazel_module", "path_to_target", "target") + fields: dict[str, str] = {} + for field in string_fields: + field_value = value.get(field) + if not isinstance(field_value, str): + raise ValueError( + f"External needs descriptor field '{field}' must be a string." + ) + fields[field] = field_value + + is_local = value.get("is_local") + if not isinstance(is_local, bool): + raise ValueError( + "External needs descriptor field 'is_local' must be a boolean." + ) + + resolved_path = value.get("resolved_path") + if resolved_path is not None and not isinstance(resolved_path, str): + raise ValueError( + "External needs descriptor field 'resolved_path' must be a string or null." + ) + + return ExternalNeedsSource( + **fields, + is_local=is_local, + resolved_path=Path(resolved_path) if resolved_path else None, + ) + + +def _is_descriptor_list( + value: object, +) -> TypeGuard[list[Mapping[str, object]]]: + if not isinstance(value, list): + return False + items = cast(list[object], value) + return all(isinstance(item, Mapping) for item in items) + + +def parse_external_needs_source_config( + raw: str, +) -> list[ExternalNeedsSource]: + """Parse the descriptor JSON supplied by the documentation CLI.""" + if raw in ("", "[]"): + return [] + + value: object = json.loads(raw) + if not _is_descriptor_list(value): + raise ValueError( + "External needs configuration must contain descriptor objects." + ) + return [_deserialize_external_needs_source(item) for item in value] From c7f76601f973a9d6f0c4a36b20efd19328845d9c Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Fri, 18 Sep 2026 13:12:43 +0200 Subject: [PATCH 2/3] refactor: route external docs through score mounts --- src/docs_cli/cli.py | 26 +-- src/docs_cli/main_test.py | 36 +---- src/extensions/score_metamodel/__init__.py | 6 +- .../score_metamodel/external_needs.py | 52 ++---- .../tests/test_external_needs.py | 153 +++--------------- .../score_sphinx_bundle/__init__.py | 1 - src/helper_lib/external_needs.py | 109 +------------ src/requirements.in | 1 - src/requirements.txt | 55 +++---- 9 files changed, 67 insertions(+), 372 deletions(-) diff --git a/src/docs_cli/cli.py b/src/docs_cli/cli.py index e98cb7f5c..699cc67e1 100644 --- a/src/docs_cli/cli.py +++ b/src/docs_cli/cli.py @@ -15,6 +15,7 @@ import argparse import hashlib +import json import logging import os import shutil @@ -31,11 +32,6 @@ from src.extensions.score_mounts._resolver import load_mounts_manifest, resolve_walk_dir from src.helper_lib import Environment, get_runfiles_dir from src.helper_lib.config import DocsCliConfig -from src.helper_lib.external_needs import ( - parse_external_needs_labels, - resolve_external_needs_sources, - serialize_external_needs_sources, -) logger = logging.getLogger(__name__) @@ -45,7 +41,7 @@ def _build_external_needs_source_config() -> str: - """Build the JSON for Sphinx's ``external_needs_source`` configuration. + """Build the JSON label list for Sphinx's ``external_needs_source`` config. ``DATA`` contains all data dependencies of the documentation target, ``EXTERNAL_NEEDS_FILES`` contains explicitly declared external-needs @@ -53,21 +49,14 @@ def _build_external_needs_source_config() -> str: ``EXTERNAL_NEEDS_LABELS`` for its explicitly declared label list. All variables contain JSON arrays of Bazel labels. - The CLI combines those arrays, discards ordinary data dependencies, turns - supported external-needs labels into descriptors, resolves their paths in - the Bazel runfiles tree, and serializes the descriptors for the Sphinx - extension. If no runfiles directory is available, the descriptors retain - no resolved path and the extension resolves them through its own fallback - (for now). + The metamodel extension filters ordinary data dependencies and resolves + supported labels against the runfiles directory supplied as a separate + Sphinx configuration value. """ data = env.string_list("DATA", "[]") external = env.string_list("EXTERNAL_NEEDS_FILES", "[]") labels = env.string_list("EXTERNAL_NEEDS_LABELS", "[]") - sources = parse_external_needs_labels(data + external + labels) - runfiles_dir = env.optional_path("RUNFILES_DIR") - if runfiles_dir is not None: - sources = resolve_external_needs_sources(sources, runfiles_dir.absolute()) - return serialize_external_needs_sources(sources) + return json.dumps(data + external + labels) def _compute_hash(files: list[Path]) -> str: @@ -190,8 +179,7 @@ def sphinx_arguments( "-T", # show details in case of errors in extensions "--jobs", "auto", - # Convert Bazel data dependencies into descriptors consumed by the - # score_metamodel extension. + # Forward Bazel data dependencies to the score_metamodel extension. f"--define=external_needs_source={_build_external_needs_source_config()}", f"--define=testcase_source_dirs={env.get('TEST_SOURCES', '[]')}", # Path to the Bazel-emitted mounts manifest (empty when no mounts are diff --git a/src/docs_cli/main_test.py b/src/docs_cli/main_test.py index 78180bec0..9d3ca0acb 100644 --- a/src/docs_cli/main_test.py +++ b/src/docs_cli/main_test.py @@ -207,28 +207,7 @@ def test_bazel_configuration_resolves_runfiles_and_preserves_repo_relative_edit_ arguments = sphinx_arguments(DocsCliConfig.from_environment()) # Assert - external_needs = json.dumps( - [ - { - "bazel_module": "", - "path_to_target": "", - "target": "needs_json", - "is_local": True, - "resolved_path": str( - workspace / "runfiles/_main/needs_json/_build/needs/needs.json" - ), - }, - { - "bazel_module": "vendor", - "path_to_target": "", - "target": "needs_json", - "is_local": False, - "resolved_path": str( - workspace / "runfiles/vendor+/needs_json/_build/needs/needs.json" - ), - }, - ] - ) + external_needs = json.dumps(["//:needs_json", "@vendor//:needs_json"]) expected_arguments = { # Generated configuration and metamodel paths use the runfiles tree. "-c", @@ -289,18 +268,7 @@ def test_bazel_needs_action_uses_external_needs_labels_channel( arguments = sphinx_arguments(DocsCliConfig.from_environment()) # Assert - expected_source: dict[str, object] = { - "bazel_module": "", - "path_to_target": "", - "target": "needs_json", - "is_local": True, - "resolved_path": str( - workspace / "runfiles/_main/needs_json/_build/needs/needs.json" - ), - } - assert ( - f"--define=external_needs_source={json.dumps([expected_source])}" in arguments - ) + assert '--define=external_needs_source=["//:needs_json"]' in arguments def test_direct_invocation_resolves_paths_relative_to_cwd( diff --git a/src/extensions/score_metamodel/__init__.py b/src/extensions/score_metamodel/__init__.py index 83593785d..41847944f 100644 --- a/src/extensions/score_metamodel/__init__.py +++ b/src/extensions/score_metamodel/__init__.py @@ -285,11 +285,7 @@ def setup(app: Sphinx) -> dict[str, str | bool]: config_setdefault(app.config, "needs_reproducible_json", True) config_setdefault(app.config, "needs_json_remove_defaults", True) - # sphinx-collections runs on default prio 500. - # We need to populate the sphinx-collections config before that happens. - # If we put it anywhere higher it seems that other things already lock the needs - # To ensure that this runs first before locking happens priot is => 425 - # The lower the number the higher priority it has (runs earlier) + # Populate external Needs before Sphinx-Needs locks its configuration. _ = app.connect("config-inited", connect_external_needs, priority=425) discover_checks() diff --git a/src/extensions/score_metamodel/external_needs.py b/src/extensions/score_metamodel/external_needs.py index c3f36de6d..54ad90fc6 100644 --- a/src/extensions/score_metamodel/external_needs.py +++ b/src/extensions/score_metamodel/external_needs.py @@ -14,6 +14,7 @@ import json import subprocess from pathlib import Path +from typing import cast from sphinx.application import Sphinx from sphinx.config import Config @@ -26,7 +27,7 @@ external_needs_runfiles_path, external_needs_source_path as _external_needs_source_path, parse_bazel_external_need, - parse_external_needs_source_config as _parse_external_needs_source_config, + parse_external_needs_labels, ) logger = logging.getLogger(__name__) @@ -123,15 +124,26 @@ def temp(self: NeedsList): def get_external_needs_source(external_needs_source: str) -> list[ExternalNeedsSource]: if external_needs_source: - # The CLI supplies resolved descriptors for Bazel invocations. try: - return _parse_external_needs_source_config(external_needs_source) + raw_labels: object = json.loads(external_needs_source) except json.JSONDecodeError as e: logger.error( f"Failed to parse external needs sources from " f"external_needs_source {external_needs_source}: {e}" ) raise SystemExit(1) from e + if not isinstance(raw_labels, list): + raise ValueError( + "External needs configuration must contain Bazel label strings." + ) + labels: list[str] = [] + for label in cast(list[object], raw_labels): + if not isinstance(label, str): + raise ValueError( + "External needs configuration must contain Bazel label strings." + ) + labels.append(label) + return parse_external_needs_labels(labels) else: # This is the path taken for anything that doesn't # run via `bazel` e.g. esbonio or other direct executions @@ -163,30 +175,6 @@ def add_external_needs_json( ) -def add_external_docs_sources( - e: ExternalNeedsSource, config: Config, runfiles_dir: Path | None -): - # Note that bazel does NOT write the files under e.target! - # The runfiles layout mirrors the original git layout: same-repo mounts live - # under `_main/…`, cross-module mounts under `{e.bazel_module}+/…` - # (see _runfiles_module_dir). - docs_source_path = _external_needs_source_path(runfiles_dir, e) - - # A cross-module root mount keeps its module name as the collection key - # (unchanged). Sub-package / same-repo mounts disambiguate via the path. - key = "/".join(c for c in (e.bazel_module, e.path_to_target) if c) or "_main" - - if "collections" not in config: - config.collections = {} - config.collections[key] = { - "driver": "symlink", - "source": str(docs_source_path), - "target": key, - } - - logger.info(f"Added external docs source: {docs_source_path} -> {key}") - - def connect_external_needs(app: Sphinx, config: Config): # Local bundle exports intentionally omit the host URL from their JSON so # the inventory remains reusable by whichever documentation site consumes @@ -200,7 +188,7 @@ def connect_external_needs(app: Sphinx, config: Config): export_values={"project_url": ""} if bundle_export else None, ) - # External needs descriptors supplied by the documentation CLI. + # External needs labels supplied by the documentation CLI. external_needs = get_external_needs_source(app.config.external_needs_source) # this sets the default value - required for the needs-config-writer @@ -208,18 +196,12 @@ def connect_external_needs(app: Sphinx, config: Config): config.needs_external_needs = [] if external_needs: - runfiles_dir = ( - _runfiles_dir(app.config) - if any(source.resolved_path is None for source in external_needs) - else None - ) + runfiles_dir = _runfiles_dir(app.config) for e in external_needs: if e.target == "needs_json": add_external_needs_json(e, app.config, runfiles_dir) elif e.target == "needs_json_file": _add_needs_json_file(e, app.config, runfiles_dir) - elif e.target == "docs_sources": - add_external_docs_sources(e, app.config, runfiles_dir) else: raise ValueError( f"Internal Error. Unknown external needs target: {e.target}" diff --git a/src/extensions/score_metamodel/tests/test_external_needs.py b/src/extensions/score_metamodel/tests/test_external_needs.py index f9bff1c80..5e421de74 100644 --- a/src/extensions/score_metamodel/tests/test_external_needs.py +++ b/src/extensions/score_metamodel/tests/test_external_needs.py @@ -30,7 +30,6 @@ _external_needs_runfiles_path, # pyright: ignore[reportPrivateUsage] - white-box unit test _external_needs_source_path, # pyright: ignore[reportPrivateUsage] - white-box unit test _runfiles_dir, # pyright: ignore[reportPrivateUsage] - white-box unit test - add_external_docs_sources, add_external_needs_json, get_external_needs_source, ) @@ -89,16 +88,6 @@ def test_extend_needs_json_exporter_can_override_bundle_export_metadata( ("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( @@ -126,15 +115,6 @@ def test_external_needs_runfiles_path_is_environment_independent( ), Path("/runfiles/repo+/docs/needs.json"), ), - ( - ExternalNeedsSource( - bazel_module="", - path_to_target="docs", - target="docs_sources", - is_local=True, - ), - Path("/runfiles/_main/docs"), - ), ], ) def test_external_needs_source_path_selects_target_layout( @@ -159,18 +139,9 @@ def test_configured_runfiles_dir_is_used_for_external_sources( assert _runfiles_dir(config) == configured_runfiles -def test_descriptor_config_preserves_resolved_source_path() -> None: - resolved_path = Path("/runfiles/vendor+/needs_json/_build/needs/needs.json") +def test_external_needs_source_config_parses_labels() -> None: raw = json.dumps( - [ - { - "bazel_module": "vendor", - "path_to_target": "", - "target": "needs_json", - "is_local": False, - "resolved_path": str(resolved_path), - } - ] + ["@vendor//:needs_json"], ) sources = get_external_needs_source(raw) @@ -180,15 +151,26 @@ def test_descriptor_config_preserves_resolved_source_path() -> None: bazel_module="vendor", path_to_target="", target="needs_json", - resolved_path=resolved_path, ) ] - assert _external_needs_source_path(None, sources[0]) == resolved_path + assert _external_needs_source_path(Path("/runfiles"), sources[0]) == Path( + "/runfiles/vendor+/needs_json/_build/needs/needs.json" + ) -def test_external_needs_source_rejects_label_lists() -> None: - with pytest.raises(ValueError, match="descriptor objects"): - get_external_needs_source('["//:needs_json"]') +def test_external_needs_source_rejects_descriptors() -> None: + with pytest.raises(ValueError, match="Bazel label strings"): + get_external_needs_source( + json.dumps( + [ + { + "bazel_module": "vendor", + "path_to_target": "", + "target": "needs_json", + } + ] + ) + ) def test_add_external_needs_json_appends_entry( @@ -299,102 +281,3 @@ def test_add_external_needs_json_missing_file_keeps_list_empty( # Assert assert config.needs_external_needs == [] - - -def test_add_external_docs_sources_adds_collection( - tmp_path: Path, -) -> None: - """add_external_docs_sources should add one symlink collection entry.""" - e = ExternalNeedsSource( - bazel_module="third_party_docs", target="docs_sources", path_to_target="" - ) - config = Config() - config.collections = {} - - add_external_docs_sources(e, config, tmp_path) - - assert config.collections is not None - assert "third_party_docs" in config.collections - entry = config.collections["third_party_docs"] - assert entry["driver"] == "symlink" - assert entry["source"] == str(tmp_path / "third_party_docs+") - assert entry["target"] == "third_party_docs" - - -def test_add_external_docs_sources_local_sub_package( - tmp_path: Path, -) -> None: - """A same-repo sub-package `docs_sources` mount resolves under `_main/`. - - Mirrors test_add_external_needs_json_appends_entry_local but for the - `docs_sources` path: the local branch stages under `_main/…`, appends - `path_to_target`, and the collection key falls through the - `bazel_module + path_to_target` join (bazel_module empty for a local mount). - """ - e = ExternalNeedsSource( - bazel_module="", - target="docs_sources", - path_to_target="src/tests/e2e/external_needs/producer", - is_local=True, - ) - config = Config() - config.collections = {} - - add_external_docs_sources(e, config, tmp_path) - - assert config.collections is not None - key = "src/tests/e2e/external_needs/producer" - assert key in config.collections - entry = config.collections[key] - assert entry["driver"] == "symlink" - assert entry["source"] == str( - tmp_path / "_main" / "src/tests/e2e/external_needs/producer" - ) - assert entry["target"] == key - - -def test_add_external_docs_sources_local_root_key_fallback( - tmp_path: Path, -) -> None: - """A same-repo root `docs_sources` mount falls back to the `_main` key. - - With both bazel_module and path_to_target empty, the key join yields "" and - the `or "_main"` fallback names the collection, while the source stays at the - `_main` runfiles root. - """ - e = ExternalNeedsSource( - bazel_module="", - target="docs_sources", - path_to_target="", - is_local=True, - ) - config = Config() - config.collections = {} - - add_external_docs_sources(e, config, tmp_path) - - assert config.collections is not None - assert "_main" in config.collections - entry = config.collections["_main"] - assert entry["driver"] == "symlink" - assert entry["source"] == str(tmp_path / "_main") - assert entry["target"] == "_main" - - -def test_add_external_docs_sources_ide_support_uses_runfiles_tree() -> None: - """add_external_docs_sources should use the IDE fallback runfiles tree.""" - e = ExternalNeedsSource( - bazel_module="third_party_docs", target="docs_sources", path_to_target="" - ) - config = Config() - config.collections = {} - - add_external_docs_sources(e, config, Path("/tmp/ide_support.runfiles")) - - assert config.collections == { - "third_party_docs": { - "driver": "symlink", - "source": "/tmp/ide_support.runfiles/third_party_docs+", - "target": "third_party_docs", - } - } diff --git a/src/extensions/score_sphinx_bundle/__init__.py b/src/extensions/score_sphinx_bundle/__init__.py index 1628647fc..81b4eae47 100644 --- a/src/extensions/score_sphinx_bundle/__init__.py +++ b/src/extensions/score_sphinx_bundle/__init__.py @@ -35,7 +35,6 @@ "score_source_code_linker", "score_draw_uml_funcs", "score_layout", - "sphinx_collections", "sphinxcontrib.mermaid", "needs_config_writer", "score_sync_toml", diff --git a/src/helper_lib/external_needs.py b/src/helper_lib/external_needs.py index f14500b0c..b6f8cdd0f 100644 --- a/src/helper_lib/external_needs.py +++ b/src/helper_lib/external_needs.py @@ -13,11 +13,8 @@ """Shared parsing and path resolution for Bazel external documentation inputs.""" -import json -from collections.abc import Mapping -from dataclasses import dataclass, replace +from dataclasses import dataclass from pathlib import Path -from typing import TypeGuard, cast @dataclass @@ -29,7 +26,6 @@ class ExternalNeedsSource: # `_main/…`. False for a cross-module mount (`@repo//…:needs_json`), whose # runfiles live under `{bazel_module}+/…`. is_local: bool = False - resolved_path: Path | None = None def parse_bazel_external_need(s: str) -> ExternalNeedsSource | None: @@ -47,7 +43,7 @@ def parse_bazel_external_need(s: str) -> ExternalNeedsSource | None: repo, path_to_target = repo_and_path.split("//", 1) repo = repo.lstrip("@") - if target in ("needs_json", "needs_json_file", "docs_sources"): + if target in ("needs_json", "needs_json_file"): return ExternalNeedsSource( bazel_module=repo, path_to_target=path_to_target, @@ -84,111 +80,14 @@ def external_needs_runfiles_path( def external_needs_source_path( runfiles_dir: Path | None, source: ExternalNeedsSource ) -> Path: - """Return a source's resolved path or derive it from the runfiles root.""" - if source.resolved_path is not None: - return source.resolved_path + """Derive a source path from the Bazel runfiles root.""" if runfiles_dir is None: - raise ValueError( - "An external needs source has no resolved path or runfiles root." - ) + raise ValueError("An external needs source has no runfiles root.") if source.target == "needs_json": suffix = (source.target, "_build", "needs", "needs.json") elif source.target == "needs_json_file": suffix = ("needs.json",) - elif source.target == "docs_sources": - suffix = () else: raise ValueError(f"Unsupported external needs target: {source.target}") return external_needs_runfiles_path(runfiles_dir, source, *suffix) - - -def resolve_external_needs_sources( - sources: list[ExternalNeedsSource], runfiles_dir: Path -) -> list[ExternalNeedsSource]: - """Attach absolute runfiles paths to parsed external-needs sources.""" - return [ - replace( - source, - resolved_path=external_needs_source_path(runfiles_dir, source), - ) - for source in sources - ] - - -def serialize_external_needs_sources( - sources: list[ExternalNeedsSource], -) -> str: - """Serialize external-needs descriptors for a Sphinx config value.""" - return json.dumps( - [ - { - "bazel_module": source.bazel_module, - "path_to_target": source.path_to_target, - "target": source.target, - "is_local": source.is_local, - "resolved_path": ( - str(source.resolved_path) - if source.resolved_path is not None - else None - ), - } - for source in sources - ] - ) - - -def _deserialize_external_needs_source( - value: Mapping[str, object], -) -> ExternalNeedsSource: - string_fields = ("bazel_module", "path_to_target", "target") - fields: dict[str, str] = {} - for field in string_fields: - field_value = value.get(field) - if not isinstance(field_value, str): - raise ValueError( - f"External needs descriptor field '{field}' must be a string." - ) - fields[field] = field_value - - is_local = value.get("is_local") - if not isinstance(is_local, bool): - raise ValueError( - "External needs descriptor field 'is_local' must be a boolean." - ) - - resolved_path = value.get("resolved_path") - if resolved_path is not None and not isinstance(resolved_path, str): - raise ValueError( - "External needs descriptor field 'resolved_path' must be a string or null." - ) - - return ExternalNeedsSource( - **fields, - is_local=is_local, - resolved_path=Path(resolved_path) if resolved_path else None, - ) - - -def _is_descriptor_list( - value: object, -) -> TypeGuard[list[Mapping[str, object]]]: - if not isinstance(value, list): - return False - items = cast(list[object], value) - return all(isinstance(item, Mapping) for item in items) - - -def parse_external_needs_source_config( - raw: str, -) -> list[ExternalNeedsSource]: - """Parse the descriptor JSON supplied by the documentation CLI.""" - if raw in ("", "[]"): - return [] - - value: object = json.loads(raw) - if not _is_descriptor_list(value): - raise ValueError( - "External needs configuration must contain descriptor objects." - ) - return [_deserialize_external_needs_source(item) for item in value] diff --git a/src/requirements.in b/src/requirements.in index e3f3a8fcd..5d14d1a26 100644 --- a/src/requirements.in +++ b/src/requirements.in @@ -1,6 +1,5 @@ Sphinx sphinx-needs -sphinx-collections sphinxcontrib-plantuml pydata-sphinx-theme sphinx-design diff --git a/src/requirements.txt b/src/requirements.txt index 17c7f9a55..49b3c4874 100644 --- a/src/requirements.txt +++ b/src/requirements.txt @@ -27,10 +27,10 @@ babel==2.18.0 \ basedpyright==1.39.10 \ --hash=sha256:c8eaf5302f3265e275c7df4fba194d7afa7c1cb53fbfd448e90098360aca2c2e \ --hash=sha256:cbd75d83c0be841329bcfef2d2f1182f152a6d975b8eb199e75cf5b8e9a3de78 - # via -r requirements.in + # via -r src/requirements.in bazel-runfiles==2.3.2 \ --hash=sha256:1e69e329f824e05384cad02979e3a186da6c59b4424b805e15d41a386f157617 - # via -r requirements.in + # via -r src/requirements.in beautifulsoup4==4.15.0 \ --hash=sha256:288e3ca7d54b06f2ac191970bc275c1939cb46d450b255bf6718b04aa37ab4f7 \ --hash=sha256:d6f88de62e1d4e38ecb1077eb9724cd0eff29d2a08ca16a401e9b9e93f117cf9 @@ -403,7 +403,7 @@ debugpy==1.8.21 \ --hash=sha256:f843a8b08c2edeaf9b1582eed4f25441af21a297c22ff16bf76a662557aa9c9e \ --hash=sha256:fe0744a12353406de0ae8ccff0d0a4a666f00801a3db8fd04e7a5f761cd520e8 \ --hash=sha256:ffd932c6796afadab6993ec96745918a8cb2444dbd392074f769db5ea40ab440 - # via -r requirements.in + # via -r src/requirements.in docutils==0.22.4 \ --hash=sha256:4db53b1fde9abecbb74d91230d32ab626d94f6badfc575d6db9194a49df29968 \ --hash=sha256:d0013f540772d1420576855455d050a2180186c91c15779301ac2ccb3eeb68de @@ -463,14 +463,6 @@ fonttools==4.63.0 \ --hash=sha256:ef3048ef05dbb552b89817713d9cac912e00d0fde4a3105c00d29e52e10c89af \ --hash=sha256:fd1e3094f42d806d3d7c79162fc59e5910fcbe3a7360c385b8da969bc4493745 # via matplotlib -gitdb==4.0.12 \ - --hash=sha256:5ef71f855d191a3326fcfbc0d5da835f26b13fbcba60c32c21091c349ffdb571 \ - --hash=sha256:67073e15955400952c6565cc3e707c554a4eea2e428946f7a4c162fab9bd9bcf - # via gitpython -gitpython==3.1.59 \ - --hash=sha256:0a1475cfdc38a5bfba1a3e9a4a9da52a39749ecec322b772915c019f94e5b7e4 \ - --hash=sha256:67a82f537384578643624c8b2c531938a9b82be431663e575dcf638526631d4c - # via sphinx-collections h11==0.16.0 \ --hash=sha256:4e35b956cf45792e4caa5885e69fba00bdbc6ffafbfa020300e549b208ee5ff1 \ --hash=sha256:63cf8bbe7522de3bf65932fda1d9c2772064ffb3dae62d55932da54b31cb6c86 @@ -602,7 +594,6 @@ jinja2==3.1.6 \ # myst-parser # pydata-sphinx-theme # sphinx - # sphinx-collections # sphinxcontrib-mermaid jsonschema-rs==0.37.4 \ --hash=sha256:03b34f911e99343fc388651688683010daee538a3cf8cf86a7997bca28fdf16b \ @@ -908,11 +899,11 @@ minijinja==2.22.0 \ myst-parser==5.1.0 \ --hash=sha256:9c91c52b3cdb4d94a6506e4fab4e2f296c7623a0da0dcbe6de1565c3dad67a8a \ --hash=sha256:ab69322dc6719dcc7f296479dbb70181b66df6ed315064f92dbc85c0e1bf2f02 - # via -r requirements.in + # via -r src/requirements.in needs-config-writer==0.2.4 \ --hash=sha256:0f0702574081bb8ed7d896aadfb73c0e48af099dc0d4227cc2bac957ed8ea4f6 \ --hash=sha256:7c89375848c822e891b3cca48783f3cc3f7cbd3c02cba19418de146ca077f212 - # via -r requirements.in + # via -r src/requirements.in nodejs-wheel-binaries==24.16.0 \ --hash=sha256:2f3036292811514ba847b3708492644764f88a833ac425c5f55007014308ddfd \ --hash=sha256:3d0370fe7120ce9697a4f60d40480d2bd8808d9f30131458d5afc0040d4e5a51 \ @@ -1001,7 +992,6 @@ packaging==26.3 \ # matplotlib # pytest # sphinx - # sphinx-collections pillow==12.3.0 \ --hash=sha256:00808c5e14ef63ac5161091d242999076604ff74b883423a11e5d7bbb38bf756 \ --hash=sha256:04f01d28a6aaff387bf842a13be313df23ba0597a44f1a976c9feb3c6ff4711a \ @@ -1102,15 +1092,15 @@ pycparser==3.0 \ pydata-sphinx-theme==0.20.0 \ --hash=sha256:0da172d41e19a66de875f4002f7054b385372ec65763852193791e658d50bb4a \ --hash=sha256:56744483c9d72c783e075de716ab95d486108b69605df7528078090b73f11f69 - # via -r requirements.in + # via -r src/requirements.in pyfakefs==6.2.0 \ --hash=sha256:0968a49db692694ffed420e54a9f1cbae4636637b880e8ab09c8ccc0f11bd7ae \ --hash=sha256:e59a36db447bf509ce9c97ab3d1510c08cc51895c5311325a560a5e5b5dc1940 - # via -r requirements.in + # via -r src/requirements.in pygithub==2.10.0 \ --hash=sha256:192ada2a76e4afc7d6b37e500c9bfeba1731e6506697445a5ba1c4af8bf0b924 \ --hash=sha256:90ff24ef1cd1bd57124c2a3869cafee9d7b066909129ecdaba2c2d1903bc118d - # via -r requirements.in + # via -r src/requirements.in pygments==2.20.0 \ --hash=sha256:6757cd03768053ff99f3039c1a36d6c0aa0b263438fcab17520b30a303a82b5f \ --hash=sha256:81a9e26dd42fd28a23a2d169d86d7ac03b46e2f8b59ed4698fb4785f946d0176 @@ -1158,7 +1148,7 @@ pyparsing==3.3.2 \ pytest==9.1.1 \ --hash=sha256:1088fbde8f2b49d95a549a195707afa7a76a3ce9bcadc26b6d71f0ffda5fe313 \ --hash=sha256:37a86b45efb9a47a61a36449063e8e18d0cab3161329fc099eb21783169c4f0c - # via -r requirements.in + # via -r src/requirements.in python-dateutil==2.9.0.post0 \ --hash=sha256:37dd54208da7e1cd875388217d5e00ebd4179249f90fb72437e91a35459a0ad3 \ --hash=sha256:a8b2bc7bffae282281c8140a97d3aa9c14da0b136dfe83f850eea9a5f7470427 @@ -1256,7 +1246,7 @@ requests-file==2.1.0 \ rich==15.0.0 \ --hash=sha256:33bd4ef74232fb73fe9279a257718407f169c09b78a87ad3d296f548e27de0bb \ --hash=sha256:edd07a4824c6b40189fb7ac9bc4c52536e9780fbbfbddf6f1e2502c31b068c36 - # via -r requirements.in + # via -r src/requirements.in roman-numerals==4.1.0 \ --hash=sha256:1af8b147eb1405d5839e78aeb93131690495fe9da5c91856cb33ad55a7f1e5b2 \ --hash=sha256:647ba99caddc2cc1e55a51e4360689115551bf4476d90e8162cf8c345fe233c7 @@ -1264,15 +1254,11 @@ roman-numerals==4.1.0 \ ruamel-yaml==0.19.1 \ --hash=sha256:27592957fedf6e0b62f281e96effd28043345e0e66001f97683aa9a40c667c93 \ --hash=sha256:53eb66cd27849eff968ebf8f0bf61f46cdac2da1d1f3576dd4ccee9b25c31993 - # via -r requirements.in + # via -r src/requirements.in six==1.17.0 \ --hash=sha256:4721f391ed90541fddacab5acf947aa0d3dc7d27b2e1e8eda2be8970586c3274 \ --hash=sha256:ff70335d468e7eb6ec65b95b99d3a2836546063f63acc5171de367e834932a81 # via python-dateutil -smmap==5.0.3 \ - --hash=sha256:4d9debb8b99007ae47165abc08670bd74cb74b5227dda7f643eccc4e9eb5642c \ - --hash=sha256:c106e05d5a61449cf6ba9a1e650227ecfb141590d2a98412103ff35d89fc7b2f - # via gitdb snowballstemmer==3.1.1 \ --hash=sha256:7e207fa178741da09cdee59d3ecec3827ad5f92b1fc5c9ff3755b639f71f5752 \ --hash=sha256:e07bbc54a0d798fe6010a12398422e62a8bfbba95c394fd0956ef58cb4d3e260 @@ -1285,12 +1271,11 @@ sphinx==9.1.0 \ --hash=sha256:7741722357dd75f8190766926071fed3bdc211c74dd2d7d4df5404da95930ddb \ --hash=sha256:c84fdd4e782504495fe4f2c0b3413d6c2bf388589bb352d439b2a3bb99991978 # via - # -r requirements.in + # -r src/requirements.in # myst-parser # needs-config-writer # pydata-sphinx-theme # sphinx-autobuild - # sphinx-collections # sphinx-data-viewer # sphinx-design # sphinx-mounts @@ -1301,11 +1286,7 @@ sphinx==9.1.0 \ sphinx-autobuild==2025.8.25 \ --hash=sha256:9cf5aab32853c8c31af572e4fecdc09c997e2b8be5a07daf2a389e270e85b213 \ --hash=sha256:b750ac7d5a18603e4665294323fd20f6dcc0a984117026d1986704fa68f0379a - # via -r requirements.in -sphinx-collections==0.3.2 \ - --hash=sha256:42506b0ec456b70a63be1641a84b33399503f3233bfdc1d181775c7475466b85 \ - --hash=sha256:ab93ab151a045b9dd70a8ed6f8462f3f096c1a3a73bbe1ca8efbda093b5ac6da - # via -r requirements.in + # via -r src/requirements.in sphinx-data-viewer==0.1.5 \ --hash=sha256:a7d5e58613562bb745380bfe61ca8b69997998167fd6fa9aea55606c9a4b17e4 \ --hash=sha256:b74b1d304c505c464d07c7b225ed0d84ea02dcc88bc1c49cdad7c2275fbbdad4 @@ -1313,16 +1294,16 @@ sphinx-data-viewer==0.1.5 \ sphinx-design==0.7.0 \ --hash=sha256:d2a3f5b19c24b916adb52f97c5f00efab4009ca337812001109084a740ec9b7a \ --hash=sha256:f82bf179951d58f55dca78ab3706aeafa496b741a91b1911d371441127d64282 - # via -r requirements.in + # via -r src/requirements.in sphinx-mounts==0.1.4 \ --hash=sha256:3369794f5a835d87e771d1c3075fdef95fd4ab7d1b6c6afb30a4abfd48102b54 \ --hash=sha256:c88098b57891a8046965c3987f1246af9457cae962ba28b658ffab5f44ed2af7 - # via -r requirements.in + # via -r src/requirements.in sphinx-needs[plotting]==8.3.1 \ --hash=sha256:cca706698cbb0c2fac5c8bf21fc30cd825473f97123783713cf5c9a9d01216c2 \ --hash=sha256:ecc9807c06dd3698ecf78d3a6c0867a3bbaed56204f8cbc21fdd3e1282c2d2bb # via - # -r requirements.in + # -r src/requirements.in # needs-config-writer sphinxcontrib-applehelp==2.0.0 \ --hash=sha256:2f29ef331735ce958efa4734873f084941970894c6090408b079c61b2e1c06d1 \ @@ -1347,10 +1328,10 @@ sphinxcontrib-jsmath==1.0.1 \ sphinxcontrib-mermaid==2.1.0 \ --hash=sha256:13c5f9ac395cb6abf403eca34e228dc9fb3a30c9d960dbf3e40e9a8cef969549 \ --hash=sha256:417cd144ec4b28852f46ba653f02ce8e538881c812111671a4c30344e87f2112 - # via -r requirements.in + # via -r src/requirements.in sphinxcontrib-plantuml==0.31 \ --hash=sha256:fd74752f8ea070e641c3f8a402fccfa1d4a4056e0967b56033d2a76282d9f956 - # via -r requirements.in + # via -r src/requirements.in sphinxcontrib-qthelp==2.0.0 \ --hash=sha256:4fe7d0ac8fc171045be623aba3e2a8f613f8682731f9153bb2e40ece16b9bbab \ --hash=sha256:b18a828cdba941ccd6ee8445dbe72ffa3ef8cbe7505d8cd1fa0d42d3f2d5f3eb From 91f685bd6f59cd074a3dfc280448e187abe6375c Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Fri, 18 Sep 2026 14:09:27 +0200 Subject: [PATCH 3/3] test: cover external needs label parsing --- .../tests/test_external_needs.py | 27 +++++++++++++++++++ .../score_source_code_linker/xml_parser.py | 2 +- src/helper_lib/external_needs.py | 2 +- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/extensions/score_metamodel/tests/test_external_needs.py b/src/extensions/score_metamodel/tests/test_external_needs.py index 5e421de74..2b7b33677 100644 --- a/src/extensions/score_metamodel/tests/test_external_needs.py +++ b/src/extensions/score_metamodel/tests/test_external_needs.py @@ -36,6 +36,11 @@ from sphinx.config import Config from sphinx_needs.needsfile import NeedsList +from src.helper_lib.external_needs import ( + parse_bazel_external_need, + parse_external_needs_labels, +) + def test_extend_needs_json_exporter_uses_configured_value( monkeypatch: pytest.MonkeyPatch, @@ -96,6 +101,28 @@ def test_external_needs_runfiles_path_is_environment_independent( assert _external_needs_runfiles_path(Path("/runfiles"), source, *suffix) == expected +def test_parse_bazel_external_need_marks_same_repository_labels_local() -> None: + assert parse_bazel_external_need("//pkg:needs_json") == ExternalNeedsSource( + bazel_module="", + path_to_target="pkg", + target="needs_json", + is_local=True, + ) + + +def test_parse_external_needs_labels_filters_ordinary_data_labels() -> None: + assert parse_external_needs_labels( + ["docs/index.rst", "//pkg:needs_json", "assets/logo.svg"] + ) == [ + ExternalNeedsSource( + bazel_module="", + path_to_target="pkg", + target="needs_json", + is_local=True, + ) + ] + + @pytest.mark.parametrize( ("source", "expected"), [ diff --git a/src/extensions/score_source_code_linker/xml_parser.py b/src/extensions/score_source_code_linker/xml_parser.py index f20a6006e..8e39a8f41 100644 --- a/src/extensions/score_source_code_linker/xml_parser.py +++ b/src/extensions/score_source_code_linker/xml_parser.py @@ -60,7 +60,7 @@ def parse_testcase_source_dirs(v: str) -> list[str]: The value arrives as a `str(list)` produced by Starlark (double-quoted, i.e. valid JSON), mirroring how `external_needs_source` is parsed in - ` score_metamodel.external_needs.get_external_needs_source`. + `score_metamodel.external_needs.get_external_needs_source`. """ if v in ("[]", ""): return [] diff --git a/src/helper_lib/external_needs.py b/src/helper_lib/external_needs.py index b6f8cdd0f..8e8fd8a69 100644 --- a/src/helper_lib/external_needs.py +++ b/src/helper_lib/external_needs.py @@ -37,7 +37,7 @@ def parse_bazel_external_need(s: str) -> ExternalNeedsSource | None: if "//" not in s or ":" not in s: raise ValueError( - f"Unsuported external data dependency: '{s}'. Must contain '//' & ':'" + f"Unsupported external data dependency: '{s}'. Must contain '//' & ':'" ) repo_and_path, target = s.split(":", 1) repo, path_to_target = repo_and_path.split("//", 1)