From a8dcbb7dbe9b4497aebb703f0dbe59cbafc6f905 Mon Sep 17 00:00:00 2001 From: subramaniak Date: Wed, 1 Jul 2026 07:00:31 +0000 Subject: [PATCH 01/14] feat: add DR-008 resolved-dependency resolve + override mechanism --- scripts/known_good/models/module.py | 3 + scripts/known_good/resolved_dependencies.py | 421 ++++++++++++++++++ .../tests/test_resolved_dependencies.py | 289 ++++++++++++ .../update_module_from_known_good.py | 142 +++--- 4 files changed, 795 insertions(+), 60 deletions(-) create mode 100644 scripts/known_good/resolved_dependencies.py create mode 100644 scripts/known_good/tests/test_resolved_dependencies.py diff --git a/scripts/known_good/models/module.py b/scripts/known_good/models/module.py index 72cae75c678..de01225a368 100644 --- a/scripts/known_good/models/module.py +++ b/scripts/known_good/models/module.py @@ -36,6 +36,7 @@ class Metadata: exclude_test_targets: list[str] = field(default_factory=lambda: []) langs: list[str] = field(default_factory=lambda: ["cpp", "rust"]) rust_coverage_config: str | None = "ferrocene-coverage" # Optional field for Rust coverage configuration + bazel_config: list[str] = field(default_factory=lambda: []) @classmethod def from_dict(cls, data: Dict[str, Any]) -> Metadata: @@ -53,6 +54,7 @@ def from_dict(cls, data: Dict[str, Any]) -> Metadata: exclude_test_targets=data.get("exclude_test_targets", []), langs=data.get("langs", ["cpp", "rust"]), rust_coverage_config=data.get("rust_coverage_config", "ferrocene-coverage"), + bazel_config=data.get("bazel_config", []), ) def to_dict(self) -> Dict[str, Any]: @@ -67,6 +69,7 @@ def to_dict(self) -> Dict[str, Any]: "exclude_test_targets": self.exclude_test_targets, "langs": self.langs, "rust_coverage_config": self.rust_coverage_config, + "bazel_config": self.bazel_config, } diff --git a/scripts/known_good/resolved_dependencies.py b/scripts/known_good/resolved_dependencies.py new file mode 100644 index 00000000000..6baebf85cdf --- /dev/null +++ b/scripts/known_good/resolved_dependencies.py @@ -0,0 +1,421 @@ +#!/usr/bin/env python3 +# ******************************************************************************* +# 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 +# ******************************************************************************* +"""Resolved dependency versions from the reference_integration root. + +DR-008 Option 4 requires that the dependency versions ``reference_integration`` +resolves are pushed *into* each module so the module's own unit tests + coverage +run against the resolved set (not against the versions the module declares in its +released ``MODULE.bazel``). + +This module provides :class:`ResolvedDependencies`, which: + +* holds the resolved version/commit per dependency (sourced from ref_int's root — + either ``known_good.json`` for local runs, or the Stage-1 ``stage1-resolved-deps`` + artifact for CI runs so the resolution flows Stage 1 -> Stage 2), and +* exposes an interface to **scan** an individual module's ``MODULE.bazel`` and + **overwrite** the declared dependency versions to match the resolved set, by + appending the matching ``git_override`` / ``single_version_override`` directives. + +The injection is append-only and operates on the CI checkout of the module — it is +never committed back to the module's released sources (DR-008 "temporary mechanism"). +""" + +from __future__ import annotations + +import argparse +import json +import logging +import re +import sys +from pathlib import Path +from typing import Dict, List, Optional + +# Import ``models`` + ``generate_override_directive`` whether this file is loaded as +# ``known_good.resolved_dependencies`` (scripts/ on path, e.g. from quality_runners.py) +# or as ``resolved_dependencies`` (scripts/known_good/ on path). Preferring the +# package-qualified form keeps a single ``Module`` class identity in the package context. +_HERE = Path(__file__).resolve().parent +try: + from known_good.models.known_good import load_known_good + from known_good.models.module import Module + from known_good.update_module_from_known_good import generate_override_directive +except ImportError: + if str(_HERE) not in sys.path: + sys.path.insert(0, str(_HERE)) + from models.known_good import load_known_good # noqa: E402 + from models.module import Module # noqa: E402 + from update_module_from_known_good import generate_override_directive # noqa: E402 + +# Marker delimiting the block we append, so injection is idempotent / detectable. +INJECTION_BEGIN = "# --- BEGIN ref_int resolved-deps injection (DR-008 Option 4) ---" +INJECTION_END = "# --- END ref_int resolved-deps injection (DR-008 Option 4) ---" + +# The single file that carries the resolved set from Stage 1 (resolve) to Stage 2 +# (per-module validation). It is the only handoff needed: first-party commits + +# third-party resolved versions, merged. The lock travels alongside only as evidence. +MANIFEST_NAME = "resolved_versions.json" + +# Built-in / non-registry modules that must not be given a single_version_override. +_SKIP_MODULES = {"bazel_tools"} + +# Capture the module name from any ``bazel_dep(name = "...")`` call (name is the first arg). +_BAZEL_DEP_RE = re.compile(r'bazel_dep\(\s*name\s*=\s*"([^"]+)"') +# Capture an existing override target so we don't inject a duplicate for the same module. +_OVERRIDE_RE = re.compile( + r'(?:git_override|single_version_override|local_path_override|archive_override)\(\s*module_name\s*=\s*"([^"]+)"' +) +# Parsers for reconstructing the resolved set from generated score_modules_*.MODULE.bazel. +_GIT_OVERRIDE_BLOCK_RE = re.compile(r"git_override\((?P.*?)\)", re.S) +_SINGLE_VERSION_BLOCK_RE = re.compile(r"single_version_override\((?P.*?)\)", re.S) +_FIELD_RE = lambda field: re.compile(rf'{field}\s*=\s*"([^"]+)"') # noqa: E731 + + +class ResolvedDependencies: + """Resolved dependency versions from the reference_integration root. + + Holds a ``name -> Module`` map of the dependencies ref_int pins, and provides an + interface to scan + overwrite a module's ``MODULE.bazel`` to those versions. + """ + + def __init__(self, resolved: Dict[str, Module]): + self._resolved = resolved + + # -- construction: "resolved deps versions from ref_int root" -------------------- + + @classmethod + def from_known_good(cls, known_good_path: Path) -> "ResolvedDependencies": + """Build from ``known_good.json`` (local / dev source of the resolved pins).""" + kg = load_known_good(Path(known_good_path).resolve()) + resolved: Dict[str, Module] = {} + for group in kg.modules.values(): + for module in group.values(): + resolved[module.name] = module + return cls(resolved) + + @classmethod + def from_resolved_artifact(cls, artifact_dir: Path) -> "ResolvedDependencies": + """Build from the Stage-1 ``stage1-resolved-deps`` artifact. + + The handoff is the single ``resolved_versions.json`` manifest (see + :meth:`from_mod_graph` / :meth:`to_file`). For backward compatibility, if the + manifest is absent the older format is parsed: the generated + ``score_modules_*.MODULE.bazel`` override files, gated on the presence of + ``MODULE.bazel.lock`` as evidence of full resolution. + """ + artifact_dir = Path(artifact_dir) + + manifest = artifact_dir / MANIFEST_NAME + if manifest.is_file(): + return cls.from_file(manifest) + + # Legacy fallback: reconstruct from the generated override files. + lock = artifact_dir / "MODULE.bazel.lock" + if not lock.is_file(): + raise FileNotFoundError( + f"Neither {MANIFEST_NAME} nor MODULE.bazel.lock found in resolved-deps artifact " + f"{artifact_dir}; Stage 2 must consume the Stage-1 resolved dependency set." + ) + + module_files = sorted(artifact_dir.glob("score_modules_*.MODULE.bazel")) + if not module_files: + raise FileNotFoundError(f"No score_modules_*.MODULE.bazel files in resolved-deps artifact {artifact_dir}.") + + resolved: Dict[str, Module] = {} + for mf in module_files: + for module in cls._parse_override_file(mf.read_text()): + resolved[module.name] = module + return cls(resolved) + + @classmethod + def from_mod_graph(cls, mod_graph_json: Path, override_files: List[Path]) -> "ResolvedDependencies": + """Build the *complete* resolved set by merging two sources. + + * The override directives ref_int actually declares — parsed from its root + ``MODULE.bazel`` and the ``bazel_common/*.MODULE.bazel`` files it ``include()``s. + This carries every module ref_int pins by a non-registry source as its real + directive: ``git_override(commit, remote)`` for ``score_*`` plus third-party like + ``trlc`` / ``flatbuffers`` / ``rules_oci``, and ``single_version_override`` where + ref_int pins a registry version. The graph cannot supply these — it reports + overridden modules as version ``0.0.0``. + * ``bazel mod graph --output=json`` — the post-MVS resolved version of every other + (registry) module (protobuf, abseil, rules_rust, ...), emitted as + ``single_version_override`` so each module under test is forced to the exact + version ref_int resolved (MVS is graph-global, so a module's own subgraph could + otherwise select a different version). + + ``archive_override`` / ``local_path_override`` targets (e.g. ``rules_boost``) cannot + be represented and are logged as not carried. + """ + resolved: Dict[str, Module] = {} + unrepresentable: List[str] = [] + for f in override_files: + # Drop comment-only lines first: hand-written MODULE.bazel files contain + # commented-out overrides (e.g. "# git_override(... rules_rpm ...)") that must + # not be captured. Inline trailing comments (after a value) are left intact. + text = "\n".join(ln for ln in Path(f).read_text().splitlines() if not ln.lstrip().startswith("#")) + for module in cls._parse_override_file(text): # git_override + single_version_override + resolved[module.name] = module + for m in re.finditer(r'(archive_override|local_path_override)\(\s*module_name\s*=\s*"([^"]+)"', text): + unrepresentable.append(f"{m.group(2)} ({m.group(1)})") + + graph = json.loads(Path(mod_graph_json).read_text()) + versions: Dict[str, str] = {} + _collect_resolved_versions(graph, versions) + skipped: List[str] = [] + for name, version in versions.items(): + if name in resolved or name in _SKIP_MODULES: + continue # already carried by an override directive, or non-overridable + if not version or version == "0.0.0": + # Non-registry version: ref_int pins it via an override we did not capture + # (e.g. archive_override). single_version_override cannot reproduce it. + skipped.append(name) + continue + resolved[name] = Module(name=name, hash="", repo="", version=version) + + if unrepresentable: + logging.warning( + "Overrides not carried into manifest (need manual handling): %s", ", ".join(unrepresentable) + ) + if skipped: + logging.warning( + "Graph modules at version 0.0.0 with no carried override, skipped: %s", ", ".join(sorted(skipped)) + ) + return cls(resolved) + + def to_file(self, path: Path) -> None: + """Serialize the resolved set to the JSON manifest (Stage 1 -> Stage 2 handoff). + + Only the fields needed to regenerate the override directive are stored + (``version`` for single_version_override; ``repo`` + ``hash`` for git_override). + Metadata is intentionally omitted — the manifest carries dependency pins, not the + module-under-test's test configuration (that comes from known_good.json). + """ + modules = {} + for name in sorted(self._resolved): + m = self._resolved[name] + entry: Dict[str, object] = {"version": m.version} if m.version else {"repo": m.repo, "hash": m.hash} + if m.bazel_patches: + entry["bazel_patches"] = m.bazel_patches + modules[name] = entry + Path(path).write_text(json.dumps({"modules": modules}, indent=2) + "\n") + + @classmethod + def from_file(cls, path: Path) -> "ResolvedDependencies": + """Load a resolved set previously written by :meth:`to_file`.""" + data = json.loads(Path(path).read_text()) + resolved = {name: Module.from_dict(name, md) for name, md in data.get("modules", {}).items()} + return cls(resolved) + + @staticmethod + def _parse_override_file(text: str) -> List[Module]: + """Reconstruct Module objects from generated git/single_version override blocks.""" + modules: List[Module] = [] + + for match in _GIT_OVERRIDE_BLOCK_RE.finditer(text): + body = match.group("body") + name = _field(body, "module_name") + commit = _field(body, "commit") + remote = _field(body, "remote") + if name and commit and remote: + modules.append(Module(name=name, hash=commit, repo=remote)) + + for match in _SINGLE_VERSION_BLOCK_RE.finditer(text): + body = match.group("body") + name = _field(body, "module_name") + version = _field(body, "version") + if name and version: + modules.append(Module(name=name, hash="", repo="", version=version)) + + return modules + + # -- interface: scan + overwrite a module's MODULE.bazel ------------------------- + + @property + def names(self) -> set[str]: + return set(self._resolved) + + def get(self, name: str) -> Optional[Module]: + return self._resolved.get(name) + + def scan(self, module_bazel: Path) -> List[str]: + """Return the names of dependencies a module declares via ``bazel_dep``.""" + text = Path(module_bazel).read_text() + # Ignore anything inside a previous injection block so re-scans are stable. + text = self._strip_injection(text) + return _BAZEL_DEP_RE.findall(text) + + def overwrite(self, module_bazel: Path, *, module_under_test: Optional[str] = None, write: bool = True) -> str: + """Overwrite a module's declared dependency versions with the resolved set. + + Appends a ``git_override`` / ``single_version_override`` directive for every + dependency the module declares that we have a resolved version for, so the + module (and all its transitive deps) build against ref_int's resolved versions. + + * Skips the module under test itself (the root is never overridden). + * Skips dependencies that already carry an override in the file. + * Re-running is idempotent: a prior injection block is replaced. + """ + module_bazel = Path(module_bazel) + original = self._strip_injection(module_bazel.read_text()) + + declared = set(_BAZEL_DEP_RE.findall(original)) + already_overridden = set(_OVERRIDE_RE.findall(original)) + + from dataclasses import replace as _replace + + directives: List[str] = [] + # Inject overrides only for deps the module actually declares (intersected with the + # resolved set). Bazel fails with "root module specifies overrides on nonexistent + # module(s)" if an override targets a module that is not in this module's dependency + # graph, so the full resolved set cannot be injected wholesale — a declared bazel_dep + # is by definition in the graph, which makes its override safe. + for name in sorted(declared): + if name == module_under_test: + continue # the module under test is the root; never override it + if name in already_overridden: + continue # respect an override the module already declares + module = self._resolved.get(name) + if module is None: + continue # dep ref_int does not pin; resolves normally + # Strip bazel_patches: they reference //patches/... labels in ref_int's + # workspace which do not exist inside another module's checkout. + module = _replace(module, bazel_patches=None) + directive = generate_override_directive(module) + if directive is None: + continue + directives.append(directive) + + if not directives: + patched = original + else: + body = "\n".join(directives) + patched = f"{original.rstrip()}\n\n{INJECTION_BEGIN}\n{body}\n{INJECTION_END}\n" + + if write: + module_bazel.write_text(patched) + return patched + + @staticmethod + def _strip_injection(text: str) -> str: + """Remove a previously appended injection block, if present.""" + pattern = re.compile( + re.escape(INJECTION_BEGIN) + r".*?" + re.escape(INJECTION_END) + r"\n?", + re.S, + ) + return pattern.sub("", text).rstrip() + "\n" if pattern.search(text) else text + + +def _field(body: str, field: str) -> str: + match = _FIELD_RE(field).search(body) + return match.group(1) if match else "" + + +def _collect_resolved_versions(node: dict, acc: Dict[str, str]) -> None: + """Walk a ``bazel mod graph --output=json`` tree, recording name -> resolved version. + + Each node carries the post-MVS ``name`` and ``version``; a module can appear many + times in the graph but always at the single resolved version, so deduping by name is + safe. The ```` node has an empty version and is skipped implicitly. + """ + for dep in node.get("dependencies", []): + name, version = dep.get("name"), dep.get("version") + if name and version: + acc[name] = version + _collect_resolved_versions(dep, acc) + + +def _parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser( + description="Resolve (Stage 1) or inject (Stage 2) ref_int's resolved dependency set (DR-008 Option 4)." + ) + parser.add_argument( + "module_bazel", + type=Path, + nargs="?", + default=None, + help="Inject mode: path to the module's MODULE.bazel to overwrite. Omit when using --export.", + ) + parser.add_argument( + "--known-good-path", + type=Path, + default=_HERE.parents[1] / "known_good.json", + help="Resolved set source: known_good.json (default; first-party commit pins).", + ) + parser.add_argument( + "--resolved-deps", + type=Path, + default=None, + help="Inject mode: Stage-1 stage1-resolved-deps artifact dir (overrides --known-good-path).", + ) + parser.add_argument( + "--mod-graph", + type=Path, + default=None, + help="Export mode: 'bazel mod graph --output=json' output, merged with known_good.json.", + ) + parser.add_argument( + "--export", + type=Path, + default=None, + help=f"Export mode: write the merged resolved set to this {MANIFEST_NAME} manifest and exit.", + ) + parser.add_argument( + "--module-under-test", + default=None, + help="Name of the module under test (never overridden as it is the root).", + ) + parser.add_argument("--dry-run", action="store_true", help="Print patched content instead of writing.") + return parser.parse_args() + + +def main() -> None: + args = _parse_args() + + # Export mode (Stage 1): build the manifest by merging the override directives ref_int + # declares (root MODULE.bazel + bazel_common/*.MODULE.bazel) with the resolved registry + # versions from 'bazel mod graph'. + if args.export is not None: + if args.mod_graph is None: + raise SystemExit("--export requires --mod-graph (output of 'bazel mod graph --output=json')") + repo_root = _HERE.parents[1] + override_files = [repo_root / "MODULE.bazel", *sorted((repo_root / "bazel_common").glob("*.MODULE.bazel"))] + override_files = [f for f in override_files if f.is_file()] + resolved = ResolvedDependencies.from_mod_graph(args.mod_graph, override_files) + Path(args.export).parent.mkdir(parents=True, exist_ok=True) + resolved.to_file(args.export) + print(f"Wrote resolved dependency manifest ({len(resolved.names)} modules) to {args.export}") + return + + # Inject mode (Stage 2): overwrite a module's MODULE.bazel with the resolved set. + if args.module_bazel is None: + raise SystemExit("module_bazel is required unless --export is given") + + if args.resolved_deps: + resolved = ResolvedDependencies.from_resolved_artifact(args.resolved_deps) + else: + resolved = ResolvedDependencies.from_known_good(args.known_good_path) + + patched = resolved.overwrite( + args.module_bazel, + module_under_test=args.module_under_test, + write=not args.dry_run, + ) + if args.dry_run: + print(patched) + else: + print(f"Injected resolved-deps overrides into {args.module_bazel}") + + +if __name__ == "__main__": + main() diff --git a/scripts/known_good/tests/test_resolved_dependencies.py b/scripts/known_good/tests/test_resolved_dependencies.py new file mode 100644 index 00000000000..37608b76004 --- /dev/null +++ b/scripts/known_good/tests/test_resolved_dependencies.py @@ -0,0 +1,289 @@ +# ******************************************************************************* +# 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 +# ******************************************************************************* +"""Unit tests for ResolvedDependencies (DR-008 Option 4 dependency injection). + +Self-contained: builds the resolved set from a temporary known_good.json and +overwrites a temporary module MODULE.bazel — no cloned repos or Bazel required. +""" + +import json +import sys +from pathlib import Path + +import pytest + +# Make scripts/known_good importable when run via plain pytest. +_KG_DIR = Path(__file__).resolve().parents[1] +if str(_KG_DIR) not in sys.path: + sys.path.insert(0, str(_KG_DIR)) + +from resolved_dependencies import ( # noqa: E402 + INJECTION_BEGIN, + INJECTION_END, + ResolvedDependencies, +) + +KNOWN_GOOD = { + "modules": { + "target_sw": { + "score_baselibs": { + "repo": "https://github.com/eclipse-score/baselibs.git", + "hash": "cab36dd7de92aaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "bazel_patches": ["patches/baselibs/001-fix.patch"], + }, + "score_logging": { + "repo": "https://github.com/eclipse-score/logging.git", + "hash": "0e9187f79a99bbbbbbbbbbbbbbbbbbbbbbbbbbbb", + }, + "score_persistency": { + "repo": "https://github.com/eclipse-score/persistency.git", + "hash": "4d1fa1ae3c55cccccccccccccccccccccccccccc", + }, + }, + "tooling": { + "score_tooling": { + "repo": "https://github.com/eclipse-score/tooling.git", + "version": "1.2.0", + }, + }, + }, + "timestamp": "2026-01-01T00:00:00+00:00Z", +} + +MODULE_BAZEL = """\ +module(name = "score_persistency", version = "0.0.0") + +bazel_dep(name = "rules_cc", version = "0.2.17") +bazel_dep(name = "score_baselibs", version = "0.2.7") +bazel_dep(name = "score_logging", version = "0.2.0") +bazel_dep(name = "score_tooling", version = "1.0.0") +bazel_dep(name = "score_unpinned", version = "9.9.9") +""" + + +@pytest.fixture +def known_good_file(tmp_path: Path) -> Path: + p = tmp_path / "known_good.json" + p.write_text(json.dumps(KNOWN_GOOD)) + return p + + +@pytest.fixture +def module_bazel(tmp_path: Path) -> Path: + p = tmp_path / "MODULE.bazel" + p.write_text(MODULE_BAZEL) + return p + + +@pytest.fixture +def resolved(known_good_file: Path) -> ResolvedDependencies: + return ResolvedDependencies.from_known_good(known_good_file) + + +class TestFromKnownGood: + def test_names_span_all_groups(self, resolved: ResolvedDependencies): + assert {"score_baselibs", "score_logging", "score_persistency", "score_tooling"} <= resolved.names + + def test_get_returns_resolved_commit(self, resolved: ResolvedDependencies): + assert resolved.get("score_baselibs").hash.startswith("cab36dd7de92") + + def test_version_module_kept(self, resolved: ResolvedDependencies): + assert resolved.get("score_tooling").version == "1.2.0" + + +class TestScan: + def test_returns_declared_deps(self, resolved: ResolvedDependencies, module_bazel: Path): + declared = resolved.scan(module_bazel) + assert "score_baselibs" in declared + assert "score_unpinned" in declared + assert "rules_cc" in declared + + +class TestOverwrite: + def test_pins_declared_resolved_siblings(self, resolved: ResolvedDependencies, module_bazel: Path): + patched = resolved.overwrite(module_bazel, module_under_test="score_persistency", write=False) + block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] + assert 'git_override(\n module_name = "score_baselibs"' in block + assert 'commit = "cab36dd7de92aaaaaaaaaaaaaaaaaaaaaaaaaaaa"' in block + # version module -> single_version_override + assert 'single_version_override(\n module_name = "score_tooling"' in block + assert 'version = "1.2.0"' in block + + def test_strips_patches(self, resolved: ResolvedDependencies, module_bazel: Path): + # bazel_patches reference //patches/... labels that exist only in ref_int's + # workspace, so they are stripped from the injected overrides. + patched = resolved.overwrite(module_bazel, module_under_test="score_persistency", write=False) + assert "patches/baselibs/001-fix.patch" not in patched + assert "patch_strip" not in patched + + def test_skips_resolved_dep_not_declared(self, resolved: ResolvedDependencies, tmp_path: Path): + # Only declared deps are injected. Overriding a module that is NOT in the module's + # dependency graph makes Bazel fail ("overrides on nonexistent module(s)"), so a + # resolved dep the module does not declare must NOT be injected. + mod = tmp_path / "MODULE.bazel" + mod.write_text( + 'module(name = "score_persistency", version = "0.0.0")\nbazel_dep(name = "score_baselibs", version = "0.1")\n' + ) + block = resolved.overwrite(mod, module_under_test="score_persistency", write=False).split(INJECTION_BEGIN)[1] + assert 'module_name = "score_baselibs"' in block # declared -> injected + assert 'module_name = "score_logging"' not in block # not declared -> not injected + + def test_skips_root_module(self, resolved: ResolvedDependencies, module_bazel: Path): + patched = resolved.overwrite(module_bazel, module_under_test="score_persistency", write=False) + block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] + assert 'module_name = "score_persistency"' not in block + + def test_skips_unpinned_third_party(self, resolved: ResolvedDependencies, module_bazel: Path): + patched = resolved.overwrite(module_bazel, module_under_test="score_persistency", write=False) + block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] + assert "score_unpinned" not in block + assert "rules_cc" not in block + + def test_idempotent(self, resolved: ResolvedDependencies, module_bazel: Path): + first = resolved.overwrite(module_bazel, module_under_test="score_persistency", write=True) + second = resolved.overwrite(module_bazel, module_under_test="score_persistency", write=True) + assert first == second + assert second.count(INJECTION_BEGIN) == 1 + + def test_skips_dep_with_existing_override(self, resolved: ResolvedDependencies, tmp_path: Path): + mod = tmp_path / "MODULE.bazel" + mod.write_text( + MODULE_BAZEL + '\ngit_override(\n module_name = "score_logging",\n commit = "deadbeef",\n' + ' remote = "https://example.com/x.git",\n)\n' + ) + patched = resolved.overwrite(mod, module_under_test="score_persistency", write=False) + block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] + assert 'module_name = "score_logging"' not in block # respected pre-existing override + + +class TestMetadataBazelConfig: + def test_bazel_config_roundtrip(self): + from models.module import Metadata + + m = Metadata.from_dict({"bazel_config": ["bl-x86_64-linux"]}) + assert m.bazel_config == ["bl-x86_64-linux"] + assert m.to_dict()["bazel_config"] == ["bl-x86_64-linux"] + + def test_bazel_config_default_empty(self): + from models.module import Metadata + + m = Metadata.from_dict({}) + assert m.bazel_config == [] + + def test_bazel_config_multi(self): + from models.module import Metadata + + m = Metadata.from_dict({"bazel_config": ["per-x86_64-linux", "ferrocene-coverage"]}) + assert m.bazel_config == ["per-x86_64-linux", "ferrocene-coverage"] + + +class TestFromModGraph: + @staticmethod + def _graph() -> dict: + # Mirrors 'bazel mod graph --output=json': overridden modules report version 0.0.0. + return { + "key": "", + "name": "ref_int", + "version": "", + "dependencies": [ + {"name": "trlc", "version": "0.0.0"}, # git_override (carried from file) + {"name": "rules_boost", "version": "0.0.0"}, # archive_override (not representable) + {"name": "score_baselibs", "version": "0.0.0"}, # git_override (carried from file) + { + "name": "protobuf", + "version": "29.1", + "dependencies": [ + {"name": "abseil-cpp", "version": "20250512.1"}, + ], + }, + ], + } + + def test_merges_overrides_and_registry_versions(self, tmp_path: Path): + graph = tmp_path / "graph.json" + graph.write_text(json.dumps(self._graph())) + root = tmp_path / "MODULE.bazel" + root.write_text( + 'git_override(\n module_name = "trlc",\n commit = "abc1234",\n' + ' remote = "https://github.com/x/trlc.git",\n)\n' + 'archive_override(\n module_name = "rules_boost",\n urls = ["https://e/x.tar"],\n)\n' + ) + scoremods = tmp_path / "score_modules_target_sw.MODULE.bazel" + scoremods.write_text( + 'git_override(\n module_name = "score_baselibs",\n commit = "def5678",\n' + ' remote = "https://github.com/eclipse-score/baselibs.git",\n)\n' + ) + + rd = ResolvedDependencies.from_mod_graph(graph, [root, scoremods]) + # Overridden modules carried as their real git_override (graph's 0.0.0 ignored). + assert rd.get("trlc").hash == "abc1234" + assert rd.get("score_baselibs").hash == "def5678" + # Registry modules carried from the resolved graph version. + assert rd.get("protobuf").version == "29.1" + assert rd.get("abseil-cpp").version == "20250512.1" + # archive_override target at 0.0.0 is not representable -> not carried. + assert rd.get("rules_boost") is None + + def test_ignores_commented_out_overrides(self, tmp_path: Path): + graph = tmp_path / "graph.json" + graph.write_text(json.dumps({"key": "", "name": "r", "version": "", "dependencies": []})) + root = tmp_path / "MODULE.bazel" + root.write_text( + '# git_override(\n# module_name = "rules_rpm",\n' + '# commit = "a78e559cf81754c199c926229dc6b4443e1ff149",\n' + '# remote = "https://github.com/eclipse-score/inc_os_autosd.git",\n# )\n' + ) + rd = ResolvedDependencies.from_mod_graph(graph, [root]) + assert rd.get("rules_rpm") is None # commented-out override must not be carried + + +class TestManifestRoundtrip: + def test_to_file_is_lean_and_roundtrips(self, tmp_path: Path, resolved: ResolvedDependencies): + manifest = tmp_path / "resolved_versions.json" + resolved.to_file(manifest) + data = json.loads(manifest.read_text())["modules"] + assert "metadata" not in data["score_baselibs"] # lean: no test-config noise + assert data["score_tooling"] == {"version": "1.2.0"} + loaded = ResolvedDependencies.from_file(manifest) + assert loaded.get("score_baselibs").hash == resolved.get("score_baselibs").hash + assert loaded.get("score_tooling").version == "1.2.0" + + +class TestFromResolvedArtifact: + def test_prefers_manifest(self, tmp_path: Path, resolved: ResolvedDependencies): + art = tmp_path / "art" + art.mkdir() + resolved.to_file(art / "resolved_versions.json") + # With the manifest present, no lock / score_modules files are required. + parsed = ResolvedDependencies.from_resolved_artifact(art) + assert parsed.get("score_baselibs").hash == resolved.get("score_baselibs").hash + assert parsed.get("score_tooling").version == "1.2.0" + + def test_requires_manifest_or_lockfile(self, tmp_path: Path): + (tmp_path / "score_modules_target_sw.MODULE.bazel").write_text("bazel_dep(name='x')\n") + with pytest.raises(FileNotFoundError): + ResolvedDependencies.from_resolved_artifact(tmp_path) + + def test_roundtrip_known_good_to_artifact(self, tmp_path: Path, resolved: ResolvedDependencies): + # Build an artifact dir mirroring stage1-resolved-deps, then parse it back. + from update_module_from_known_good import generate_git_override_blocks + + art = tmp_path / "art" + art.mkdir() + (art / "MODULE.bazel.lock").write_text("{}") + blocks = generate_git_override_blocks(list(resolved._resolved.values()), {}) + (art / "score_modules_target_sw.MODULE.bazel").write_text("\n".join(blocks)) + + parsed = ResolvedDependencies.from_resolved_artifact(art) + assert parsed.get("score_baselibs").hash == resolved.get("score_baselibs").hash + assert parsed.get("score_tooling").version == "1.2.0" diff --git a/scripts/known_good/update_module_from_known_good.py b/scripts/known_good/update_module_from_known_good.py index 2d61ea2a805..9174070b97a 100755 --- a/scripts/known_good/update_module_from_known_good.py +++ b/scripts/known_good/update_module_from_known_good.py @@ -35,76 +35,98 @@ from pathlib import Path from typing import Dict, List, Optional -from models import Module -from models.known_good import load_known_good +# Import models whether run standalone (scripts/known_good on path) or imported as part +# of the ``known_good`` package (scripts/ on path, where bare ``models`` is shadowed by +# the separate scripts/models package). +try: + from known_good.models.known_good import load_known_good + from known_good.models.module import Module +except ImportError: + from models import Module + from models.known_good import load_known_good # Configure logging logging.basicConfig(level=logging.WARNING, format="%(levelname)s: %(message)s") +def generate_override_directive(module: Module, repo_commit_dict: Optional[Dict[str, str]] = None) -> Optional[str]: + """Generate the override directive (single_version_override / git_override) for a module. + + Returns just the override call (without the preceding ``bazel_dep(...)`` line), so the + same logic can be reused both to (a) build ref_int's score_modules_*.MODULE.bazel files + (composed with a bazel_dep line by ``generate_git_override_blocks``) and to (b) inject + overrides into a module's own MODULE.bazel where the bazel_dep is already declared + (see ``ResolvedDependencies.overwrite`` in resolved_dependencies.py). + + Returns ``None`` (and logs a warning) when the module has neither a usable version nor a + valid repo+commit, mirroring the skip behaviour of the original generator. + """ + repo_commit_dict = repo_commit_dict or {} + commit = module.hash + + # Allow overriding specific repos via command line + if module.repo in repo_commit_dict: + commit = repo_commit_dict[module.repo] + + # Generate patches lines if bazel_patches exist + patches_lines = "" + if module.bazel_patches: + patches_lines = " patches = [\n" + for patch in module.bazel_patches: + patches_lines += f' "{patch}",\n' + patches_lines += " ],\n" + patch_strip_line = " patch_strip = 1,\n" if patches_lines else "" + + if module.version: + # If version is provided, use single_version_override + return ( + "single_version_override(\n" + f' module_name = "{module.name}",\n' + f"{patch_strip_line}" + f"{patches_lines}" + f' version = "{module.version}",\n' + ")\n" + ) + + if not module.repo or not commit: + logging.warning( + "Skipping module %s with missing repo or commit: repo=%s, commit=%s", + module.name, + module.repo, + commit, + ) + return None + + # Validate commit hash format (7-40 hex characters) + if not re.match(r"^[a-fA-F0-9]{7,40}$", commit): + logging.warning( + "Skipping module %s with invalid commit hash: %s", + module.name, + commit, + ) + return None + + # If no version, use git_override. Only include patch_strip if there are patches to apply. + return ( + "git_override(\n" + f' module_name = "{module.name}",\n' + f' commit = "{commit}",\n' + f"{patch_strip_line}" + f"{patches_lines}" + f' remote = "{module.repo}",\n' + ")\n" + ) + + def generate_git_override_blocks(modules: List[Module], repo_commit_dict: Dict[str, str]) -> List[str]: """Generate bazel_dep and git_override blocks for each module.""" blocks = [] for module in modules: - commit = module.hash - - # Allow overriding specific repos via command line - if module.repo in repo_commit_dict: - commit = repo_commit_dict[module.repo] - - # Generate patches lines if bazel_patches exist - patches_lines = "" - if module.bazel_patches: - patches_lines = " patches = [\n" - for patch in module.bazel_patches: - patches_lines += f' "{patch}",\n' - patches_lines += " ],\n" - patch_strip_line = " patch_strip = 1,\n" if patches_lines else "" - - if module.version: - # If version is provided, use bazel_dep with single_version_override - block = ( - f'bazel_dep(name = "{module.name}")\n' - "single_version_override(\n" - f' module_name = "{module.name}",\n' - f"{patch_strip_line}" - f"{patches_lines}" - f' version = "{module.version}",\n' - ")\n" - ) - else: - if not module.repo or not commit: - logging.warning( - "Skipping module %s with missing repo or commit: repo=%s, commit=%s", - module.name, - module.repo, - commit, - ) - continue - - # Validate commit hash format (7-40 hex characters) - if not re.match(r"^[a-fA-F0-9]{7,40}$", commit): - logging.warning( - "Skipping module %s with invalid commit hash: %s", - module.name, - commit, - ) - continue - - # If no version, use bazel_dep with git_override - # Only include patch_strip if there are patches to apply - block = ( - f'bazel_dep(name = "{module.name}")\n' - "git_override(\n" - f' module_name = "{module.name}",\n' - f' commit = "{commit}",\n' - f"{patch_strip_line}" - f"{patches_lines}" - f' remote = "{module.repo}",\n' - ")\n" - ) - blocks.append(block) + directive = generate_override_directive(module, repo_commit_dict) + if directive is None: + continue + blocks.append(f'bazel_dep(name = "{module.name}")\n' + directive) return blocks From 6f83e039150a68af0f150ded21e046eb5c7a0da6 Mon Sep 17 00:00:00 2001 From: subramaniak Date: Thu, 2 Jul 2026 11:42:58 +0530 Subject: [PATCH 02/14] fix: remove bazel_config, always overwrite overrides, clean injection markers, simplify imports --- scripts/known_good/models/module.py | 5 +- scripts/known_good/resolved_dependencies.py | 56 ++++++------------- .../tests/test_resolved_dependencies.py | 40 ++++--------- 3 files changed, 28 insertions(+), 73 deletions(-) diff --git a/scripts/known_good/models/module.py b/scripts/known_good/models/module.py index de01225a368..73adfe5f9a4 100644 --- a/scripts/known_good/models/module.py +++ b/scripts/known_good/models/module.py @@ -35,8 +35,7 @@ class Metadata: extra_test_config: list[str] = field(default_factory=lambda: []) exclude_test_targets: list[str] = field(default_factory=lambda: []) langs: list[str] = field(default_factory=lambda: ["cpp", "rust"]) - rust_coverage_config: str | None = "ferrocene-coverage" # Optional field for Rust coverage configuration - bazel_config: list[str] = field(default_factory=lambda: []) + rust_coverage_config: str | None = "ferrocene-coverage" @classmethod def from_dict(cls, data: Dict[str, Any]) -> Metadata: @@ -54,7 +53,6 @@ def from_dict(cls, data: Dict[str, Any]) -> Metadata: exclude_test_targets=data.get("exclude_test_targets", []), langs=data.get("langs", ["cpp", "rust"]), rust_coverage_config=data.get("rust_coverage_config", "ferrocene-coverage"), - bazel_config=data.get("bazel_config", []), ) def to_dict(self) -> Dict[str, Any]: @@ -69,7 +67,6 @@ def to_dict(self) -> Dict[str, Any]: "exclude_test_targets": self.exclude_test_targets, "langs": self.langs, "rust_coverage_config": self.rust_coverage_config, - "bazel_config": self.bazel_config, } diff --git a/scripts/known_good/resolved_dependencies.py b/scripts/known_good/resolved_dependencies.py index 6baebf85cdf..09a004c5b47 100644 --- a/scripts/known_good/resolved_dependencies.py +++ b/scripts/known_good/resolved_dependencies.py @@ -13,22 +13,15 @@ # ******************************************************************************* """Resolved dependency versions from the reference_integration root. -DR-008 Option 4 requires that the dependency versions ``reference_integration`` -resolves are pushed *into* each module so the module's own unit tests + coverage -run against the resolved set (not against the versions the module declares in its -released ``MODULE.bazel``). - -This module provides :class:`ResolvedDependencies`, which: - -* holds the resolved version/commit per dependency (sourced from ref_int's root — - either ``known_good.json`` for local runs, or the Stage-1 ``stage1-resolved-deps`` - artifact for CI runs so the resolution flows Stage 1 -> Stage 2), and -* exposes an interface to **scan** an individual module's ``MODULE.bazel`` and - **overwrite** the declared dependency versions to match the resolved set, by - appending the matching ``git_override`` / ``single_version_override`` directives. - -The injection is append-only and operates on the CI checkout of the module — it is -never committed back to the module's released sources (DR-008 "temporary mechanism"). +Provides :class:`ResolvedDependencies`, which holds the resolved version/commit per +dependency (sourced from ref_int's root — either ``known_good.json`` for local runs, +or the Stage-1 ``stage1-resolved-deps`` artifact for CI runs), and exposes an interface +to **scan** an individual module's ``MODULE.bazel`` and **overwrite** the declared +dependency versions to match the resolved set by appending the matching +``git_override`` / ``single_version_override`` directives. + +The injection operates on the CI checkout of the module — it is never committed back +to the module's released sources. """ from __future__ import annotations @@ -37,29 +30,18 @@ import json import logging import re -import sys from pathlib import Path from typing import Dict, List, Optional -# Import ``models`` + ``generate_override_directive`` whether this file is loaded as -# ``known_good.resolved_dependencies`` (scripts/ on path, e.g. from quality_runners.py) -# or as ``resolved_dependencies`` (scripts/known_good/ on path). Preferring the -# package-qualified form keeps a single ``Module`` class identity in the package context. _HERE = Path(__file__).resolve().parent -try: - from known_good.models.known_good import load_known_good - from known_good.models.module import Module - from known_good.update_module_from_known_good import generate_override_directive -except ImportError: - if str(_HERE) not in sys.path: - sys.path.insert(0, str(_HERE)) - from models.known_good import load_known_good # noqa: E402 - from models.module import Module # noqa: E402 - from update_module_from_known_good import generate_override_directive # noqa: E402 + +from known_good.models.known_good import load_known_good +from known_good.models.module import Module +from known_good.update_module_from_known_good import generate_override_directive # Marker delimiting the block we append, so injection is idempotent / detectable. -INJECTION_BEGIN = "# --- BEGIN ref_int resolved-deps injection (DR-008 Option 4) ---" -INJECTION_END = "# --- END ref_int resolved-deps injection (DR-008 Option 4) ---" +INJECTION_BEGIN = "# --- BEGIN ref_int resolved-deps injection ---" +INJECTION_END = "# --- END ref_int resolved-deps injection ---" # The single file that carries the resolved set from Stage 1 (resolve) to Stage 2 # (per-module validation). It is the only handoff needed: first-party commits + @@ -71,10 +53,6 @@ # Capture the module name from any ``bazel_dep(name = "...")`` call (name is the first arg). _BAZEL_DEP_RE = re.compile(r'bazel_dep\(\s*name\s*=\s*"([^"]+)"') -# Capture an existing override target so we don't inject a duplicate for the same module. -_OVERRIDE_RE = re.compile( - r'(?:git_override|single_version_override|local_path_override|archive_override)\(\s*module_name\s*=\s*"([^"]+)"' -) # Parsers for reconstructing the resolved set from generated score_modules_*.MODULE.bazel. _GIT_OVERRIDE_BLOCK_RE = re.compile(r"git_override\((?P.*?)\)", re.S) _SINGLE_VERSION_BLOCK_RE = re.compile(r"single_version_override\((?P.*?)\)", re.S) @@ -270,7 +248,6 @@ def overwrite(self, module_bazel: Path, *, module_under_test: Optional[str] = No original = self._strip_injection(module_bazel.read_text()) declared = set(_BAZEL_DEP_RE.findall(original)) - already_overridden = set(_OVERRIDE_RE.findall(original)) from dataclasses import replace as _replace @@ -280,11 +257,10 @@ def overwrite(self, module_bazel: Path, *, module_under_test: Optional[str] = No # module(s)" if an override targets a module that is not in this module's dependency # graph, so the full resolved set cannot be injected wholesale — a declared bazel_dep # is by definition in the graph, which makes its override safe. + # ref_int always decides the version — any existing module-level override is replaced. for name in sorted(declared): if name == module_under_test: continue # the module under test is the root; never override it - if name in already_overridden: - continue # respect an override the module already declares module = self._resolved.get(name) if module is None: continue # dep ref_int does not pin; resolves normally diff --git a/scripts/known_good/tests/test_resolved_dependencies.py b/scripts/known_good/tests/test_resolved_dependencies.py index 37608b76004..a51aded6ead 100644 --- a/scripts/known_good/tests/test_resolved_dependencies.py +++ b/scripts/known_good/tests/test_resolved_dependencies.py @@ -22,12 +22,12 @@ import pytest -# Make scripts/known_good importable when run via plain pytest. -_KG_DIR = Path(__file__).resolve().parents[1] -if str(_KG_DIR) not in sys.path: - sys.path.insert(0, str(_KG_DIR)) +# Make scripts/ importable so known_good.* package resolves when run via plain pytest. +_SCRIPTS_DIR = Path(__file__).resolve().parents[2] +if str(_SCRIPTS_DIR) not in sys.path: + sys.path.insert(0, str(_SCRIPTS_DIR)) -from resolved_dependencies import ( # noqa: E402 +from known_good.resolved_dependencies import ( # noqa: E402 INJECTION_BEGIN, INJECTION_END, ResolvedDependencies, @@ -155,7 +155,8 @@ def test_idempotent(self, resolved: ResolvedDependencies, module_bazel: Path): assert first == second assert second.count(INJECTION_BEGIN) == 1 - def test_skips_dep_with_existing_override(self, resolved: ResolvedDependencies, tmp_path: Path): + def test_overwrites_dep_with_existing_override(self, resolved: ResolvedDependencies, tmp_path: Path): + # ref_int always decides the version — a pre-existing override in the module is replaced. mod = tmp_path / "MODULE.bazel" mod.write_text( MODULE_BAZEL + '\ngit_override(\n module_name = "score_logging",\n commit = "deadbeef",\n' @@ -163,28 +164,9 @@ def test_skips_dep_with_existing_override(self, resolved: ResolvedDependencies, ) patched = resolved.overwrite(mod, module_under_test="score_persistency", write=False) block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] - assert 'module_name = "score_logging"' not in block # respected pre-existing override - - -class TestMetadataBazelConfig: - def test_bazel_config_roundtrip(self): - from models.module import Metadata - - m = Metadata.from_dict({"bazel_config": ["bl-x86_64-linux"]}) - assert m.bazel_config == ["bl-x86_64-linux"] - assert m.to_dict()["bazel_config"] == ["bl-x86_64-linux"] - - def test_bazel_config_default_empty(self): - from models.module import Metadata - - m = Metadata.from_dict({}) - assert m.bazel_config == [] - - def test_bazel_config_multi(self): - from models.module import Metadata - - m = Metadata.from_dict({"bazel_config": ["per-x86_64-linux", "ferrocene-coverage"]}) - assert m.bazel_config == ["per-x86_64-linux", "ferrocene-coverage"] + # ref_int's resolved commit must appear in the injection block, overwriting "deadbeef" + assert 'module_name = "score_logging"' in block + assert "deadbeef" not in block class TestFromModGraph: @@ -276,7 +258,7 @@ def test_requires_manifest_or_lockfile(self, tmp_path: Path): def test_roundtrip_known_good_to_artifact(self, tmp_path: Path, resolved: ResolvedDependencies): # Build an artifact dir mirroring stage1-resolved-deps, then parse it back. - from update_module_from_known_good import generate_git_override_blocks + from known_good.update_module_from_known_good import generate_git_override_blocks art = tmp_path / "art" art.mkdir() From e6ddcc91e6490185a37924ae641bf8575045a201 Mon Sep 17 00:00:00 2001 From: subramaniak Date: Thu, 2 Jul 2026 11:53:24 +0530 Subject: [PATCH 03/14] fix: revert update_module_from_known_good.py, move generate_override_directive inline, add BUILD for bazel run --- scripts/known_good/BUILD | 37 +++++ scripts/known_good/resolved_dependencies.py | 57 ++++++- .../tests/test_resolved_dependencies.py | 11 +- .../update_module_from_known_good.py | 142 ++++++++---------- scripts/tooling/BUILD | 8 + 5 files changed, 168 insertions(+), 87 deletions(-) create mode 100644 scripts/known_good/BUILD diff --git a/scripts/known_good/BUILD b/scripts/known_good/BUILD new file mode 100644 index 00000000000..647a539c997 --- /dev/null +++ b/scripts/known_good/BUILD @@ -0,0 +1,37 @@ +# ******************************************************************************* +# 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 +# ******************************************************************************* +load("@rules_python//python:defs.bzl", "py_binary", "py_library") + +# Library target: the known_good package (models + generators). +# Used as a dep by //scripts/tooling and by the resolve_deps binary below. +py_library( + name = "known_good", + srcs = glob( + ["**/*.py"], + exclude = ["tests/**"], + ), + visibility = ["//visibility:public"], +) + +# Runnable binary for the resolve + inject workflow. +# Stage 1 (export): bazel run //scripts/known_good:resolve_deps -- \ +# --mod-graph graph.json --export artifacts/resolved_versions.json +# Stage 2 (inject): bazel run //scripts/known_good:resolve_deps -- \ +# _module/MODULE.bazel --resolved-deps _resolved_deps/ +py_binary( + name = "resolve_deps", + srcs = ["resolved_dependencies.py"], + main = "resolved_dependencies.py", + visibility = ["//visibility:public"], + deps = [":known_good"], +) diff --git a/scripts/known_good/resolved_dependencies.py b/scripts/known_good/resolved_dependencies.py index 09a004c5b47..d77f9dd3f42 100644 --- a/scripts/known_good/resolved_dependencies.py +++ b/scripts/known_good/resolved_dependencies.py @@ -37,12 +37,67 @@ from known_good.models.known_good import load_known_good from known_good.models.module import Module -from known_good.update_module_from_known_good import generate_override_directive # Marker delimiting the block we append, so injection is idempotent / detectable. INJECTION_BEGIN = "# --- BEGIN ref_int resolved-deps injection ---" INJECTION_END = "# --- END ref_int resolved-deps injection ---" + +def generate_override_directive(module: Module, repo_commit_dict: Optional[Dict[str, str]] = None) -> Optional[str]: + """Return the override directive (single_version_override / git_override) for a module. + + Returns just the override call without a preceding ``bazel_dep(...)`` line, so the + same logic can be reused both to build ref_int's score_modules_*.MODULE.bazel files + and to inject overrides into a module's own MODULE.bazel where bazel_dep is already + declared (see :meth:`ResolvedDependencies.overwrite`). + + Returns ``None`` when the module has neither a usable version nor a valid repo+commit. + """ + repo_commit_dict = repo_commit_dict or {} + commit = module.hash + + if module.repo in repo_commit_dict: + commit = repo_commit_dict[module.repo] + + patches_lines = "" + if module.bazel_patches: + patches_lines = " patches = [\n" + for patch in module.bazel_patches: + patches_lines += f' "{patch}",\n' + patches_lines += " ],\n" + patch_strip_line = " patch_strip = 1,\n" if patches_lines else "" + + if module.version: + return ( + "single_version_override(\n" + f' module_name = "{module.name}",\n' + f"{patch_strip_line}" + f"{patches_lines}" + f' version = "{module.version}",\n' + ")\n" + ) + + if not module.repo or not commit: + logging.warning( + "Skipping module %s with missing repo or commit: repo=%s, commit=%s", + module.name, module.repo, commit, + ) + return None + + if not re.match(r"^[a-fA-F0-9]{7,40}$", commit): + logging.warning("Skipping module %s with invalid commit hash: %s", module.name, commit) + return None + + return ( + "git_override(\n" + f' module_name = "{module.name}",\n' + f' commit = "{commit}",\n' + f"{patch_strip_line}" + f"{patches_lines}" + f' remote = "{module.repo}",\n' + ")\n" + ) + # The single file that carries the resolved set from Stage 1 (resolve) to Stage 2 # (per-module validation). It is the only handoff needed: first-party commits + # third-party resolved versions, merged. The lock travels alongside only as evidence. diff --git a/scripts/known_good/tests/test_resolved_dependencies.py b/scripts/known_good/tests/test_resolved_dependencies.py index a51aded6ead..511bb9fc2b2 100644 --- a/scripts/known_good/tests/test_resolved_dependencies.py +++ b/scripts/known_good/tests/test_resolved_dependencies.py @@ -31,6 +31,7 @@ INJECTION_BEGIN, INJECTION_END, ResolvedDependencies, + generate_override_directive, ) KNOWN_GOOD = { @@ -257,13 +258,15 @@ def test_requires_manifest_or_lockfile(self, tmp_path: Path): ResolvedDependencies.from_resolved_artifact(tmp_path) def test_roundtrip_known_good_to_artifact(self, tmp_path: Path, resolved: ResolvedDependencies): - # Build an artifact dir mirroring stage1-resolved-deps, then parse it back. - from known_good.update_module_from_known_good import generate_git_override_blocks - + # Build an artifact dir mirroring stage1-resolved-deps (legacy format), then parse it back. art = tmp_path / "art" art.mkdir() (art / "MODULE.bazel.lock").write_text("{}") - blocks = generate_git_override_blocks(list(resolved._resolved.values()), {}) + blocks = [] + for m in resolved._resolved.values(): + directive = generate_override_directive(m) + if directive: + blocks.append(f'bazel_dep(name = "{m.name}")\n' + directive) (art / "score_modules_target_sw.MODULE.bazel").write_text("\n".join(blocks)) parsed = ResolvedDependencies.from_resolved_artifact(art) diff --git a/scripts/known_good/update_module_from_known_good.py b/scripts/known_good/update_module_from_known_good.py index 9174070b97a..2d61ea2a805 100755 --- a/scripts/known_good/update_module_from_known_good.py +++ b/scripts/known_good/update_module_from_known_good.py @@ -35,98 +35,76 @@ from pathlib import Path from typing import Dict, List, Optional -# Import models whether run standalone (scripts/known_good on path) or imported as part -# of the ``known_good`` package (scripts/ on path, where bare ``models`` is shadowed by -# the separate scripts/models package). -try: - from known_good.models.known_good import load_known_good - from known_good.models.module import Module -except ImportError: - from models import Module - from models.known_good import load_known_good +from models import Module +from models.known_good import load_known_good # Configure logging logging.basicConfig(level=logging.WARNING, format="%(levelname)s: %(message)s") -def generate_override_directive(module: Module, repo_commit_dict: Optional[Dict[str, str]] = None) -> Optional[str]: - """Generate the override directive (single_version_override / git_override) for a module. - - Returns just the override call (without the preceding ``bazel_dep(...)`` line), so the - same logic can be reused both to (a) build ref_int's score_modules_*.MODULE.bazel files - (composed with a bazel_dep line by ``generate_git_override_blocks``) and to (b) inject - overrides into a module's own MODULE.bazel where the bazel_dep is already declared - (see ``ResolvedDependencies.overwrite`` in resolved_dependencies.py). - - Returns ``None`` (and logs a warning) when the module has neither a usable version nor a - valid repo+commit, mirroring the skip behaviour of the original generator. - """ - repo_commit_dict = repo_commit_dict or {} - commit = module.hash - - # Allow overriding specific repos via command line - if module.repo in repo_commit_dict: - commit = repo_commit_dict[module.repo] - - # Generate patches lines if bazel_patches exist - patches_lines = "" - if module.bazel_patches: - patches_lines = " patches = [\n" - for patch in module.bazel_patches: - patches_lines += f' "{patch}",\n' - patches_lines += " ],\n" - patch_strip_line = " patch_strip = 1,\n" if patches_lines else "" - - if module.version: - # If version is provided, use single_version_override - return ( - "single_version_override(\n" - f' module_name = "{module.name}",\n' - f"{patch_strip_line}" - f"{patches_lines}" - f' version = "{module.version}",\n' - ")\n" - ) - - if not module.repo or not commit: - logging.warning( - "Skipping module %s with missing repo or commit: repo=%s, commit=%s", - module.name, - module.repo, - commit, - ) - return None - - # Validate commit hash format (7-40 hex characters) - if not re.match(r"^[a-fA-F0-9]{7,40}$", commit): - logging.warning( - "Skipping module %s with invalid commit hash: %s", - module.name, - commit, - ) - return None - - # If no version, use git_override. Only include patch_strip if there are patches to apply. - return ( - "git_override(\n" - f' module_name = "{module.name}",\n' - f' commit = "{commit}",\n' - f"{patch_strip_line}" - f"{patches_lines}" - f' remote = "{module.repo}",\n' - ")\n" - ) - - def generate_git_override_blocks(modules: List[Module], repo_commit_dict: Dict[str, str]) -> List[str]: """Generate bazel_dep and git_override blocks for each module.""" blocks = [] for module in modules: - directive = generate_override_directive(module, repo_commit_dict) - if directive is None: - continue - blocks.append(f'bazel_dep(name = "{module.name}")\n' + directive) + commit = module.hash + + # Allow overriding specific repos via command line + if module.repo in repo_commit_dict: + commit = repo_commit_dict[module.repo] + + # Generate patches lines if bazel_patches exist + patches_lines = "" + if module.bazel_patches: + patches_lines = " patches = [\n" + for patch in module.bazel_patches: + patches_lines += f' "{patch}",\n' + patches_lines += " ],\n" + patch_strip_line = " patch_strip = 1,\n" if patches_lines else "" + + if module.version: + # If version is provided, use bazel_dep with single_version_override + block = ( + f'bazel_dep(name = "{module.name}")\n' + "single_version_override(\n" + f' module_name = "{module.name}",\n' + f"{patch_strip_line}" + f"{patches_lines}" + f' version = "{module.version}",\n' + ")\n" + ) + else: + if not module.repo or not commit: + logging.warning( + "Skipping module %s with missing repo or commit: repo=%s, commit=%s", + module.name, + module.repo, + commit, + ) + continue + + # Validate commit hash format (7-40 hex characters) + if not re.match(r"^[a-fA-F0-9]{7,40}$", commit): + logging.warning( + "Skipping module %s with invalid commit hash: %s", + module.name, + commit, + ) + continue + + # If no version, use bazel_dep with git_override + # Only include patch_strip if there are patches to apply + block = ( + f'bazel_dep(name = "{module.name}")\n' + "git_override(\n" + f' module_name = "{module.name}",\n' + f' commit = "{commit}",\n' + f"{patch_strip_line}" + f"{patches_lines}" + f' remote = "{module.repo}",\n' + ")\n" + ) + blocks.append(block) return blocks diff --git a/scripts/tooling/BUILD b/scripts/tooling/BUILD index c2088414897..854bae80e41 100644 --- a/scripts/tooling/BUILD +++ b/scripts/tooling/BUILD @@ -81,6 +81,14 @@ py_binary( visibility = ["//visibility:public"], ) +# Alias: expose the resolve_deps script under //scripts/tooling so it can be +# invoked as `bazel run //scripts/tooling:resolve_deps` alongside other tooling scripts. +alias( + name = "resolve_deps", + actual = "//scripts/known_good:resolve_deps", + visibility = ["//visibility:public"], +) + # Tests target score_py_pytest( name = "tooling_tests", From e131bdda0ccd470e3e84ebb026dc2f39d2be2a83 Mon Sep 17 00:00:00 2001 From: subramaniak Date: Thu, 2 Jul 2026 12:19:11 +0530 Subject: [PATCH 04/14] fix: ruff format fix for scripts/tooling/BUILD --- scripts/known_good/models/module.py | 16 ++-- scripts/known_good/resolved_dependencies.py | 54 ++++++++------ .../tests/test_resolved_dependencies.py | 5 +- .../update_module_from_known_good.py | 16 ++-- scripts/tooling/BUILD | 74 ++++++++++--------- 5 files changed, 87 insertions(+), 78 deletions(-) diff --git a/scripts/known_good/models/module.py b/scripts/known_good/models/module.py index 73adfe5f9a4..7028fa988d1 100644 --- a/scripts/known_good/models/module.py +++ b/scripts/known_good/models/module.py @@ -32,13 +32,13 @@ class Metadata: """ code_root_path: str = "//score/..." - extra_test_config: list[str] = field(default_factory=lambda: []) - exclude_test_targets: list[str] = field(default_factory=lambda: []) + extra_test_config: list[str] = field(default_factory=list) + exclude_test_targets: list[str] = field(default_factory=list) langs: list[str] = field(default_factory=lambda: ["cpp", "rust"]) rust_coverage_config: str | None = "ferrocene-coverage" @classmethod - def from_dict(cls, data: Dict[str, Any]) -> Metadata: + def from_dict(cls, data: dict[str, Any]) -> Metadata: """Create a Metadata instance from a dictionary. Args: @@ -55,7 +55,7 @@ def from_dict(cls, data: Dict[str, Any]) -> Metadata: rust_coverage_config=data.get("rust_coverage_config", "ferrocene-coverage"), ) - def to_dict(self) -> Dict[str, Any]: + def to_dict(self) -> dict[str, Any]: """Convert Metadata instance to dictionary representation. Returns: @@ -82,7 +82,7 @@ class Module: pin_version: bool = False @classmethod - def from_dict(cls, name: str, module_data: Dict[str, Any]) -> Module: + def from_dict(cls, name: str, module_data: dict[str, Any]) -> Module: """Create a Module instance from a dictionary representation. Args: @@ -149,7 +149,7 @@ def from_dict(cls, name: str, module_data: Dict[str, Any]) -> Module: ) @classmethod - def parse_modules(cls, modules_dict: Dict[str, Any]) -> List[Module]: + def parse_modules(cls, modules_dict: dict[str, Any]) -> list[Module]: """Parse modules dictionary into Module dataclass instances. Args: @@ -190,13 +190,13 @@ def owner_repo(self) -> str: return f"{parts[0]}/{parts[1]}" - def to_dict(self) -> Dict[str, Any]: + def to_dict(self) -> dict[str, Any]: """Convert Module instance to dictionary representation for JSON output. Returns: Dictionary with module configuration """ - result: Dict[str, Any] = {"repo": self.repo} + result: dict[str, Any] = {"repo": self.repo} if self.version: result["version"] = self.version else: diff --git a/scripts/known_good/resolved_dependencies.py b/scripts/known_good/resolved_dependencies.py index d77f9dd3f42..543f845c1fc 100644 --- a/scripts/known_good/resolved_dependencies.py +++ b/scripts/known_good/resolved_dependencies.py @@ -31,19 +31,18 @@ import logging import re from pathlib import Path -from typing import Dict, List, Optional - -_HERE = Path(__file__).resolve().parent from known_good.models.known_good import load_known_good from known_good.models.module import Module +_HERE = Path(__file__).resolve().parent + # Marker delimiting the block we append, so injection is idempotent / detectable. INJECTION_BEGIN = "# --- BEGIN ref_int resolved-deps injection ---" INJECTION_END = "# --- END ref_int resolved-deps injection ---" -def generate_override_directive(module: Module, repo_commit_dict: Optional[Dict[str, str]] = None) -> Optional[str]: +def generate_override_directive(module: Module, repo_commit_dict: dict[str, str] | None = None) -> str | None: """Return the override directive (single_version_override / git_override) for a module. Returns just the override call without a preceding ``bazel_dep(...)`` line, so the @@ -80,7 +79,9 @@ def generate_override_directive(module: Module, repo_commit_dict: Optional[Dict[ if not module.repo or not commit: logging.warning( "Skipping module %s with missing repo or commit: repo=%s, commit=%s", - module.name, module.repo, commit, + module.name, + module.repo, + commit, ) return None @@ -98,6 +99,7 @@ def generate_override_directive(module: Module, repo_commit_dict: Optional[Dict[ ")\n" ) + # The single file that carries the resolved set from Stage 1 (resolve) to Stage 2 # (per-module validation). It is the only handoff needed: first-party commits + # third-party resolved versions, merged. The lock travels alongside only as evidence. @@ -121,23 +123,23 @@ class ResolvedDependencies: interface to scan + overwrite a module's ``MODULE.bazel`` to those versions. """ - def __init__(self, resolved: Dict[str, Module]): + def __init__(self, resolved: dict[str, Module]): self._resolved = resolved # -- construction: "resolved deps versions from ref_int root" -------------------- @classmethod - def from_known_good(cls, known_good_path: Path) -> "ResolvedDependencies": + def from_known_good(cls, known_good_path: Path) -> ResolvedDependencies: """Build from ``known_good.json`` (local / dev source of the resolved pins).""" kg = load_known_good(Path(known_good_path).resolve()) - resolved: Dict[str, Module] = {} + resolved: dict[str, Module] = {} for group in kg.modules.values(): for module in group.values(): resolved[module.name] = module return cls(resolved) @classmethod - def from_resolved_artifact(cls, artifact_dir: Path) -> "ResolvedDependencies": + def from_resolved_artifact(cls, artifact_dir: Path) -> ResolvedDependencies: """Build from the Stage-1 ``stage1-resolved-deps`` artifact. The handoff is the single ``resolved_versions.json`` manifest (see @@ -164,14 +166,14 @@ def from_resolved_artifact(cls, artifact_dir: Path) -> "ResolvedDependencies": if not module_files: raise FileNotFoundError(f"No score_modules_*.MODULE.bazel files in resolved-deps artifact {artifact_dir}.") - resolved: Dict[str, Module] = {} + resolved: dict[str, Module] = {} for mf in module_files: for module in cls._parse_override_file(mf.read_text()): resolved[module.name] = module return cls(resolved) @classmethod - def from_mod_graph(cls, mod_graph_json: Path, override_files: List[Path]) -> "ResolvedDependencies": + def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> ResolvedDependencies: """Build the *complete* resolved set by merging two sources. * The override directives ref_int actually declares — parsed from its root @@ -190,8 +192,8 @@ def from_mod_graph(cls, mod_graph_json: Path, override_files: List[Path]) -> "Re ``archive_override`` / ``local_path_override`` targets (e.g. ``rules_boost``) cannot be represented and are logged as not carried. """ - resolved: Dict[str, Module] = {} - unrepresentable: List[str] = [] + resolved: dict[str, Module] = {} + unrepresentable: list[str] = [] for f in override_files: # Drop comment-only lines first: hand-written MODULE.bazel files contain # commented-out overrides (e.g. "# git_override(... rules_rpm ...)") that must @@ -203,9 +205,9 @@ def from_mod_graph(cls, mod_graph_json: Path, override_files: List[Path]) -> "Re unrepresentable.append(f"{m.group(2)} ({m.group(1)})") graph = json.loads(Path(mod_graph_json).read_text()) - versions: Dict[str, str] = {} + versions: dict[str, str] = {} _collect_resolved_versions(graph, versions) - skipped: List[str] = [] + skipped: list[str] = [] for name, version in versions.items(): if name in resolved or name in _SKIP_MODULES: continue # already carried by an override directive, or non-overridable @@ -237,23 +239,23 @@ def to_file(self, path: Path) -> None: modules = {} for name in sorted(self._resolved): m = self._resolved[name] - entry: Dict[str, object] = {"version": m.version} if m.version else {"repo": m.repo, "hash": m.hash} + entry: dict[str, object] = {"version": m.version} if m.version else {"repo": m.repo, "hash": m.hash} if m.bazel_patches: entry["bazel_patches"] = m.bazel_patches modules[name] = entry Path(path).write_text(json.dumps({"modules": modules}, indent=2) + "\n") @classmethod - def from_file(cls, path: Path) -> "ResolvedDependencies": + def from_file(cls, path: Path) -> ResolvedDependencies: """Load a resolved set previously written by :meth:`to_file`.""" data = json.loads(Path(path).read_text()) resolved = {name: Module.from_dict(name, md) for name, md in data.get("modules", {}).items()} return cls(resolved) @staticmethod - def _parse_override_file(text: str) -> List[Module]: + def _parse_override_file(text: str) -> list[Module]: """Reconstruct Module objects from generated git/single_version override blocks.""" - modules: List[Module] = [] + modules: list[Module] = [] for match in _GIT_OVERRIDE_BLOCK_RE.finditer(text): body = match.group("body") @@ -278,17 +280,21 @@ def _parse_override_file(text: str) -> List[Module]: def names(self) -> set[str]: return set(self._resolved) - def get(self, name: str) -> Optional[Module]: + @property + def modules(self) -> dict[str, Module]: + return dict(self._resolved) + + def get(self, name: str) -> Module | None: return self._resolved.get(name) - def scan(self, module_bazel: Path) -> List[str]: + def scan(self, module_bazel: Path) -> list[str]: """Return the names of dependencies a module declares via ``bazel_dep``.""" text = Path(module_bazel).read_text() # Ignore anything inside a previous injection block so re-scans are stable. text = self._strip_injection(text) return _BAZEL_DEP_RE.findall(text) - def overwrite(self, module_bazel: Path, *, module_under_test: Optional[str] = None, write: bool = True) -> str: + def overwrite(self, module_bazel: Path, *, module_under_test: str | None = None, write: bool = True) -> str: """Overwrite a module's declared dependency versions with the resolved set. Appends a ``git_override`` / ``single_version_override`` directive for every @@ -306,7 +312,7 @@ def overwrite(self, module_bazel: Path, *, module_under_test: Optional[str] = No from dataclasses import replace as _replace - directives: List[str] = [] + directives: list[str] = [] # Inject overrides only for deps the module actually declares (intersected with the # resolved set). Bazel fails with "root module specifies overrides on nonexistent # module(s)" if an override targets a module that is not in this module's dependency @@ -352,7 +358,7 @@ def _field(body: str, field: str) -> str: return match.group(1) if match else "" -def _collect_resolved_versions(node: dict, acc: Dict[str, str]) -> None: +def _collect_resolved_versions(node: dict, acc: dict[str, str]) -> None: """Walk a ``bazel mod graph --output=json`` tree, recording name -> resolved version. Each node carries the post-MVS ``name`` and ``version``; a module can appear many diff --git a/scripts/known_good/tests/test_resolved_dependencies.py b/scripts/known_good/tests/test_resolved_dependencies.py index 511bb9fc2b2..436669abb39 100644 --- a/scripts/known_good/tests/test_resolved_dependencies.py +++ b/scripts/known_good/tests/test_resolved_dependencies.py @@ -133,7 +133,8 @@ def test_skips_resolved_dep_not_declared(self, resolved: ResolvedDependencies, t # resolved dep the module does not declare must NOT be injected. mod = tmp_path / "MODULE.bazel" mod.write_text( - 'module(name = "score_persistency", version = "0.0.0")\nbazel_dep(name = "score_baselibs", version = "0.1")\n' + 'module(name = "score_persistency", version = "0.0.0")\n' + 'bazel_dep(name = "score_baselibs", version = "0.1")\n' ) block = resolved.overwrite(mod, module_under_test="score_persistency", write=False).split(INJECTION_BEGIN)[1] assert 'module_name = "score_baselibs"' in block # declared -> injected @@ -263,7 +264,7 @@ def test_roundtrip_known_good_to_artifact(self, tmp_path: Path, resolved: Resolv art.mkdir() (art / "MODULE.bazel.lock").write_text("{}") blocks = [] - for m in resolved._resolved.values(): + for m in resolved.modules.values(): directive = generate_override_directive(m) if directive: blocks.append(f'bazel_dep(name = "{m.name}")\n' + directive) diff --git a/scripts/known_good/update_module_from_known_good.py b/scripts/known_good/update_module_from_known_good.py index 2d61ea2a805..b38b4045870 100755 --- a/scripts/known_good/update_module_from_known_good.py +++ b/scripts/known_good/update_module_from_known_good.py @@ -42,7 +42,7 @@ logging.basicConfig(level=logging.WARNING, format="%(levelname)s: %(message)s") -def generate_git_override_blocks(modules: List[Module], repo_commit_dict: Dict[str, str]) -> List[str]: +def generate_git_override_blocks(modules: list[Module], repo_commit_dict: dict[str, str]) -> list[str]: """Generate bazel_dep and git_override blocks for each module.""" blocks = [] @@ -109,7 +109,7 @@ def generate_git_override_blocks(modules: List[Module], repo_commit_dict: Dict[s return blocks -def generate_local_override_blocks(modules: List[Module]) -> List[str]: +def generate_local_override_blocks(modules: list[Module]) -> list[str]: """Generate bazel_dep and local_path_override blocks for each module.""" blocks = [] @@ -127,7 +127,7 @@ def generate_local_override_blocks(modules: List[Module]) -> List[str]: return blocks -def generate_coverage_blocks(modules: List[Module]) -> List[str]: +def generate_coverage_blocks(modules: list[Module]) -> list[str]: """Generate rust_coverage_report blocks for each module with rust impl.""" blocks = ["""load("@score_tooling//:defs.bzl", "rust_coverage_report")"""] @@ -161,9 +161,9 @@ def generate_coverage_blocks(modules: List[Module]) -> List[str]: def generate_file_content( args: argparse.Namespace, - modules: List[Module], - repo_commit_dict: Dict[str, str], - timestamp: Optional[str] = None, + modules: list[Module], + repo_commit_dict: dict[str, str], + timestamp: str | None = None, file_type: str = "module", ) -> str: """Generate the complete content for score_modules.MODULE.bazel.""" @@ -292,9 +292,9 @@ def main() -> None: try: known_good = load_known_good(Path(known_path)) except FileNotFoundError as e: - raise SystemExit(f"ERROR: {e}") + raise SystemExit(f"ERROR: {e}") from e except ValueError as e: - raise SystemExit(f"ERROR: {e}") + raise SystemExit(f"ERROR: {e}") from e if not known_good.modules: raise SystemExit("No modules found in known_good.json") diff --git a/scripts/tooling/BUILD b/scripts/tooling/BUILD index 854bae80e41..6e848c58f51 100644 --- a/scripts/tooling/BUILD +++ b/scripts/tooling/BUILD @@ -22,84 +22,86 @@ load("@score_tooling//python_basics:defs.bzl", "score_py_pytest") # `bazel run //scripts/tooling:requirements.update -- --upgrade` compile_pip_requirements( - name = "requirements", - srcs = [ + name="requirements", + srcs=[ "requirements.in", "@score_tooling//python_basics:requirements.txt", ], - extra_args = [ + extra_args=[ "--no-annotate", ], - requirements_txt = "requirements.txt", - tags = [ + requirements_txt="requirements.txt", + tags=[ "manual", ], ) # Library target py_library( - name = "lib", - srcs = glob(["lib/**/*.py"]), - visibility = ["//visibility:public"], + name="lib", + srcs=glob(["lib/**/*.py"]), + visibility=["//visibility:public"], ) # CLI library target (shared between binary and tests) py_library( - name = "cli", - srcs = glob(["cli/**/*.py"]), - data = [ + name="cli", + srcs=glob(["cli/**/*.py"]), + data=[ ":cli/misc/assets/report_template.html", ], - deps = [":lib"] + all_requirements, + deps=[":lib"] + all_requirements, ) # CLI binary target py_binary( - name = "tooling", - srcs = ["cli/main.py"], - main = "cli/main.py", - visibility = ["//visibility:public"], - deps = [":cli"], + name="tooling", + srcs=["cli/main.py"], + main="cli/main.py", + visibility=["//visibility:public"], + deps=[":cli"], ) # Workflow scripts as executables py_binary( - name = "checkout_repos", - srcs = ["cli/workflow/checkout_repos.py"], - main = "cli/workflow/checkout_repos.py", - visibility = ["//visibility:public"], - deps = [ + name="checkout_repos", + srcs=["cli/workflow/checkout_repos.py"], + main="cli/workflow/checkout_repos.py", + visibility=["//visibility:public"], + deps=[ ":cli", ":lib", - ] + all_requirements, + ] + + all_requirements, ) py_binary( - name = "recategorize_guidelines", - srcs = ["cli/workflow/recategorize_guidelines.py"], - main = "cli/workflow/recategorize_guidelines.py", - visibility = ["//visibility:public"], + name="recategorize_guidelines", + srcs=["cli/workflow/recategorize_guidelines.py"], + main="cli/workflow/recategorize_guidelines.py", + visibility=["//visibility:public"], ) # Alias: expose the resolve_deps script under //scripts/tooling so it can be # invoked as `bazel run //scripts/tooling:resolve_deps` alongside other tooling scripts. alias( - name = "resolve_deps", - actual = "//scripts/known_good:resolve_deps", - visibility = ["//visibility:public"], + name="resolve_deps", + actual="//scripts/known_good:resolve_deps", + visibility=["//visibility:public"], ) # Tests target score_py_pytest( - name = "tooling_tests", - srcs = glob(["tests/**/*.py"]), - data = [ + name="tooling_tests", + srcs=glob(["tests/**/*.py"]), + data=[ ":cli/misc/assets/report_template.html", "//:known_good.json", ], - pytest_config = "//:pyproject.toml", - deps = [ + pytest_config="//:pyproject.toml", + deps=[ ":cli", ":lib", - ] + all_requirements, + ] + + all_requirements, ) From 529351d79b80650f43c2a9d6d201ab93209e7641 Mon Sep 17 00:00:00 2001 From: subramaniak Date: Thu, 2 Jul 2026 07:10:38 +0000 Subject: [PATCH 05/14] fix: revert out-of-scope changes to module.py, update_module_from_known_good.py, and tooling BUILD format --- scripts/known_good/models/module.py | 18 ++--- .../update_module_from_known_good.py | 16 ++-- scripts/tooling/BUILD | 77 +++++++++---------- 3 files changed, 54 insertions(+), 57 deletions(-) diff --git a/scripts/known_good/models/module.py b/scripts/known_good/models/module.py index 7028fa988d1..72cae75c678 100644 --- a/scripts/known_good/models/module.py +++ b/scripts/known_good/models/module.py @@ -32,13 +32,13 @@ class Metadata: """ code_root_path: str = "//score/..." - extra_test_config: list[str] = field(default_factory=list) - exclude_test_targets: list[str] = field(default_factory=list) + extra_test_config: list[str] = field(default_factory=lambda: []) + exclude_test_targets: list[str] = field(default_factory=lambda: []) langs: list[str] = field(default_factory=lambda: ["cpp", "rust"]) - rust_coverage_config: str | None = "ferrocene-coverage" + rust_coverage_config: str | None = "ferrocene-coverage" # Optional field for Rust coverage configuration @classmethod - def from_dict(cls, data: dict[str, Any]) -> Metadata: + def from_dict(cls, data: Dict[str, Any]) -> Metadata: """Create a Metadata instance from a dictionary. Args: @@ -55,7 +55,7 @@ def from_dict(cls, data: dict[str, Any]) -> Metadata: rust_coverage_config=data.get("rust_coverage_config", "ferrocene-coverage"), ) - def to_dict(self) -> dict[str, Any]: + def to_dict(self) -> Dict[str, Any]: """Convert Metadata instance to dictionary representation. Returns: @@ -82,7 +82,7 @@ class Module: pin_version: bool = False @classmethod - def from_dict(cls, name: str, module_data: dict[str, Any]) -> Module: + def from_dict(cls, name: str, module_data: Dict[str, Any]) -> Module: """Create a Module instance from a dictionary representation. Args: @@ -149,7 +149,7 @@ def from_dict(cls, name: str, module_data: dict[str, Any]) -> Module: ) @classmethod - def parse_modules(cls, modules_dict: dict[str, Any]) -> list[Module]: + def parse_modules(cls, modules_dict: Dict[str, Any]) -> List[Module]: """Parse modules dictionary into Module dataclass instances. Args: @@ -190,13 +190,13 @@ def owner_repo(self) -> str: return f"{parts[0]}/{parts[1]}" - def to_dict(self) -> dict[str, Any]: + def to_dict(self) -> Dict[str, Any]: """Convert Module instance to dictionary representation for JSON output. Returns: Dictionary with module configuration """ - result: dict[str, Any] = {"repo": self.repo} + result: Dict[str, Any] = {"repo": self.repo} if self.version: result["version"] = self.version else: diff --git a/scripts/known_good/update_module_from_known_good.py b/scripts/known_good/update_module_from_known_good.py index b38b4045870..2d61ea2a805 100755 --- a/scripts/known_good/update_module_from_known_good.py +++ b/scripts/known_good/update_module_from_known_good.py @@ -42,7 +42,7 @@ logging.basicConfig(level=logging.WARNING, format="%(levelname)s: %(message)s") -def generate_git_override_blocks(modules: list[Module], repo_commit_dict: dict[str, str]) -> list[str]: +def generate_git_override_blocks(modules: List[Module], repo_commit_dict: Dict[str, str]) -> List[str]: """Generate bazel_dep and git_override blocks for each module.""" blocks = [] @@ -109,7 +109,7 @@ def generate_git_override_blocks(modules: list[Module], repo_commit_dict: dict[s return blocks -def generate_local_override_blocks(modules: list[Module]) -> list[str]: +def generate_local_override_blocks(modules: List[Module]) -> List[str]: """Generate bazel_dep and local_path_override blocks for each module.""" blocks = [] @@ -127,7 +127,7 @@ def generate_local_override_blocks(modules: list[Module]) -> list[str]: return blocks -def generate_coverage_blocks(modules: list[Module]) -> list[str]: +def generate_coverage_blocks(modules: List[Module]) -> List[str]: """Generate rust_coverage_report blocks for each module with rust impl.""" blocks = ["""load("@score_tooling//:defs.bzl", "rust_coverage_report")"""] @@ -161,9 +161,9 @@ def generate_coverage_blocks(modules: list[Module]) -> list[str]: def generate_file_content( args: argparse.Namespace, - modules: list[Module], - repo_commit_dict: dict[str, str], - timestamp: str | None = None, + modules: List[Module], + repo_commit_dict: Dict[str, str], + timestamp: Optional[str] = None, file_type: str = "module", ) -> str: """Generate the complete content for score_modules.MODULE.bazel.""" @@ -292,9 +292,9 @@ def main() -> None: try: known_good = load_known_good(Path(known_path)) except FileNotFoundError as e: - raise SystemExit(f"ERROR: {e}") from e + raise SystemExit(f"ERROR: {e}") except ValueError as e: - raise SystemExit(f"ERROR: {e}") from e + raise SystemExit(f"ERROR: {e}") if not known_good.modules: raise SystemExit("No modules found in known_good.json") diff --git a/scripts/tooling/BUILD b/scripts/tooling/BUILD index 6e848c58f51..d989b765db4 100644 --- a/scripts/tooling/BUILD +++ b/scripts/tooling/BUILD @@ -22,86 +22,83 @@ load("@score_tooling//python_basics:defs.bzl", "score_py_pytest") # `bazel run //scripts/tooling:requirements.update -- --upgrade` compile_pip_requirements( - name="requirements", - srcs=[ + name = "requirements", + srcs = [ "requirements.in", "@score_tooling//python_basics:requirements.txt", ], - extra_args=[ + extra_args = [ "--no-annotate", ], - requirements_txt="requirements.txt", - tags=[ + requirements_txt = "requirements.txt", + tags = [ "manual", ], ) # Library target py_library( - name="lib", - srcs=glob(["lib/**/*.py"]), - visibility=["//visibility:public"], + name = "lib", + srcs = glob(["lib/**/*.py"]), + visibility = ["//visibility:public"], ) # CLI library target (shared between binary and tests) py_library( - name="cli", - srcs=glob(["cli/**/*.py"]), - data=[ + name = "cli", + srcs = glob(["cli/**/*.py"]), + data = [ ":cli/misc/assets/report_template.html", ], - deps=[":lib"] + all_requirements, + deps = [":lib"] + all_requirements, ) # CLI binary target py_binary( - name="tooling", - srcs=["cli/main.py"], - main="cli/main.py", - visibility=["//visibility:public"], - deps=[":cli"], + name = "tooling", + srcs = ["cli/main.py"], + main = "cli/main.py", + visibility = ["//visibility:public"], + deps = [":cli"], ) # Workflow scripts as executables py_binary( - name="checkout_repos", - srcs=["cli/workflow/checkout_repos.py"], - main="cli/workflow/checkout_repos.py", - visibility=["//visibility:public"], - deps=[ + name = "checkout_repos", + srcs = ["cli/workflow/checkout_repos.py"], + main = "cli/workflow/checkout_repos.py", + visibility = ["//visibility:public"], + deps = [ ":cli", ":lib", - ] - + all_requirements, + ] + all_requirements, ) py_binary( - name="recategorize_guidelines", - srcs=["cli/workflow/recategorize_guidelines.py"], - main="cli/workflow/recategorize_guidelines.py", - visibility=["//visibility:public"], + name = "recategorize_guidelines", + srcs = ["cli/workflow/recategorize_guidelines.py"], + main = "cli/workflow/recategorize_guidelines.py", + visibility = ["//visibility:public"], ) -# Alias: expose the resolve_deps script under //scripts/tooling so it can be -# invoked as `bazel run //scripts/tooling:resolve_deps` alongside other tooling scripts. +# Alias: expose resolve_deps under //scripts/tooling for `bazel run //scripts/tooling:resolve_deps`. alias( - name="resolve_deps", - actual="//scripts/known_good:resolve_deps", - visibility=["//visibility:public"], + name = "resolve_deps", + actual = "//scripts/known_good:resolve_deps", + visibility = ["//visibility:public"], ) # Tests target score_py_pytest( - name="tooling_tests", - srcs=glob(["tests/**/*.py"]), - data=[ + name = "tooling_tests", + srcs = glob(["tests/**/*.py"]), + data = [ ":cli/misc/assets/report_template.html", "//:known_good.json", ], - pytest_config="//:pyproject.toml", - deps=[ + pytest_config = "//:pyproject.toml", + deps = [ ":cli", ":lib", - ] - + all_requirements, + ] + all_requirements, ) From 43118612a57946beb7bcf3efe3812636e998a557 Mon Sep 17 00:00:00 2001 From: subramaniak Date: Fri, 3 Jul 2026 07:40:49 +0000 Subject: [PATCH 06/14] feat: warn on unresolved declared deps, add bazel test target for known_good tests --- scripts/known_good/BUILD | 11 +++++++++++ scripts/known_good/resolved_dependencies.py | 14 ++++++++++++-- .../known_good/tests/test_resolved_dependencies.py | 12 ++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/scripts/known_good/BUILD b/scripts/known_good/BUILD index 647a539c997..12a21236bac 100644 --- a/scripts/known_good/BUILD +++ b/scripts/known_good/BUILD @@ -11,6 +11,7 @@ # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* load("@rules_python//python:defs.bzl", "py_binary", "py_library") +load("@score_tooling//python_basics:defs.bzl", "score_py_pytest") # Library target: the known_good package (models + generators). # Used as a dep by //scripts/tooling and by the resolve_deps binary below. @@ -23,6 +24,16 @@ py_library( visibility = ["//visibility:public"], ) +# Tests for the known_good package (currently: ResolvedDependencies). +# Not part of //scripts/tooling:tooling_tests, whose glob is scoped to scripts/tooling/tests/. +score_py_pytest( + name = "known_good_tests", + srcs = glob(["tests/**/*.py"]), + data = ["//:known_good.json"], + pytest_config = "//:pyproject.toml", + deps = [":known_good"], +) + # Runnable binary for the resolve + inject workflow. # Stage 1 (export): bazel run //scripts/known_good:resolve_deps -- \ # --mod-graph graph.json --export artifacts/resolved_versions.json diff --git a/scripts/known_good/resolved_dependencies.py b/scripts/known_good/resolved_dependencies.py index 543f845c1fc..05dc7801768 100644 --- a/scripts/known_good/resolved_dependencies.py +++ b/scripts/known_good/resolved_dependencies.py @@ -302,7 +302,11 @@ def overwrite(self, module_bazel: Path, *, module_under_test: str | None = None, module (and all its transitive deps) build against ref_int's resolved versions. * Skips the module under test itself (the root is never overridden). - * Skips dependencies that already carry an override in the file. + * Always overwrites: any existing override the module already declares is replaced. + * A declared dependency with no entry in the resolved set is expected not to occur + when the resolved set comes from ref_int's full ``bazel mod graph`` (it is a + superset of every module's own graph) — if it does happen, a warning is logged + and that dependency is left to resolve on its own rather than failing the run. * Re-running is idempotent: a prior injection block is replaced. """ module_bazel = Path(module_bazel) @@ -324,7 +328,13 @@ def overwrite(self, module_bazel: Path, *, module_under_test: str | None = None, continue # the module under test is the root; never override it module = self._resolved.get(name) if module is None: - continue # dep ref_int does not pin; resolves normally + logging.warning( + "%s declares %s, which has no entry in the resolved set; " + "leaving it to resolve on its own instead of failing the run.", + module_bazel, + name, + ) + continue # Strip bazel_patches: they reference //patches/... labels in ref_int's # workspace which do not exist inside another module's checkout. module = _replace(module, bazel_patches=None) diff --git a/scripts/known_good/tests/test_resolved_dependencies.py b/scripts/known_good/tests/test_resolved_dependencies.py index 436669abb39..344de47d462 100644 --- a/scripts/known_good/tests/test_resolved_dependencies.py +++ b/scripts/known_good/tests/test_resolved_dependencies.py @@ -17,6 +17,7 @@ """ import json +import logging import sys from pathlib import Path @@ -157,6 +158,17 @@ def test_idempotent(self, resolved: ResolvedDependencies, module_bazel: Path): assert first == second assert second.count(INJECTION_BEGIN) == 1 + def test_warns_on_declared_dep_not_in_resolved_set( + self, resolved: ResolvedDependencies, module_bazel: Path, caplog: pytest.LogCaptureFixture + ): + # "score_unpinned" is declared in MODULE_BAZEL but has no known_good.json entry. + # This is expected to be effectively impossible once the resolved set is sourced + # from the full 'bazel mod graph' (a superset of any module's own graph), so it + # must be surfaced as a warning rather than silently ignored. + with caplog.at_level(logging.WARNING): + resolved.overwrite(module_bazel, module_under_test="score_persistency", write=False) + assert "score_unpinned" in caplog.text + def test_overwrites_dep_with_existing_override(self, resolved: ResolvedDependencies, tmp_path: Path): # ref_int always decides the version — a pre-existing override in the module is replaced. mod = tmp_path / "MODULE.bazel" From fce6a51e7f8e5639349617b1e3c67d820135d58a Mon Sep 17 00:00:00 2001 From: subramaniak Date: Fri, 24 Jul 2026 05:04:35 +0000 Subject: [PATCH 07/14] fix: overwrite() replaces a module's own override instead of duplicating it, and make resolved_dependencies importable standalone --- scripts/known_good/resolved_dependencies.py | 48 +++++++++++++++++-- .../tests/test_resolved_dependencies.py | 4 ++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/scripts/known_good/resolved_dependencies.py b/scripts/known_good/resolved_dependencies.py index 05dc7801768..21e65136386 100644 --- a/scripts/known_good/resolved_dependencies.py +++ b/scripts/known_good/resolved_dependencies.py @@ -30,12 +30,18 @@ import json import logging import re +import sys from pathlib import Path -from known_good.models.known_good import load_known_good -from known_good.models.module import Module - _HERE = Path(__file__).resolve().parent +try: + from known_good.models.known_good import load_known_good + from known_good.models.module import Module +except ImportError: + if str(_HERE) not in sys.path: + sys.path.insert(0, str(_HERE)) + from models.known_good import load_known_good # noqa: E402 + from models.module import Module # noqa: E402 # Marker delimiting the block we append, so injection is idempotent / detectable. INJECTION_BEGIN = "# --- BEGIN ref_int resolved-deps injection ---" @@ -317,6 +323,7 @@ def overwrite(self, module_bazel: Path, *, module_under_test: str | None = None, from dataclasses import replace as _replace directives: list[str] = [] + injected_names: list[str] = [] # Inject overrides only for deps the module actually declares (intersected with the # resolved set). Bazel fails with "root module specifies overrides on nonexistent # module(s)" if an override targets a module that is not in this module's dependency @@ -342,6 +349,13 @@ def overwrite(self, module_bazel: Path, *, module_under_test: str | None = None, if directive is None: continue directives.append(directive) + injected_names.append(name) + + # ref_int's injected override must be the ONLY override for each dep. A module that + # pins a dep with its own git_override/single_version_override (e.g. score_platform) + # would otherwise trip Bazel's "multiple overrides for dep found". Remove the + # module's own override for every dep we inject so ref_int's resolved version wins. + original = _strip_existing_overrides(original, injected_names) if not directives: patched = original @@ -363,6 +377,34 @@ def _strip_injection(text: str) -> str: return pattern.sub("", text).rstrip() + "\n" if pattern.search(text) else text +_OVERRIDE_KINDS = ( + "git_override", + "single_version_override", + "archive_override", + "local_path_override", + "multiple_version_override", +) + + +def _strip_existing_overrides(text: str, names: list[str]) -> str: + """Remove any ``*_override(module_name = "", ...)`` the module declares itself. + + ref_int re-injects its own resolved override for each of ``names``; Bazel forbids two + overrides for the same module, so a module's pre-existing override must be removed first. + Matches from the override call to its closing ``)`` on its own line. + """ + if not names: + return text + kinds = "|".join(_OVERRIDE_KINDS) + for name in names: + pattern = re.compile( + r"(?:" + kinds + r")\s*\(\s*module_name\s*=\s*\"" + re.escape(name) + r"\".*?\n\)\n?", + re.S, + ) + text = pattern.sub("", text) + return text.rstrip() + "\n" + + def _field(body: str, field: str) -> str: match = _FIELD_RE(field).search(body) return match.group(1) if match else "" diff --git a/scripts/known_good/tests/test_resolved_dependencies.py b/scripts/known_good/tests/test_resolved_dependencies.py index 344de47d462..90579c91f79 100644 --- a/scripts/known_good/tests/test_resolved_dependencies.py +++ b/scripts/known_good/tests/test_resolved_dependencies.py @@ -181,6 +181,10 @@ def test_overwrites_dep_with_existing_override(self, resolved: ResolvedDependenc # ref_int's resolved commit must appear in the injection block, overwriting "deadbeef" assert 'module_name = "score_logging"' in block assert "deadbeef" not in block + # the module's OWN override must be removed from the whole file — otherwise Bazel + # aborts with "multiple overrides for dep score_logging found". + assert "deadbeef" not in patched + assert patched.count('module_name = "score_logging"') == 1 class TestFromModGraph: From d14e22874e78c968c75c12199e6afb867724b8c8 Mon Sep 17 00:00:00 2001 From: subramaniak Date: Mon, 3 Aug 2026 03:24:04 +0000 Subject: [PATCH 08/14] feat: reach one level of transitive git_override deps via a module's own MODULE.bazel --- scripts/known_good/resolved_dependencies.py | 109 ++++++++++++++++- .../tests/test_resolved_dependencies.py | 114 ++++++++++++++++++ 2 files changed, 222 insertions(+), 1 deletion(-) diff --git a/scripts/known_good/resolved_dependencies.py b/scripts/known_good/resolved_dependencies.py index 21e65136386..c0090a89902 100644 --- a/scripts/known_good/resolved_dependencies.py +++ b/scripts/known_good/resolved_dependencies.py @@ -31,6 +31,9 @@ import logging import re import sys +import urllib.error +import urllib.request +from collections.abc import Callable from pathlib import Path _HERE = Path(__file__).resolve().parent @@ -106,6 +109,46 @@ def generate_override_directive(module: Module, repo_commit_dict: dict[str, str] ) +# Returns the bazel_dep names a git-overridden dependency itself declares in its own +# MODULE.bazel, given that dependency's resolved Module (repo + commit). Used by +# ResolvedDependencies.overwrite() to reach one level of transitive pass-through — see +# fetch_module_bazel_deps for the real (network) implementation. +TransitiveFetcher = Callable[["Module"], "set[str]"] + + +def fetch_module_bazel_deps(module: Module, timeout: float = 10.0) -> set[str]: + """Fetch a git-overridden dependency's own MODULE.bazel and return its bazel_dep names. + + A module we override by commit (e.g. score_tooling) may itself declare further + bazel_deps with their own git_override inside ITS MODULE.bazel (e.g. trlc, lobster). + Bazel discards overrides declared by a non-root module, so unless the module we are + injecting into repeats them at its own root, they fall through to an unresolvable + registry lookup (e.g. "module lobster@0.0.0 not found in registries"). This lets + :meth:`ResolvedDependencies.overwrite` carry those one level deeper. + + Best-effort and github.com-only (true for every first-party score_* module and the + trlc/lobster third-party deps ref_int pins — see Module.owner_repo): any failure + (unsupported remote, network error, timeout) logs a warning and returns an empty set + rather than failing the run — same graceful-degradation philosophy as the rest of + this module. + """ + if not module.repo or not module.hash: + return set() + try: + owner_repo = module.owner_repo + except ValueError as exc: + logging.warning("Cannot fetch MODULE.bazel for %s: %s", module.name, exc) + return set() + url = f"https://raw.githubusercontent.com/{owner_repo}/{module.hash}/MODULE.bazel" + try: + with urllib.request.urlopen(url, timeout=timeout) as resp: # noqa: S310 + text = resp.read().decode("utf-8", errors="replace") + except (urllib.error.URLError, TimeoutError, ValueError) as exc: + logging.warning("Could not fetch %s to check for transitive overrides: %s", url, exc) + return set() + return _non_dev_bazel_dep_names(text) + + # The single file that carries the resolved set from Stage 1 (resolve) to Stage 2 # (per-module validation). It is the only handoff needed: first-party commits + # third-party resolved versions, merged. The lock travels alongside only as evidence. @@ -116,12 +159,35 @@ def generate_override_directive(module: Module, repo_commit_dict: dict[str, str] # Capture the module name from any ``bazel_dep(name = "...")`` call (name is the first arg). _BAZEL_DEP_RE = re.compile(r'bazel_dep\(\s*name\s*=\s*"([^"]+)"') +# The full body of a bazel_dep(...) call, to additionally check for dev_dependency = True. +_BAZEL_DEP_CALL_RE = re.compile(r"bazel_dep\((?P.*?)\)", re.S) +_DEV_DEPENDENCY_RE = re.compile(r"dev_dependency\s*=\s*True") # Parsers for reconstructing the resolved set from generated score_modules_*.MODULE.bazel. _GIT_OVERRIDE_BLOCK_RE = re.compile(r"git_override\((?P.*?)\)", re.S) _SINGLE_VERSION_BLOCK_RE = re.compile(r"single_version_override\((?P.*?)\)", re.S) _FIELD_RE = lambda field: re.compile(rf'{field}\s*=\s*"([^"]+)"') # noqa: E731 +def _non_dev_bazel_dep_names(text: str) -> set[str]: + """Names from ``bazel_dep(...)`` calls that are NOT ``dev_dependency``-only. + + A ``dev_dependency`` bazel_dep only takes effect when the declaring module is itself + the Bazel root. Used to inspect a *dependency's own* MODULE.bazel (e.g. score_tooling) + while it is never root — the module under test is — so its dev deps never enter the + actual graph. Including them would inject overrides for names Bazel does not resolve, + tripping "the root module specifies overrides on nonexistent module(s)". + """ + names: set[str] = set() + for match in _BAZEL_DEP_CALL_RE.finditer(text): + body = match.group("body") + if _DEV_DEPENDENCY_RE.search(body): + continue + name = _field(body, "name") + if name: + names.add(name) + return names + + class ResolvedDependencies: """Resolved dependency versions from the reference_integration root. @@ -300,7 +366,14 @@ def scan(self, module_bazel: Path) -> list[str]: text = self._strip_injection(text) return _BAZEL_DEP_RE.findall(text) - def overwrite(self, module_bazel: Path, *, module_under_test: str | None = None, write: bool = True) -> str: + def overwrite( + self, + module_bazel: Path, + *, + module_under_test: str | None = None, + write: bool = True, + fetch_transitive_deps: TransitiveFetcher | None = None, + ) -> str: """Overwrite a module's declared dependency versions with the resolved set. Appends a ``git_override`` / ``single_version_override`` directive for every @@ -314,6 +387,12 @@ def overwrite(self, module_bazel: Path, *, module_under_test: str | None = None, superset of every module's own graph) — if it does happen, a warning is logged and that dependency is left to resolve on its own rather than failing the run. * Re-running is idempotent: a prior injection block is replaced. + * ``fetch_transitive_deps``, if given, is used to reach one level deeper: for every + git-overridden dependency just injected (e.g. score_tooling), it returns that + dependency's own declared bazel_dep names, and any of those present in the + resolved set but not already declared by this module are injected too. Without + it (the default), only the module's own directly-declared deps are considered — + see :func:`fetch_module_bazel_deps` for the real (network-based) implementation. """ module_bazel = Path(module_bazel) original = self._strip_injection(module_bazel.read_text()) @@ -351,6 +430,33 @@ def overwrite(self, module_bazel: Path, *, module_under_test: str | None = None, directives.append(directive) injected_names.append(name) + if fetch_transitive_deps is not None: + # One level of transitive reach: a dependency we just overrode by commit (e.g. + # score_tooling) may itself declare further bazel_deps with their own + # git_override inside ITS MODULE.bazel (e.g. trlc, lobster). Bazel discards + # overrides declared by a non-root module, so unless this module repeats them + # itself, they fall through to an unresolvable registry lookup the moment we + # upgrade the carrier to ref_int's resolved commit — even though the module's + # own (older) version of that carrier never needed them. single_version_override + # carriers are registry-resolved and carry no such risk, so only git-overridden + # ones are checked. + for carrier_name in list(injected_names): + carrier = self._resolved[carrier_name] + if not carrier.repo or not carrier.hash: + continue + for sub_name in sorted(fetch_transitive_deps(carrier)): + if sub_name == module_under_test or sub_name in declared or sub_name in injected_names: + continue + sub_module = self._resolved.get(sub_name) + if sub_module is None: + continue + sub_module = _replace(sub_module, bazel_patches=None) + directive = generate_override_directive(sub_module) + if directive is None: + continue + directives.append(directive) + injected_names.append(sub_name) + # ref_int's injected override must be the ONLY override for each dep. A module that # pins a dep with its own git_override/single_version_override (e.g. score_platform) # would otherwise trip Bazel's "multiple overrides for dep found". Remove the @@ -499,6 +605,7 @@ def main() -> None: args.module_bazel, module_under_test=args.module_under_test, write=not args.dry_run, + fetch_transitive_deps=fetch_module_bazel_deps, ) if args.dry_run: print(patched) diff --git a/scripts/known_good/tests/test_resolved_dependencies.py b/scripts/known_good/tests/test_resolved_dependencies.py index 90579c91f79..e961d8a283d 100644 --- a/scripts/known_good/tests/test_resolved_dependencies.py +++ b/scripts/known_good/tests/test_resolved_dependencies.py @@ -19,7 +19,9 @@ import json import logging import sys +import urllib.error from pathlib import Path +from unittest.mock import patch import pytest @@ -28,10 +30,12 @@ if str(_SCRIPTS_DIR) not in sys.path: sys.path.insert(0, str(_SCRIPTS_DIR)) +from known_good.models.module import Module # noqa: E402 from known_good.resolved_dependencies import ( # noqa: E402 INJECTION_BEGIN, INJECTION_END, ResolvedDependencies, + fetch_module_bazel_deps, generate_override_directive, ) @@ -92,6 +96,60 @@ def resolved(known_good_file: Path) -> ResolvedDependencies: return ResolvedDependencies.from_known_good(known_good_file) +class TestFetchModuleBazelDeps: + """fetch_module_bazel_deps: the real (HTTP) implementation of TransitiveFetcher.""" + + @staticmethod + def _module(**overrides) -> Module: + defaults = {"name": "score_tooling", "hash": "abc1234", "repo": "https://github.com/eclipse-score/tooling.git"} + return Module(**{**defaults, **overrides}) + + def test_parses_bazel_dep_names_from_fetched_text(self): + fake_text = 'module(name = "score_tooling")\nbazel_dep(name = "trlc", version = "0.0.0")\n' + with patch("known_good.resolved_dependencies.urllib.request.urlopen") as mock_urlopen: + mock_urlopen.return_value.__enter__.return_value.read.return_value = fake_text.encode() + names = fetch_module_bazel_deps(self._module()) + assert names == {"trlc"} + # the raw-content URL is derived from the module's own repo + commit + (url,), _ = mock_urlopen.call_args + assert url == "https://raw.githubusercontent.com/eclipse-score/tooling/abc1234/MODULE.bazel" + + def test_excludes_dev_dependency_bazel_deps(self): + # dev_dependency bazel_deps only take effect when the DECLARING module is itself + # the Bazel root, which it never is here (see score_baselibs' own MODULE.bazel, + # which declares score_platform/toolchains_llvm as dev_dependency = True — pulling + # those in for an unrelated module trips Bazel's "overrides on nonexistent + # module(s)", since they never actually enter that module's graph). + fake_text = ( + 'bazel_dep(name = "trlc", version = "0.0.0")\n' + 'bazel_dep(name = "toolchains_llvm", version = "1.6.0", dev_dependency = True)\n' + ) + with patch("known_good.resolved_dependencies.urllib.request.urlopen") as mock_urlopen: + mock_urlopen.return_value.__enter__.return_value.read.return_value = fake_text.encode() + names = fetch_module_bazel_deps(self._module()) + assert names == {"trlc"} + + def test_missing_repo_or_hash_returns_empty_without_fetching(self): + with patch("known_good.resolved_dependencies.urllib.request.urlopen") as mock_urlopen: + assert fetch_module_bazel_deps(self._module(hash="")) == set() + assert fetch_module_bazel_deps(self._module(repo="")) == set() + mock_urlopen.assert_not_called() + + def test_unsupported_remote_returns_empty_without_fetching(self): + with patch("known_good.resolved_dependencies.urllib.request.urlopen") as mock_urlopen: + names = fetch_module_bazel_deps(self._module(repo="https://gitlab.com/example/repo.git")) + assert names == set() + mock_urlopen.assert_not_called() + + def test_network_failure_returns_empty_instead_of_raising(self, caplog: pytest.LogCaptureFixture): + with patch("known_good.resolved_dependencies.urllib.request.urlopen") as mock_urlopen: + mock_urlopen.side_effect = urllib.error.URLError("boom") + with caplog.at_level(logging.WARNING): + names = fetch_module_bazel_deps(self._module()) + assert names == set() + assert "boom" in caplog.text + + class TestFromKnownGood: def test_names_span_all_groups(self, resolved: ResolvedDependencies): assert {"score_baselibs", "score_logging", "score_persistency", "score_tooling"} <= resolved.names @@ -187,6 +245,62 @@ def test_overwrites_dep_with_existing_override(self, resolved: ResolvedDependenc assert patched.count('module_name = "score_logging"') == 1 +class TestOverwriteTransitive: + """fetch_transitive_deps: one level of transitive reach (e.g. score_tooling -> trlc/lobster).""" + + def test_injects_dep_found_via_carrier(self, resolved: ResolvedDependencies, tmp_path: Path): + # Module declares ONLY score_baselibs. score_logging is never declared directly, + # only discovered because the fake fetcher reports it as something score_baselibs' + # own MODULE.bazel declares. + mod = tmp_path / "MODULE.bazel" + mod.write_text('module(name = "score_persistency", version = "0.0.0")\nbazel_dep(name = "score_baselibs")\n') + + def fake_fetch(module: Module) -> set[str]: + return {"score_logging"} if module.name == "score_baselibs" else set() + + patched = resolved.overwrite( + mod, module_under_test="score_persistency", write=False, fetch_transitive_deps=fake_fetch + ) + block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] + assert 'module_name = "score_baselibs"' in block + assert 'module_name = "score_logging"' in block + + def test_without_fetcher_transitive_dep_not_injected(self, resolved: ResolvedDependencies, tmp_path: Path): + # Default behaviour (no fetch_transitive_deps given) is unchanged: only directly + # declared deps are considered. + mod = tmp_path / "MODULE.bazel" + mod.write_text('module(name = "score_persistency", version = "0.0.0")\nbazel_dep(name = "score_baselibs")\n') + patched = resolved.overwrite(mod, module_under_test="score_persistency", write=False) + assert 'module_name = "score_logging"' not in patched + + def test_does_not_duplicate_already_declared_dep(self, resolved: ResolvedDependencies, module_bazel: Path): + # score_logging is already declared directly in MODULE_BAZEL; even though the fake + # fetcher also reports it via score_baselibs, it must not be injected twice. + def fake_fetch(module: Module) -> set[str]: + return {"score_logging"} if module.name == "score_baselibs" else set() + + patched = resolved.overwrite( + module_bazel, module_under_test="score_persistency", write=False, fetch_transitive_deps=fake_fetch + ) + assert patched.count('module_name = "score_logging"') == 1 + + def test_skips_single_version_override_carriers(self, resolved: ResolvedDependencies, module_bazel: Path): + # score_tooling is declared with a plain version (single_version_override kind) in + # this fixture -> registry-resolved, no non-root-override risk, so the fetcher must + # not even be called for it. score_baselibs/score_logging ARE git-overridden. + calls: list[str] = [] + + def fake_fetch(module: Module) -> set[str]: + calls.append(module.name) + return set() + + resolved.overwrite( + module_bazel, module_under_test="score_persistency", write=False, fetch_transitive_deps=fake_fetch + ) + assert "score_tooling" not in calls + assert "score_baselibs" in calls + + class TestFromModGraph: @staticmethod def _graph() -> dict: From 29590007564741ca92f3cac8f89e88cb7b523a52 Mon Sep 17 00:00:00 2001 From: subramaniak Date: Thu, 6 Aug 2026 13:09:55 +0000 Subject: [PATCH 09/14] feat: pin a module's full transitive closure from the Stage-1 graph instead of only its declared deps --- scripts/known_good/BUILD | 18 +- scripts/known_good/resolved_dependencies.py | 412 ++++++++++++------ .../tests/test_resolved_dependencies.py | 211 +++++---- 3 files changed, 405 insertions(+), 236 deletions(-) diff --git a/scripts/known_good/BUILD b/scripts/known_good/BUILD index 12a21236bac..11f18e00a6d 100644 --- a/scripts/known_good/BUILD +++ b/scripts/known_good/BUILD @@ -35,10 +35,20 @@ score_py_pytest( ) # Runnable binary for the resolve + inject workflow. -# Stage 1 (export): bazel run //scripts/known_good:resolve_deps -- \ -# --mod-graph graph.json --export artifacts/resolved_versions.json -# Stage 2 (inject): bazel run //scripts/known_good:resolve_deps -- \ -# _module/MODULE.bazel --resolved-deps _resolved_deps/ +# +# Stage 1 (export) — 'bazel mod graph' is a prerequisite; run it first and pass the result: +# bazel mod graph --output=json > graph.json +# bazel run //scripts/known_good:resolve_deps -- \ +# --mod-graph graph.json --export _resolved_deps/resolved_versions.json +# Writes the manifest and stores graph.json next to it; both are published as the +# stage1-resolved-deps artifact. Paths are resolved against BUILD_WORKSPACE_DIRECTORY, +# so graph.json does not need to be listed in data = [...]. +# +# Stage 2 (inject) — consumes that same directory: +# bazel run //scripts/known_good:resolve_deps -- \ +# _module/MODULE.bazel --resolved-deps _resolved_deps/ +# The manifest supplies each module's resolved version; graph.json identifies the +# module-under-test's transitive closure so all of it is pinned, not only direct deps. py_binary( name = "resolve_deps", srcs = ["resolved_dependencies.py"], diff --git a/scripts/known_good/resolved_dependencies.py b/scripts/known_good/resolved_dependencies.py index c0090a89902..aa6ae25ccea 100644 --- a/scripts/known_good/resolved_dependencies.py +++ b/scripts/known_good/resolved_dependencies.py @@ -29,14 +29,27 @@ import argparse import json import logging +import os import re import sys -import urllib.error -import urllib.request -from collections.abc import Callable from pathlib import Path _HERE = Path(__file__).resolve().parent + + +def _repo_root() -> Path: + """ref_int's workspace root. + + Prefers the environment Bazel sets for ``bazel run`` targets so paths passed on the + command line resolve against the user's workspace rather than the runfiles tree (and + so ``graph.json`` need not be declared in ``data = [...]``). Falls back to walking up + from this file for direct ``python3 scripts/...`` invocations. + """ + for var in ("BUILD_WORKSPACE_DIRECTORY", "BUILD_WORKING_DIRECTORY"): + value = os.environ.get(var) + if value: + return Path(value) + return _HERE.parents[1] try: from known_good.models.known_good import load_known_good from known_good.models.module import Module @@ -109,44 +122,84 @@ def generate_override_directive(module: Module, repo_commit_dict: dict[str, str] ) -# Returns the bazel_dep names a git-overridden dependency itself declares in its own -# MODULE.bazel, given that dependency's resolved Module (repo + commit). Used by -# ResolvedDependencies.overwrite() to reach one level of transitive pass-through — see -# fetch_module_bazel_deps for the real (network) implementation. -TransitiveFetcher = Callable[["Module"], "set[str]"] +# The file Stage 1 stores alongside the manifest so Stage 2 can determine, for a given +# module, which *transitive* dependencies need an override (see DependencyGraph). +GRAPH_NAME = "graph.json" -def fetch_module_bazel_deps(module: Module, timeout: float = 10.0) -> set[str]: - """Fetch a git-overridden dependency's own MODULE.bazel and return its bazel_dep names. +class DependencyGraph: + """The ``bazel mod graph --output=json`` tree, queryable per module. - A module we override by commit (e.g. score_tooling) may itself declare further - bazel_deps with their own git_override inside ITS MODULE.bazel (e.g. trlc, lobster). - Bazel discards overrides declared by a non-root module, so unless the module we are - injecting into repeats them at its own root, they fall through to an unresolvable - registry lookup (e.g. "module lobster@0.0.0 not found in registries"). This lets - :meth:`ResolvedDependencies.overwrite` carry those one level deeper. + Stage 2 needs more than the module-under-test's *declared* dependencies: Bazel only + honours ``*_override`` directives from the **root** module, so any transitive dep the + module does not itself declare falls through to plain MVS and can resolve to a version + ref_int never validated (e.g. ``score_communication`` never declares ``flatbuffers``; + it arrives via ``score_baselibs``). :meth:`closure` returns the full set so + :meth:`ResolvedDependencies.overwrite` can pin every one of them. - Best-effort and github.com-only (true for every first-party score_* module and the - trlc/lobster third-party deps ref_int pins — see Module.owner_repo): any failure - (unsupported remote, network error, timeout) logs a warning and returns an empty set - rather than failing the run — same graceful-degradation philosophy as the rest of - this module. + The graph is *not* a plain tree. A module that appears more than once is emitted once + with its ``dependencies`` and thereafter as an ``unexpanded`` stub carrying no + children (in ref_int's graph: 865 unexpanded vs 157 expanded nodes). Walking the + subtree naively would therefore miss most of the closure, so nodes are indexed by name + on load and unexpanded references are resolved through that index. """ - if not module.repo or not module.hash: - return set() - try: - owner_repo = module.owner_repo - except ValueError as exc: - logging.warning("Cannot fetch MODULE.bazel for %s: %s", module.name, exc) - return set() - url = f"https://raw.githubusercontent.com/{owner_repo}/{module.hash}/MODULE.bazel" - try: - with urllib.request.urlopen(url, timeout=timeout) as resp: # noqa: S310 - text = resp.read().decode("utf-8", errors="replace") - except (urllib.error.URLError, TimeoutError, ValueError) as exc: - logging.warning("Could not fetch %s to check for transitive overrides: %s", url, exc) - return set() - return _non_dev_bazel_dep_names(text) + + def __init__(self, root: dict): + self._index: dict[str, dict] = {} + self._build_index(root) + + def _build_index(self, node: dict, seen: set[int] | None = None) -> None: + seen = set() if seen is None else seen + if id(node) in seen: + return + seen.add(id(node)) + name = node.get("name") + # Only expanded nodes carry children; the first occurrence is the authoritative one. + if name and not node.get("unexpanded") and "dependencies" in node: + self._index.setdefault(name, node) + for dep in node.get("dependencies") or []: + self._build_index(dep, seen) + + @classmethod + def from_file(cls, path: Path) -> DependencyGraph: + path = Path(path) + if not path.is_file(): + raise FileNotFoundError( + f"Dependency graph {path} not found. Stage 1 must produce it with " + f"'bazel mod graph --output=json' and store it as {GRAPH_NAME} in the " + f"stage1-resolved-deps artifact." + ) + return cls(json.loads(path.read_text())) + + @property + def names(self) -> set[str]: + return set(self._index) + + def closure(self, module_name: str) -> set[str]: + """Every module reachable from ``module_name``, excluding itself. + + Traversal follows ``dependencies`` and ``indirectDependencies``, resolving + ``unexpanded`` stubs via the name index. A ``visited`` set guards the ``cycles`` + the graph schema can carry. + """ + visited: set[str] = set() + stack = [module_name] + while stack: + node = self._index.get(stack.pop()) + if node is None: + continue # unexpanded-only or absent: nothing further to walk + for dep in node.get("dependencies") or []: + name = dep.get("name") + if name and name not in visited: + visited.add(name) + stack.append(name) + for dep in node.get("indirectDependencies") or []: + name = dep if isinstance(dep, str) else dep.get("name") + if name and name not in visited: + visited.add(name) + stack.append(name) + visited.discard(module_name) + return visited # The single file that carries the resolved set from Stage 1 (resolve) to Stage 2 @@ -159,33 +212,47 @@ def fetch_module_bazel_deps(module: Module, timeout: float = 10.0) -> set[str]: # Capture the module name from any ``bazel_dep(name = "...")`` call (name is the first arg). _BAZEL_DEP_RE = re.compile(r'bazel_dep\(\s*name\s*=\s*"([^"]+)"') -# The full body of a bazel_dep(...) call, to additionally check for dev_dependency = True. -_BAZEL_DEP_CALL_RE = re.compile(r"bazel_dep\((?P.*?)\)", re.S) -_DEV_DEPENDENCY_RE = re.compile(r"dev_dependency\s*=\s*True") # Parsers for reconstructing the resolved set from generated score_modules_*.MODULE.bazel. _GIT_OVERRIDE_BLOCK_RE = re.compile(r"git_override\((?P.*?)\)", re.S) _SINGLE_VERSION_BLOCK_RE = re.compile(r"single_version_override\((?P.*?)\)", re.S) +# multiple_version_override pins several versions of one module simultaneously; unlike the +# other two it cannot be represented by a single Module.version, so it is carried +# separately (see ResolvedDependencies._multi). +_MULTIPLE_VERSION_BLOCK_RE = re.compile(r"multiple_version_override\((?P.*?)\)", re.S) +_VERSIONS_LIST_RE = re.compile(r"versions\s*=\s*\[(?P.*?)\]", re.S) _FIELD_RE = lambda field: re.compile(rf'{field}\s*=\s*"([^"]+)"') # noqa: E731 -def _non_dev_bazel_dep_names(text: str) -> set[str]: - """Names from ``bazel_dep(...)`` calls that are NOT ``dev_dependency``-only. +def _parse_versions_list(body: str) -> list[str]: + """Extract the string items of a ``versions = [...]`` keyword argument.""" + match = _VERSIONS_LIST_RE.search(body) + return re.findall(r'"([^"]+)"', match.group("items")) if match else [] + + +def generate_multiple_version_override(module_name: str, versions: list[str]) -> str: + """Return a ``multiple_version_override`` directive for a module pinned to several versions.""" + items = "".join(f' "{v}",\n' for v in versions) + return f'multiple_version_override(\n module_name = "{module_name}",\n versions = [\n{items} ],\n)\n' - A ``dev_dependency`` bazel_dep only takes effect when the declaring module is itself - the Bazel root. Used to inspect a *dependency's own* MODULE.bazel (e.g. score_tooling) - while it is never root — the module under test is — so its dev deps never enter the - actual graph. Including them would inject overrides for names Bazel does not resolve, - tripping "the root module specifies overrides on nonexistent module(s)". + +def generate_bazel_dep(module: Module | None, name: str) -> str: + """Return the ``bazel_dep`` line that brings ``name`` into the root module's graph. + + An override is only legal for a module Bazel actually resolves; injecting one for a + transitive dependency the module-under-test does not declare would trip "the root + module specifies overrides on nonexistent module(s)". Declaring the ``bazel_dep`` + alongside the override is what makes it valid. + + The version is deliberate: a registry module repeats its *resolved* version so it + matches what MVS selects and ``--check_direct_dependencies`` stays quiet, while a + git-overridden module omits the version entirely (the override supplies the source, + and any literal here — ``0.0.0`` included — would only produce a spurious mismatch + warning). Omitting it is the same idiom the score modules already use, e.g. + ``bazel_dep(name = "score_tooling")``. """ - names: set[str] = set() - for match in _BAZEL_DEP_CALL_RE.finditer(text): - body = match.group("body") - if _DEV_DEPENDENCY_RE.search(body): - continue - name = _field(body, "name") - if name: - names.add(name) - return names + if module is not None and module.version: + return f'bazel_dep(name = "{name}", version = "{module.version}")\n' + return f'bazel_dep(name = "{name}")\n' class ResolvedDependencies: @@ -195,8 +262,11 @@ class ResolvedDependencies: interface to scan + overwrite a module's ``MODULE.bazel`` to those versions. """ - def __init__(self, resolved: dict[str, Module]): + def __init__(self, resolved: dict[str, Module], multi: dict[str, list[str]] | None = None): self._resolved = resolved + # name -> versions, for modules ref_int pins with multiple_version_override. Kept + # apart from _resolved because a Module carries exactly one version. + self._multi = multi or {} # -- construction: "resolved deps versions from ref_int root" -------------------- @@ -265,6 +335,7 @@ def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> Res be represented and are logged as not carried. """ resolved: dict[str, Module] = {} + multi: dict[str, list[str]] = {} unrepresentable: list[str] = [] for f in override_files: # Drop comment-only lines first: hand-written MODULE.bazel files contain @@ -273,6 +344,11 @@ def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> Res text = "\n".join(ln for ln in Path(f).read_text().splitlines() if not ln.lstrip().startswith("#")) for module in cls._parse_override_file(text): # git_override + single_version_override resolved[module.name] = module + for block in _MULTIPLE_VERSION_BLOCK_RE.finditer(text): + body = block.group("body") + name, versions = _field(body, "module_name"), _parse_versions_list(body) + if name and versions: + multi[name] = versions for m in re.finditer(r'(archive_override|local_path_override)\(\s*module_name\s*=\s*"([^"]+)"', text): unrepresentable.append(f"{m.group(2)} ({m.group(1)})") @@ -281,7 +357,7 @@ def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> Res _collect_resolved_versions(graph, versions) skipped: list[str] = [] for name, version in versions.items(): - if name in resolved or name in _SKIP_MODULES: + if name in resolved or name in multi or name in _SKIP_MODULES: continue # already carried by an override directive, or non-overridable if not version or version == "0.0.0": # Non-registry version: ref_int pins it via an override we did not capture @@ -298,7 +374,7 @@ def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> Res logging.warning( "Graph modules at version 0.0.0 with no carried override, skipped: %s", ", ".join(sorted(skipped)) ) - return cls(resolved) + return cls(resolved, multi) def to_file(self, path: Path) -> None: """Serialize the resolved set to the JSON manifest (Stage 1 -> Stage 2 handoff). @@ -308,21 +384,25 @@ def to_file(self, path: Path) -> None: Metadata is intentionally omitted — the manifest carries dependency pins, not the module-under-test's test configuration (that comes from known_good.json). """ - modules = {} + modules: dict[str, dict[str, object]] = {} for name in sorted(self._resolved): m = self._resolved[name] entry: dict[str, object] = {"version": m.version} if m.version else {"repo": m.repo, "hash": m.hash} if m.bazel_patches: entry["bazel_patches"] = m.bazel_patches modules[name] = entry - Path(path).write_text(json.dumps({"modules": modules}, indent=2) + "\n") + for name, versions in self._multi.items(): + modules[name] = {"versions": versions} + Path(path).write_text(json.dumps({"modules": dict(sorted(modules.items()))}, indent=2) + "\n") @classmethod def from_file(cls, path: Path) -> ResolvedDependencies: """Load a resolved set previously written by :meth:`to_file`.""" data = json.loads(Path(path).read_text()) - resolved = {name: Module.from_dict(name, md) for name, md in data.get("modules", {}).items()} - return cls(resolved) + entries = data.get("modules", {}) + multi = {name: md["versions"] for name, md in entries.items() if md.get("versions")} + resolved = {name: Module.from_dict(name, md) for name, md in entries.items() if name not in multi} + return cls(resolved, multi) @staticmethod def _parse_override_file(text: str) -> list[Module]: @@ -350,12 +430,17 @@ def _parse_override_file(text: str) -> list[Module]: @property def names(self) -> set[str]: - return set(self._resolved) + return set(self._resolved) | set(self._multi) @property def modules(self) -> dict[str, Module]: return dict(self._resolved) + @property + def multiple_versions(self) -> dict[str, list[str]]: + """Modules ref_int pins with ``multiple_version_override`` -> their versions.""" + return dict(self._multi) + def get(self, name: str) -> Module | None: return self._resolved.get(name) @@ -372,90 +457,86 @@ def overwrite( *, module_under_test: str | None = None, write: bool = True, - fetch_transitive_deps: TransitiveFetcher | None = None, + graph: DependencyGraph | None = None, ) -> str: - """Overwrite a module's declared dependency versions with the resolved set. + """Overwrite a module's dependency versions with ref_int's resolved set. + + Appends an override directive for every dependency in scope, so the module builds + and tests against exactly the versions ref_int resolved in Stage 1. - Appends a ``git_override`` / ``single_version_override`` directive for every - dependency the module declares that we have a resolved version for, so the - module (and all its transitive deps) build against ref_int's resolved versions. + Scope is the module's **transitive closure** when ``graph`` is supplied, not just + the dependencies it declares. Bazel honours ``*_override`` only from the root + module, so a transitive dependency the module does not itself declare would + otherwise fall through to plain MVS and can select a version ref_int never + validated (``score_communication`` never declares ``flatbuffers``; it arrives via + ``score_baselibs``). For each closure member that is not already declared, a + ``bazel_dep`` is emitted alongside the override — an override for a module absent + from the graph is rejected by Bazel as "the root module specifies overrides on + nonexistent module(s)", and the ``bazel_dep`` is what makes it legal. + + Without ``graph`` only declared dependencies are pinned, which leaves that + transitive gap open; Stage 2 always passes one. * Skips the module under test itself (the root is never overridden). * Always overwrites: any existing override the module already declares is replaced. - * A declared dependency with no entry in the resolved set is expected not to occur - when the resolved set comes from ref_int's full ``bazel mod graph`` (it is a - superset of every module's own graph) — if it does happen, a warning is logged - and that dependency is left to resolve on its own rather than failing the run. + * A dependency with no entry in the resolved set is left to resolve on its own and + logged. This is expected and structural rather than a defect: a module's + ``dev_dependency`` deps activate only when it is the root, which is true in + Stage 2 but not in Stage 1, so ref_int's graph never saw them. * Re-running is idempotent: a prior injection block is replaced. - * ``fetch_transitive_deps``, if given, is used to reach one level deeper: for every - git-overridden dependency just injected (e.g. score_tooling), it returns that - dependency's own declared bazel_dep names, and any of those present in the - resolved set but not already declared by this module are injected too. Without - it (the default), only the module's own directly-declared deps are considered — - see :func:`fetch_module_bazel_deps` for the real (network-based) implementation. """ module_bazel = Path(module_bazel) original = self._strip_injection(module_bazel.read_text()) declared = set(_BAZEL_DEP_RE.findall(original)) + module_under_test = module_under_test or _module_name_of(original) from dataclasses import replace as _replace + in_scope = set(declared) + if graph is not None and module_under_test: + # The closure and nothing beyond it: pinning only what is already in this + # module's graph keeps the injection faithful to Stage 1 without pulling new + # modules (and the toolchains they register) into the build. Members with no + # resolved entry are reported below rather than filtered out silently. + in_scope |= graph.closure(module_under_test) + directives: list[str] = [] injected_names: list[str] = [] - # Inject overrides only for deps the module actually declares (intersected with the - # resolved set). Bazel fails with "root module specifies overrides on nonexistent - # module(s)" if an override targets a module that is not in this module's dependency - # graph, so the full resolved set cannot be injected wholesale — a declared bazel_dep - # is by definition in the graph, which makes its override safe. - # ref_int always decides the version — any existing module-level override is replaced. - for name in sorted(declared): - if name == module_under_test: + unresolved: list[str] = [] + for name in sorted(in_scope): + if name == module_under_test or name in _SKIP_MODULES: continue # the module under test is the root; never override it - module = self._resolved.get(name) - if module is None: - logging.warning( - "%s declares %s, which has no entry in the resolved set; " - "leaving it to resolve on its own instead of failing the run.", - module_bazel, - name, - ) - continue - # Strip bazel_patches: they reference //patches/... labels in ref_int's - # workspace which do not exist inside another module's checkout. - module = _replace(module, bazel_patches=None) - directive = generate_override_directive(module) + if name in self._multi: + directive: str | None = generate_multiple_version_override(name, self._multi[name]) + module = None + else: + module = self._resolved.get(name) + if module is None: + unresolved.append(name) + continue + # Strip bazel_patches: they reference //patches/... labels in ref_int's + # workspace which do not exist inside another module's checkout. + module = _replace(module, bazel_patches=None) + directive = generate_override_directive(module) if directive is None: continue + # Only closure members the module does not declare need the bazel_dep line; + # emitting a second one for a declared dep would be a duplicate declaration. + if name not in declared: + directives.append(generate_bazel_dep(module, name)) directives.append(directive) injected_names.append(name) - if fetch_transitive_deps is not None: - # One level of transitive reach: a dependency we just overrode by commit (e.g. - # score_tooling) may itself declare further bazel_deps with their own - # git_override inside ITS MODULE.bazel (e.g. trlc, lobster). Bazel discards - # overrides declared by a non-root module, so unless this module repeats them - # itself, they fall through to an unresolvable registry lookup the moment we - # upgrade the carrier to ref_int's resolved commit — even though the module's - # own (older) version of that carrier never needed them. single_version_override - # carriers are registry-resolved and carry no such risk, so only git-overridden - # ones are checked. - for carrier_name in list(injected_names): - carrier = self._resolved[carrier_name] - if not carrier.repo or not carrier.hash: - continue - for sub_name in sorted(fetch_transitive_deps(carrier)): - if sub_name == module_under_test or sub_name in declared or sub_name in injected_names: - continue - sub_module = self._resolved.get(sub_name) - if sub_module is None: - continue - sub_module = _replace(sub_module, bazel_patches=None) - directive = generate_override_directive(sub_module) - if directive is None: - continue - directives.append(directive) - injected_names.append(sub_name) + if unresolved: + logging.warning( + "%s: no entry in the resolved set for %s; leaving them to resolve on their own. " + "Expected for dev_dependency-only deps (active only when the module is root, " + "so absent from ref_int's Stage 1 graph) and for modules ref_int pins with an " + "archive_override/local_path_override.", + module_bazel, + ", ".join(unresolved), + ) # ref_int's injected override must be the ONLY override for each dep. A module that # pins a dep with its own git_override/single_version_override (e.g. score_platform) @@ -516,6 +597,33 @@ def _field(body: str, field: str) -> str: return match.group(1) if match else "" +# A module declares its own name in the module(...) call at the top of its MODULE.bazel. +_MODULE_DECL_RE = re.compile(r"module\(\s*name\s*=\s*\"([^\"]+)\"", re.S) + + +def injected_override_names(module_bazel_text: str) -> set[str]: + """Module names ref_int injected an override for, read back from a patched MODULE.bazel. + + The authoritative answer to "did ref_int pin this?" — used by the Stage 2 verification + to tell an override that failed to take effect (ref_int's bug) from a dependency that + was never pinned at all (the module resolved it on its own). + """ + if INJECTION_BEGIN not in module_bazel_text: + return set() + block = module_bazel_text.split(INJECTION_BEGIN, 1)[1].split(INJECTION_END, 1)[0] + return set(re.findall(r'_override\(\s*module_name\s*=\s*"([^"]+)"', block)) + + +def _module_name_of(module_bazel_text: str) -> str: + """The module's own name, from the ``module(name = "...")`` call in its MODULE.bazel. + + Lets Stage 2 identify the module under test from the file itself, so the caller need + not also pass ``--module-under-test``. + """ + match = _MODULE_DECL_RE.search(module_bazel_text) + return match.group(1) if match else "" + + def _collect_resolved_versions(node: dict, acc: dict[str, str]) -> None: """Walk a ``bazel mod graph --output=json`` tree, recording name -> resolved version. @@ -544,14 +652,17 @@ def _parse_args() -> argparse.Namespace: parser.add_argument( "--known-good-path", type=Path, - default=_HERE.parents[1] / "known_good.json", - help="Resolved set source: known_good.json (default; first-party commit pins).", + default=None, + help="Export mode only: known_good.json (defaults to ref_int's). Not a valid inject source.", ) parser.add_argument( "--resolved-deps", type=Path, default=None, - help="Inject mode: Stage-1 stage1-resolved-deps artifact dir (overrides --known-good-path).", + help=( + "Inject mode (required): Stage-1 stage1-resolved-deps artifact dir, holding " + f"{MANIFEST_NAME} and {GRAPH_NAME}." + ), ) parser.add_argument( "--mod-graph", @@ -583,29 +694,50 @@ def main() -> None: if args.export is not None: if args.mod_graph is None: raise SystemExit("--export requires --mod-graph (output of 'bazel mod graph --output=json')") - repo_root = _HERE.parents[1] - override_files = [repo_root / "MODULE.bazel", *sorted((repo_root / "bazel_common").glob("*.MODULE.bazel"))] - override_files = [f for f in override_files if f.is_file()] - resolved = ResolvedDependencies.from_mod_graph(args.mod_graph, override_files) - Path(args.export).parent.mkdir(parents=True, exist_ok=True) - resolved.to_file(args.export) - print(f"Wrote resolved dependency manifest ({len(resolved.names)} modules) to {args.export}") + mod_graph = Path(args.mod_graph) + if not mod_graph.is_file(): + raise SystemExit( + f"--mod-graph {mod_graph} does not exist. Produce it first with: " + "bazel mod graph --output=json > graph.json" + ) + repo_root = _repo_root() + override_files = [ + f + for f in [repo_root / "MODULE.bazel", *sorted((repo_root / "bazel_common").glob("*.MODULE.bazel"))] + if f.is_file() + ] + resolved = ResolvedDependencies.from_mod_graph(mod_graph, override_files) + export = Path(args.export) + export.parent.mkdir(parents=True, exist_ok=True) + resolved.to_file(export) + # Stage 2 needs the graph too: the manifest says which version each module resolves + # to, the graph says which of them a given module actually depends on. + graph_copy = export.parent / GRAPH_NAME + graph_copy.write_text(mod_graph.read_text()) + print(f"Wrote resolved dependency manifest ({len(resolved.names)} modules) to {export}") + print(f"Stored dependency graph for Stage 2 at {graph_copy}") return # Inject mode (Stage 2): overwrite a module's MODULE.bazel with the resolved set. if args.module_bazel is None: raise SystemExit("module_bazel is required unless --export is given") - if args.resolved_deps: - resolved = ResolvedDependencies.from_resolved_artifact(args.resolved_deps) - else: - resolved = ResolvedDependencies.from_known_good(args.known_good_path) + # known_good.json is not a valid inject source: it carries only first-party score + # modules with no transitive registry versions, so the closure could not be pinned. + if not args.resolved_deps: + raise SystemExit( + "--resolved-deps is required for inject mode: Stage 2 must pin against the " + "Stage-1 resolved set. known_good.json carries only first-party pins and no " + "transitive versions, so it cannot back the injection." + ) + resolved = ResolvedDependencies.from_resolved_artifact(args.resolved_deps) + graph = DependencyGraph.from_file(Path(args.resolved_deps) / GRAPH_NAME) patched = resolved.overwrite( args.module_bazel, module_under_test=args.module_under_test, write=not args.dry_run, - fetch_transitive_deps=fetch_module_bazel_deps, + graph=graph, ) if args.dry_run: print(patched) diff --git a/scripts/known_good/tests/test_resolved_dependencies.py b/scripts/known_good/tests/test_resolved_dependencies.py index e961d8a283d..c3d7d0132fe 100644 --- a/scripts/known_good/tests/test_resolved_dependencies.py +++ b/scripts/known_good/tests/test_resolved_dependencies.py @@ -19,9 +19,7 @@ import json import logging import sys -import urllib.error from pathlib import Path -from unittest.mock import patch import pytest @@ -30,12 +28,11 @@ if str(_SCRIPTS_DIR) not in sys.path: sys.path.insert(0, str(_SCRIPTS_DIR)) -from known_good.models.module import Module # noqa: E402 from known_good.resolved_dependencies import ( # noqa: E402 INJECTION_BEGIN, INJECTION_END, + DependencyGraph, ResolvedDependencies, - fetch_module_bazel_deps, generate_override_directive, ) @@ -96,58 +93,57 @@ def resolved(known_good_file: Path) -> ResolvedDependencies: return ResolvedDependencies.from_known_good(known_good_file) -class TestFetchModuleBazelDeps: - """fetch_module_bazel_deps: the real (HTTP) implementation of TransitiveFetcher.""" +def _node(name: str, version: str = "1.0", deps: list[dict] | None = None, **extra) -> dict: + """An expanded graph node, matching 'bazel mod graph --output=json'.""" + return {"name": name, "version": version, "dependencies": deps or [], "indirectDependencies": [], **extra} - @staticmethod - def _module(**overrides) -> Module: - defaults = {"name": "score_tooling", "hash": "abc1234", "repo": "https://github.com/eclipse-score/tooling.git"} - return Module(**{**defaults, **overrides}) - - def test_parses_bazel_dep_names_from_fetched_text(self): - fake_text = 'module(name = "score_tooling")\nbazel_dep(name = "trlc", version = "0.0.0")\n' - with patch("known_good.resolved_dependencies.urllib.request.urlopen") as mock_urlopen: - mock_urlopen.return_value.__enter__.return_value.read.return_value = fake_text.encode() - names = fetch_module_bazel_deps(self._module()) - assert names == {"trlc"} - # the raw-content URL is derived from the module's own repo + commit - (url,), _ = mock_urlopen.call_args - assert url == "https://raw.githubusercontent.com/eclipse-score/tooling/abc1234/MODULE.bazel" - - def test_excludes_dev_dependency_bazel_deps(self): - # dev_dependency bazel_deps only take effect when the DECLARING module is itself - # the Bazel root, which it never is here (see score_baselibs' own MODULE.bazel, - # which declares score_platform/toolchains_llvm as dev_dependency = True — pulling - # those in for an unrelated module trips Bazel's "overrides on nonexistent - # module(s)", since they never actually enter that module's graph). - fake_text = ( - 'bazel_dep(name = "trlc", version = "0.0.0")\n' - 'bazel_dep(name = "toolchains_llvm", version = "1.6.0", dev_dependency = True)\n' + +def _unexpanded(name: str, version: str = "1.0") -> dict: + """A repeated reference: no 'dependencies' key, so it must be resolved via the index.""" + return {"name": name, "version": version, "unexpanded": True} + + +class TestDependencyGraph: + """Closure computation over the mod graph, including its unexpanded-node encoding.""" + + def test_closure_follows_transitive_edges(self): + baselibs = _node("score_baselibs", deps=[_node("flatbuffers")]) + graph = DependencyGraph(_node("", "", [_node("score_persistency", deps=[baselibs])])) + assert graph.closure("score_persistency") == {"score_baselibs", "flatbuffers"} + + def test_closure_resolves_unexpanded_references(self): + # Bazel emits a module's children only at its first occurrence; every later + # occurrence is an 'unexpanded' stub. Walking the subtree literally would stop at + # the stub and miss flatbuffers, which is exactly the gap this must not have. + graph = DependencyGraph( + _node( + "", + "", + [ + _node("score_baselibs", deps=[_node("flatbuffers")]), + _node("score_communication", deps=[_unexpanded("score_baselibs")]), + ], + ) ) - with patch("known_good.resolved_dependencies.urllib.request.urlopen") as mock_urlopen: - mock_urlopen.return_value.__enter__.return_value.read.return_value = fake_text.encode() - names = fetch_module_bazel_deps(self._module()) - assert names == {"trlc"} - - def test_missing_repo_or_hash_returns_empty_without_fetching(self): - with patch("known_good.resolved_dependencies.urllib.request.urlopen") as mock_urlopen: - assert fetch_module_bazel_deps(self._module(hash="")) == set() - assert fetch_module_bazel_deps(self._module(repo="")) == set() - mock_urlopen.assert_not_called() - - def test_unsupported_remote_returns_empty_without_fetching(self): - with patch("known_good.resolved_dependencies.urllib.request.urlopen") as mock_urlopen: - names = fetch_module_bazel_deps(self._module(repo="https://gitlab.com/example/repo.git")) - assert names == set() - mock_urlopen.assert_not_called() - - def test_network_failure_returns_empty_instead_of_raising(self, caplog: pytest.LogCaptureFixture): - with patch("known_good.resolved_dependencies.urllib.request.urlopen") as mock_urlopen: - mock_urlopen.side_effect = urllib.error.URLError("boom") - with caplog.at_level(logging.WARNING): - names = fetch_module_bazel_deps(self._module()) - assert names == set() - assert "boom" in caplog.text + assert graph.closure("score_communication") == {"score_baselibs", "flatbuffers"} + + def test_closure_excludes_the_module_itself(self): + graph = DependencyGraph(_node("", "", [_node("score_time", deps=[_node("rules_cc")])])) + assert "score_time" not in graph.closure("score_time") + + def test_closure_terminates_on_cycles(self): + a = _node("a") + b = _node("b", deps=[_unexpanded("a")]) + a["dependencies"] = [b] + assert DependencyGraph(_node("", "", [a])).closure("a") == {"b"} + + def test_closure_of_unknown_module_is_empty(self): + graph = DependencyGraph(_node("", "", [_node("score_time")])) + assert graph.closure("not_in_graph") == set() + + def test_from_file_reports_a_missing_graph(self, tmp_path: Path): + with pytest.raises(FileNotFoundError, match="bazel mod graph"): + DependencyGraph.from_file(tmp_path / "graph.json") class TestFromKnownGood: @@ -246,59 +242,90 @@ def test_overwrites_dep_with_existing_override(self, resolved: ResolvedDependenc class TestOverwriteTransitive: - """fetch_transitive_deps: one level of transitive reach (e.g. score_tooling -> trlc/lobster).""" + """Closure injection: pin transitive deps the module never declares itself.""" - def test_injects_dep_found_via_carrier(self, resolved: ResolvedDependencies, tmp_path: Path): - # Module declares ONLY score_baselibs. score_logging is never declared directly, - # only discovered because the fake fetcher reports it as something score_baselibs' - # own MODULE.bazel declares. - mod = tmp_path / "MODULE.bazel" - mod.write_text('module(name = "score_persistency", version = "0.0.0")\nbazel_dep(name = "score_baselibs")\n') + @staticmethod + def _graph() -> DependencyGraph: + # score_persistency -> score_baselibs -> score_logging. Only score_baselibs is + # declared directly by the module; score_logging arrives through it. + return DependencyGraph( + _node( + "", + "", + [_node("score_persistency", deps=[_node("score_baselibs", deps=[_node("score_logging")])])], + ) + ) - def fake_fetch(module: Module) -> set[str]: - return {"score_logging"} if module.name == "score_baselibs" else set() + @pytest.fixture + def only_baselibs(self, tmp_path: Path) -> Path: + p = tmp_path / "MODULE.bazel" + p.write_text('module(name = "score_persistency", version = "0.0.0")\nbazel_dep(name = "score_baselibs")\n') + return p + def test_injects_transitive_dep_with_its_bazel_dep(self, resolved: ResolvedDependencies, only_baselibs: Path): patched = resolved.overwrite( - mod, module_under_test="score_persistency", write=False, fetch_transitive_deps=fake_fetch + only_baselibs, module_under_test="score_persistency", write=False, graph=self._graph() ) block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] assert 'module_name = "score_baselibs"' in block assert 'module_name = "score_logging"' in block + # The override alone would be rejected ("overrides on nonexistent module(s)") since + # the module never declares score_logging — the bazel_dep is what makes it legal. + assert 'bazel_dep(name = "score_logging")' in block - def test_without_fetcher_transitive_dep_not_injected(self, resolved: ResolvedDependencies, tmp_path: Path): - # Default behaviour (no fetch_transitive_deps given) is unchanged: only directly - # declared deps are considered. - mod = tmp_path / "MODULE.bazel" - mod.write_text('module(name = "score_persistency", version = "0.0.0")\nbazel_dep(name = "score_baselibs")\n') - patched = resolved.overwrite(mod, module_under_test="score_persistency", write=False) - assert 'module_name = "score_logging"' not in patched - - def test_does_not_duplicate_already_declared_dep(self, resolved: ResolvedDependencies, module_bazel: Path): - # score_logging is already declared directly in MODULE_BAZEL; even though the fake - # fetcher also reports it via score_baselibs, it must not be injected twice. - def fake_fetch(module: Module) -> set[str]: - return {"score_logging"} if module.name == "score_baselibs" else set() - + def test_declared_dep_gets_no_extra_bazel_dep(self, resolved: ResolvedDependencies, only_baselibs: Path): + # score_baselibs is already declared above the block; re-declaring it would be a + # duplicate declaration of the same module. patched = resolved.overwrite( - module_bazel, module_under_test="score_persistency", write=False, fetch_transitive_deps=fake_fetch + only_baselibs, module_under_test="score_persistency", write=False, graph=self._graph() ) - assert patched.count('module_name = "score_logging"') == 1 + block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] + assert "bazel_dep" not in block.split('module_name = "score_baselibs"')[0] + assert patched.count('bazel_dep(name = "score_baselibs")') == 1 - def test_skips_single_version_override_carriers(self, resolved: ResolvedDependencies, module_bazel: Path): - # score_tooling is declared with a plain version (single_version_override kind) in - # this fixture -> registry-resolved, no non-root-override risk, so the fetcher must - # not even be called for it. score_baselibs/score_logging ARE git-overridden. - calls: list[str] = [] + def test_registry_dep_stub_repeats_the_resolved_version(self, resolved: ResolvedDependencies, tmp_path: Path): + # score_tooling is registry-pinned (version 1.2.0). Its stub must carry that exact + # version so it matches MVS and --check_direct_dependencies stays quiet; a + # git-overridden module instead gets a bare bazel_dep with no version at all. + mod = tmp_path / "MODULE.bazel" + mod.write_text('module(name = "score_persistency", version = "0.0.0")\n') + graph = DependencyGraph( + _node("", "", [_node("score_persistency", deps=[_node("score_tooling"), _node("score_logging")])]) + ) + block = ( + resolved.overwrite(mod, module_under_test="score_persistency", write=False, graph=graph) + .split(INJECTION_BEGIN)[1] + .split(INJECTION_END)[0] + ) + assert 'bazel_dep(name = "score_tooling", version = "1.2.0")' in block + assert 'bazel_dep(name = "score_logging")\n' in block - def fake_fetch(module: Module) -> set[str]: - calls.append(module.name) - return set() + def test_without_graph_only_declared_deps_are_pinned(self, resolved: ResolvedDependencies, only_baselibs: Path): + patched = resolved.overwrite(only_baselibs, module_under_test="score_persistency", write=False) + assert 'module_name = "score_logging"' not in patched - resolved.overwrite( - module_bazel, module_under_test="score_persistency", write=False, fetch_transitive_deps=fake_fetch + def test_closure_member_absent_from_resolved_set_is_skipped( + self, resolved: ResolvedDependencies, only_baselibs: Path, caplog: pytest.LogCaptureFixture + ): + # A module's dev_dependency deps activate only when it is root — true in Stage 2 but + # not in Stage 1 — so ref_int's graph never saw them. Warn, never fail. + graph = DependencyGraph( + _node("", "", [_node("score_persistency", deps=[_node("rules_doxygen")])]), ) - assert "score_tooling" not in calls - assert "score_baselibs" in calls + with caplog.at_level(logging.WARNING): + patched = resolved.overwrite( + only_baselibs, module_under_test="score_persistency", write=False, graph=graph + ) + assert "rules_doxygen" not in patched + assert "rules_doxygen" in caplog.text + + def test_module_under_test_inferred_from_module_declaration( + self, resolved: ResolvedDependencies, only_baselibs: Path + ): + # module(name = "...") identifies the root, so --module-under-test is optional. + patched = resolved.overwrite(only_baselibs, write=False, graph=self._graph()) + assert 'module_name = "score_persistency"' not in patched + assert 'module_name = "score_baselibs"' in patched class TestFromModGraph: From f9b4afef343817f83a8fb62b83238c1e8e6aee60 Mon Sep 17 00:00:00 2001 From: subramaniak Date: Tue, 11 Aug 2026 09:59:52 +0000 Subject: [PATCH 10/14] fix: key the Stage-2 pin scope on ref_int's resolved set --- scripts/known_good/resolved_dependencies.py | 248 ++++++++---------- .../tests/test_resolved_dependencies.py | 245 ++++++++++++----- scripts/tooling/BUILD | 7 - 3 files changed, 287 insertions(+), 213 deletions(-) diff --git a/scripts/known_good/resolved_dependencies.py b/scripts/known_good/resolved_dependencies.py index aa6ae25ccea..f7609350d99 100644 --- a/scripts/known_good/resolved_dependencies.py +++ b/scripts/known_good/resolved_dependencies.py @@ -50,6 +50,8 @@ def _repo_root() -> Path: if value: return Path(value) return _HERE.parents[1] + + try: from known_good.models.known_good import load_known_good from known_good.models.module import Module @@ -139,9 +141,9 @@ class DependencyGraph: The graph is *not* a plain tree. A module that appears more than once is emitted once with its ``dependencies`` and thereafter as an ``unexpanded`` stub carrying no - children (in ref_int's graph: 865 unexpanded vs 157 expanded nodes). Walking the - subtree naively would therefore miss most of the closure, so nodes are indexed by name - on load and unexpanded references are resolved through that index. + children (864 of ref_int's 1022 nodes). Walking the subtree naively would therefore miss + most of the closure, so nodes are indexed by name on load and unexpanded references are + resolved through that index. """ def __init__(self, root: dict): @@ -178,9 +180,9 @@ def names(self) -> set[str]: def closure(self, module_name: str) -> set[str]: """Every module reachable from ``module_name``, excluding itself. - Traversal follows ``dependencies`` and ``indirectDependencies``, resolving - ``unexpanded`` stubs via the name index. A ``visited`` set guards the ``cycles`` - the graph schema can carry. + ``unexpanded`` stubs are resolved via the name index; ``visited`` terminates the walk, + since the graph is a DAG with diamonds. Correct only for a graph produced without + ``--depth`` -- both callers omit it, so every edge is a ``dependencies`` entry. """ visited: set[str] = set() stack = [module_name] @@ -193,11 +195,6 @@ def closure(self, module_name: str) -> set[str]: if name and name not in visited: visited.add(name) stack.append(name) - for dep in node.get("indirectDependencies") or []: - name = dep if isinstance(dep, str) else dep.get("name") - if name and name not in visited: - visited.add(name) - stack.append(name) visited.discard(module_name) return visited @@ -212,27 +209,29 @@ def closure(self, module_name: str) -> set[str]: # Capture the module name from any ``bazel_dep(name = "...")`` call (name is the first arg). _BAZEL_DEP_RE = re.compile(r'bazel_dep\(\s*name\s*=\s*"([^"]+)"') -# Parsers for reconstructing the resolved set from generated score_modules_*.MODULE.bazel. +# The whole ``bazel_dep(...)`` argument list. ``[^)]*`` is sufficient: bazel_dep takes only +# scalar keyword arguments, never a nested call. +_BAZEL_DEP_CALL_RE = re.compile(r"bazel_dep\((?P[^)]*)\)", re.S) +# The two override kinds ref_int declares, each mapping onto a single Module. +# multiple_version_override is unsupported: ref_int declares none. archive_override / +# local_path_override cannot be reproduced at all and are reported instead. _GIT_OVERRIDE_BLOCK_RE = re.compile(r"git_override\((?P.*?)\)", re.S) _SINGLE_VERSION_BLOCK_RE = re.compile(r"single_version_override\((?P.*?)\)", re.S) -# multiple_version_override pins several versions of one module simultaneously; unlike the -# other two it cannot be represented by a single Module.version, so it is carried -# separately (see ResolvedDependencies._multi). -_MULTIPLE_VERSION_BLOCK_RE = re.compile(r"multiple_version_override\((?P.*?)\)", re.S) -_VERSIONS_LIST_RE = re.compile(r"versions\s*=\s*\[(?P.*?)\]", re.S) _FIELD_RE = lambda field: re.compile(rf'{field}\s*=\s*"([^"]+)"') # noqa: E731 -def _parse_versions_list(body: str) -> list[str]: - """Extract the string items of a ``versions = [...]`` keyword argument.""" - match = _VERSIONS_LIST_RE.search(body) - return re.findall(r'"([^"]+)"', match.group("items")) if match else [] - +def _declared_deps(text: str) -> set[str]: + """Every dependency a module declares via ``bazel_dep``, dev-declared ones included. -def generate_multiple_version_override(module_name: str, versions: list[str]) -> str: - """Return a ``multiple_version_override`` directive for a module pinned to several versions.""" - items = "".join(f' "{v}",\n' for v in versions) - return f'multiple_version_override(\n module_name = "{module_name}",\n versions = [\n{items} ],\n)\n' + The ``dev_dependency`` flag is deliberately not reported: it does not decide whether ref_int + pins a dependency -- presence in the resolved set does. See :meth:`ResolvedDependencies.overwrite`. + """ + declared: set[str] = set() + for call in _BAZEL_DEP_CALL_RE.finditer(text): + name = _FIELD_RE("name").search(call.group("body")) + if name is not None: + declared.add(name.group(1)) + return declared def generate_bazel_dep(module: Module | None, name: str) -> str: @@ -258,21 +257,22 @@ def generate_bazel_dep(module: Module | None, name: str) -> str: class ResolvedDependencies: """Resolved dependency versions from the reference_integration root. - Holds a ``name -> Module`` map of the dependencies ref_int pins, and provides an - interface to scan + overwrite a module's ``MODULE.bazel`` to those versions. + Holds a ``name -> Module`` map of the dependencies ref_int pins, and provides the + :meth:`overwrite` interface that pins a module's ``MODULE.bazel`` to those versions. """ - def __init__(self, resolved: dict[str, Module], multi: dict[str, list[str]] | None = None): + def __init__(self, resolved: dict[str, Module]): self._resolved = resolved - # name -> versions, for modules ref_int pins with multiple_version_override. Kept - # apart from _resolved because a Module carries exactly one version. - self._multi = multi or {} # -- construction: "resolved deps versions from ref_int root" -------------------- @classmethod def from_known_good(cls, known_good_path: Path) -> ResolvedDependencies: - """Build from ``known_good.json`` (local / dev source of the resolved pins).""" + """Build from ``known_good.json`` — first-party pins only. Tests and local inspection. + + Not an injection source, and ``main()`` rejects it as one: it carries no transitive + registry versions and no graph, so the closure cannot be pinned from it. + """ kg = load_known_good(Path(known_good_path).resolve()) resolved: dict[str, Module] = {} for group in kg.modules.values(): @@ -284,35 +284,21 @@ def from_known_good(cls, known_good_path: Path) -> ResolvedDependencies: def from_resolved_artifact(cls, artifact_dir: Path) -> ResolvedDependencies: """Build from the Stage-1 ``stage1-resolved-deps`` artifact. - The handoff is the single ``resolved_versions.json`` manifest (see - :meth:`from_mod_graph` / :meth:`to_file`). For backward compatibility, if the - manifest is absent the older format is parsed: the generated - ``score_modules_*.MODULE.bazel`` override files, gated on the presence of - ``MODULE.bazel.lock`` as evidence of full resolution. + The handoff is the ``resolved_versions.json`` manifest :meth:`to_file` writes. + ``graph.json`` sits beside it, loaded separately by :class:`DependencyGraph`; + ``MODULE.bazel.lock`` travels along as evidence and is not read. A missing manifest is + fatal -- Stage 2 cannot pin anything without the versions MVS selected. """ artifact_dir = Path(artifact_dir) manifest = artifact_dir / MANIFEST_NAME - if manifest.is_file(): - return cls.from_file(manifest) - - # Legacy fallback: reconstruct from the generated override files. - lock = artifact_dir / "MODULE.bazel.lock" - if not lock.is_file(): + if not manifest.is_file(): raise FileNotFoundError( - f"Neither {MANIFEST_NAME} nor MODULE.bazel.lock found in resolved-deps artifact " - f"{artifact_dir}; Stage 2 must consume the Stage-1 resolved dependency set." + f"No {MANIFEST_NAME} in resolved-deps artifact {artifact_dir}; Stage 2 must consume " + f"the Stage-1 resolved dependency set, which Stage 1 writes with " + f"'resolved_dependencies.py --mod-graph --export '." ) - - module_files = sorted(artifact_dir.glob("score_modules_*.MODULE.bazel")) - if not module_files: - raise FileNotFoundError(f"No score_modules_*.MODULE.bazel files in resolved-deps artifact {artifact_dir}.") - - resolved: dict[str, Module] = {} - for mf in module_files: - for module in cls._parse_override_file(mf.read_text()): - resolved[module.name] = module - return cls(resolved) + return cls.from_file(manifest) @classmethod def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> ResolvedDependencies: @@ -335,7 +321,6 @@ def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> Res be represented and are logged as not carried. """ resolved: dict[str, Module] = {} - multi: dict[str, list[str]] = {} unrepresentable: list[str] = [] for f in override_files: # Drop comment-only lines first: hand-written MODULE.bazel files contain @@ -344,11 +329,6 @@ def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> Res text = "\n".join(ln for ln in Path(f).read_text().splitlines() if not ln.lstrip().startswith("#")) for module in cls._parse_override_file(text): # git_override + single_version_override resolved[module.name] = module - for block in _MULTIPLE_VERSION_BLOCK_RE.finditer(text): - body = block.group("body") - name, versions = _field(body, "module_name"), _parse_versions_list(body) - if name and versions: - multi[name] = versions for m in re.finditer(r'(archive_override|local_path_override)\(\s*module_name\s*=\s*"([^"]+)"', text): unrepresentable.append(f"{m.group(2)} ({m.group(1)})") @@ -357,11 +337,12 @@ def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> Res _collect_resolved_versions(graph, versions) skipped: list[str] = [] for name, version in versions.items(): - if name in resolved or name in multi or name in _SKIP_MODULES: + if name in resolved or name in _SKIP_MODULES: continue # already carried by an override directive, or non-overridable - if not version or version == "0.0.0": - # Non-registry version: ref_int pins it via an override we did not capture - # (e.g. archive_override). single_version_override cannot reproduce it. + # 0.0.0 means ref_int pins it via an override this parser did not capture (an + # archive_override), which single_version_override cannot reproduce. Empty versions + # are already dropped by _collect_resolved_versions. + if version == "0.0.0": skipped.append(name) continue resolved[name] = Module(name=name, hash="", repo="", version=version) @@ -374,7 +355,7 @@ def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> Res logging.warning( "Graph modules at version 0.0.0 with no carried override, skipped: %s", ", ".join(sorted(skipped)) ) - return cls(resolved, multi) + return cls(resolved) def to_file(self, path: Path) -> None: """Serialize the resolved set to the JSON manifest (Stage 1 -> Stage 2 handoff). @@ -391,8 +372,6 @@ def to_file(self, path: Path) -> None: if m.bazel_patches: entry["bazel_patches"] = m.bazel_patches modules[name] = entry - for name, versions in self._multi.items(): - modules[name] = {"versions": versions} Path(path).write_text(json.dumps({"modules": dict(sorted(modules.items()))}, indent=2) + "\n") @classmethod @@ -400,13 +379,11 @@ def from_file(cls, path: Path) -> ResolvedDependencies: """Load a resolved set previously written by :meth:`to_file`.""" data = json.loads(Path(path).read_text()) entries = data.get("modules", {}) - multi = {name: md["versions"] for name, md in entries.items() if md.get("versions")} - resolved = {name: Module.from_dict(name, md) for name, md in entries.items() if name not in multi} - return cls(resolved, multi) + return cls({name: Module.from_dict(name, md) for name, md in entries.items()}) @staticmethod def _parse_override_file(text: str) -> list[Module]: - """Reconstruct Module objects from generated git/single_version override blocks.""" + """Reconstruct Module objects from ref_int's own git/single_version override blocks.""" modules: list[Module] = [] for match in _GIT_OVERRIDE_BLOCK_RE.finditer(text): @@ -426,80 +403,70 @@ def _parse_override_file(text: str) -> list[Module]: return modules - # -- interface: scan + overwrite a module's MODULE.bazel ------------------------- + # -- interface: overwrite a module's MODULE.bazel --------------------------------- @property def names(self) -> set[str]: - return set(self._resolved) | set(self._multi) + return set(self._resolved) @property def modules(self) -> dict[str, Module]: return dict(self._resolved) - @property - def multiple_versions(self) -> dict[str, list[str]]: - """Modules ref_int pins with ``multiple_version_override`` -> their versions.""" - return dict(self._multi) - def get(self, name: str) -> Module | None: return self._resolved.get(name) - def scan(self, module_bazel: Path) -> list[str]: - """Return the names of dependencies a module declares via ``bazel_dep``.""" - text = Path(module_bazel).read_text() - # Ignore anything inside a previous injection block so re-scans are stable. - text = self._strip_injection(text) - return _BAZEL_DEP_RE.findall(text) - def overwrite( self, module_bazel: Path, + graph: DependencyGraph, *, module_under_test: str | None = None, write: bool = True, - graph: DependencyGraph | None = None, ) -> str: """Overwrite a module's dependency versions with ref_int's resolved set. - Appends an override directive for every dependency in scope, so the module builds - and tests against exactly the versions ref_int resolved in Stage 1. + The rule is *presence in the resolved set*, not how the module declares a dependency: - Scope is the module's **transitive closure** when ``graph`` is supplied, not just - the dependencies it declares. Bazel honours ``*_override`` only from the root - module, so a transitive dependency the module does not itself declare would - otherwise fall through to plain MVS and can select a version ref_int never - validated (``score_communication`` never declares ``flatbuffers``; it arrives via - ``score_baselibs``). For each closure member that is not already declared, a - ``bazel_dep`` is emitted alongside the override — an override for a module absent - from the graph is rejected by Bazel as "the root module specifies overrides on - nonexistent module(s)", and the ``bazel_dep`` is what makes it legal. + * ref_int resolved a version or commit for it -> pin it, whether the module declares it + ``dev_dependency`` or not. ref_int has an answer, so it imposes it. + * ref_int resolved nothing for it -> leave the module's own declaration untouched and log + it. There is nothing to impose. - Without ``graph`` only declared dependencies are pinned, which leaves that - transitive gap open; Stage 2 always passes one. + ``dev_dependency`` is not the discriminator and is never read. The two properties are + independent: ``score_baselibs`` declares 11 dev-only deps ref_int *has* resolved + (``score_tooling``, ``score_docs_as_code``, ``toolchains_llvm``, ...), while + ``score_baselibs_rust`` is a public dep ref_int has *not* resolved. A dependency is absent + from the resolved set because nothing in ref_int's own graph reaches it -- usually it is + only reachable through some module's dev edge, inactive while ref_int is root -- or because + ref_int pins it with an ``archive_override`` the manifest cannot express. + + Scope is the module's declared deps plus ``closure()`` of the module and of each declared + dep. The closure is what makes the rule above safe rather than merely permissive: pinning + ``score_tooling`` without ``lobster``/``trlc`` aborts with ``module lobster@0.0.0 not found + in registries``, since those are non-registry and resolvable only via a root override. + ``graph`` is therefore required -- a caller that cannot supply the closure must not pin. + + Each closure member the module does not declare gets a ``bazel_dep`` alongside its + override, without which Bazel rejects it as an override on a nonexistent module. * Skips the module under test itself (the root is never overridden). - * Always overwrites: any existing override the module already declares is replaced. - * A dependency with no entry in the resolved set is left to resolve on its own and - logged. This is expected and structural rather than a defect: a module's - ``dev_dependency`` deps activate only when it is the root, which is true in - Stage 2 but not in Stage 1, so ref_int's graph never saw them. - * Re-running is idempotent: a prior injection block is replaced. + * Always overwrites an existing override; re-running replaces a prior block. """ module_bazel = Path(module_bazel) original = self._strip_injection(module_bazel.read_text()) - declared = set(_BAZEL_DEP_RE.findall(original)) + declared = _declared_deps(original) module_under_test = module_under_test or _module_name_of(original) from dataclasses import replace as _replace - in_scope = set(declared) - if graph is not None and module_under_test: - # The closure and nothing beyond it: pinning only what is already in this - # module's graph keeps the injection faithful to Stage 1 without pulling new - # modules (and the toolchains they register) into the build. Members with no - # resolved entry are reported below rather than filtered out silently. - in_scope |= graph.closure(module_under_test) + # The module's own closure, plus each declared dep's closure. The second is what reaches + # deps of a dev-declared module: Stage 1 has the modules as nodes but not the edge, since + # a dev edge is inactive unless its declaring module is root. + in_scope = set(declared) | graph.closure(module_under_test) + for dep in declared: + in_scope |= graph.closure(dep) directives: list[str] = [] injected_names: list[str] = [] @@ -507,18 +474,14 @@ def overwrite( for name in sorted(in_scope): if name == module_under_test or name in _SKIP_MODULES: continue # the module under test is the root; never override it - if name in self._multi: - directive: str | None = generate_multiple_version_override(name, self._multi[name]) - module = None - else: - module = self._resolved.get(name) - if module is None: - unresolved.append(name) - continue - # Strip bazel_patches: they reference //patches/... labels in ref_int's - # workspace which do not exist inside another module's checkout. - module = _replace(module, bazel_patches=None) - directive = generate_override_directive(module) + module = self._resolved.get(name) + if module is None: + unresolved.append(name) + continue + # Strip bazel_patches: they reference //patches/... labels in ref_int's + # workspace which do not exist inside another module's checkout. + module = _replace(module, bazel_patches=None) + directive = generate_override_directive(module) if directive is None: continue # Only closure members the module does not declare need the bazel_dep line; @@ -530,10 +493,9 @@ def overwrite( if unresolved: logging.warning( - "%s: no entry in the resolved set for %s; leaving them to resolve on their own. " - "Expected for dev_dependency-only deps (active only when the module is root, " - "so absent from ref_int's Stage 1 graph) and for modules ref_int pins with an " - "archive_override/local_path_override.", + "%s: ref_int resolved no version for %s; left as the module declares them. Expected " + "when nothing in ref_int's own graph reaches a dependency, or when ref_int pins it " + "with an archive_override/local_path_override the manifest cannot express.", module_bazel, ", ".join(unresolved), ) @@ -578,17 +540,21 @@ def _strip_existing_overrides(text: str, names: list[str]) -> str: ref_int re-injects its own resolved override for each of ``names``; Bazel forbids two overrides for the same module, so a module's pre-existing override must be removed first. - Matches from the override call to its closing ``)`` on its own line. + Only for ``names``; an override for a dep ref_int does not inject is left alone. + + Both layouts must be matched -- a surviving one makes ref_int's the *second* override and + Bazel aborts with "multiple overrides for dep found". Two patterns rather than one, since + each layout has an unambiguous terminator and a pattern covering both would also swallow a + neighbouring override. """ if not names: return text kinds = "|".join(_OVERRIDE_KINDS) for name in names: - pattern = re.compile( - r"(?:" + kinds + r")\s*\(\s*module_name\s*=\s*\"" + re.escape(name) + r"\".*?\n\)\n?", - re.S, - ) - text = pattern.sub("", text) + head = r"(?:" + kinds + r")\s*\(\s*module_name\s*=\s*\"" + re.escape(name) + r"\"" + exploded = re.compile(head + r".*?\n\)\n?", re.S) + single_line = re.compile(head + r"[^)\n]*\)[ \t]*\n?") + text = single_line.sub("", exploded.sub("", text)) return text.rstrip() + "\n" @@ -649,12 +615,6 @@ def _parse_args() -> argparse.Namespace: default=None, help="Inject mode: path to the module's MODULE.bazel to overwrite. Omit when using --export.", ) - parser.add_argument( - "--known-good-path", - type=Path, - default=None, - help="Export mode only: known_good.json (defaults to ref_int's). Not a valid inject source.", - ) parser.add_argument( "--resolved-deps", type=Path, @@ -735,9 +695,9 @@ def main() -> None: patched = resolved.overwrite( args.module_bazel, + graph, module_under_test=args.module_under_test, write=not args.dry_run, - graph=graph, ) if args.dry_run: print(patched) diff --git a/scripts/known_good/tests/test_resolved_dependencies.py b/scripts/known_good/tests/test_resolved_dependencies.py index c3d7d0132fe..a0968ee0994 100644 --- a/scripts/known_good/tests/test_resolved_dependencies.py +++ b/scripts/known_good/tests/test_resolved_dependencies.py @@ -28,12 +28,13 @@ if str(_SCRIPTS_DIR) not in sys.path: sys.path.insert(0, str(_SCRIPTS_DIR)) +from known_good.models.module import Module # noqa: E402 from known_good.resolved_dependencies import ( # noqa: E402 INJECTION_BEGIN, INJECTION_END, + MANIFEST_NAME, DependencyGraph, ResolvedDependencies, - generate_override_directive, ) KNOWN_GOOD = { @@ -93,6 +94,17 @@ def resolved(known_good_file: Path) -> ResolvedDependencies: return ResolvedDependencies.from_known_good(known_good_file) +@pytest.fixture +def flat_graph() -> DependencyGraph: + """MODULE_BAZEL's modules as edgeless nodes, so every closure is empty and scope == declared. + + overwrite() requires a graph, since without one it cannot honour "pin nothing whose closure I + do not own". Tests that are not about closure use this to isolate the rest of the behaviour. + """ + names = ["score_persistency", "rules_cc", "score_baselibs", "score_logging", "score_tooling", "score_unpinned"] + return DependencyGraph(_node("", "", [_node(n) for n in names])) + + def _node(name: str, version: str = "1.0", deps: list[dict] | None = None, **extra) -> dict: """An expanded graph node, matching 'bazel mod graph --output=json'.""" return {"name": name, "version": version, "dependencies": deps or [], "indirectDependencies": [], **extra} @@ -157,17 +169,11 @@ def test_version_module_kept(self, resolved: ResolvedDependencies): assert resolved.get("score_tooling").version == "1.2.0" -class TestScan: - def test_returns_declared_deps(self, resolved: ResolvedDependencies, module_bazel: Path): - declared = resolved.scan(module_bazel) - assert "score_baselibs" in declared - assert "score_unpinned" in declared - assert "rules_cc" in declared - - class TestOverwrite: - def test_pins_declared_resolved_siblings(self, resolved: ResolvedDependencies, module_bazel: Path): - patched = resolved.overwrite(module_bazel, module_under_test="score_persistency", write=False) + def test_pins_declared_resolved_siblings( + self, resolved: ResolvedDependencies, module_bazel: Path, flat_graph: DependencyGraph + ): + patched = resolved.overwrite(module_bazel, flat_graph, module_under_test="score_persistency", write=False) block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] assert 'git_override(\n module_name = "score_baselibs"' in block assert 'commit = "cab36dd7de92aaaaaaaaaaaaaaaaaaaaaaaaaaaa"' in block @@ -175,14 +181,16 @@ def test_pins_declared_resolved_siblings(self, resolved: ResolvedDependencies, m assert 'single_version_override(\n module_name = "score_tooling"' in block assert 'version = "1.2.0"' in block - def test_strips_patches(self, resolved: ResolvedDependencies, module_bazel: Path): + def test_strips_patches(self, resolved: ResolvedDependencies, module_bazel: Path, flat_graph: DependencyGraph): # bazel_patches reference //patches/... labels that exist only in ref_int's # workspace, so they are stripped from the injected overrides. - patched = resolved.overwrite(module_bazel, module_under_test="score_persistency", write=False) + patched = resolved.overwrite(module_bazel, flat_graph, module_under_test="score_persistency", write=False) assert "patches/baselibs/001-fix.patch" not in patched assert "patch_strip" not in patched - def test_skips_resolved_dep_not_declared(self, resolved: ResolvedDependencies, tmp_path: Path): + def test_skips_resolved_dep_not_declared( + self, resolved: ResolvedDependencies, tmp_path: Path, flat_graph: DependencyGraph + ): # Only declared deps are injected. Overriding a module that is NOT in the module's # dependency graph makes Bazel fail ("overrides on nonexistent module(s)"), so a # resolved dep the module does not declare must NOT be injected. @@ -191,46 +199,56 @@ def test_skips_resolved_dep_not_declared(self, resolved: ResolvedDependencies, t 'module(name = "score_persistency", version = "0.0.0")\n' 'bazel_dep(name = "score_baselibs", version = "0.1")\n' ) - block = resolved.overwrite(mod, module_under_test="score_persistency", write=False).split(INJECTION_BEGIN)[1] + block = resolved.overwrite(mod, flat_graph, module_under_test="score_persistency", write=False).split( + INJECTION_BEGIN + )[1] assert 'module_name = "score_baselibs"' in block # declared -> injected assert 'module_name = "score_logging"' not in block # not declared -> not injected - def test_skips_root_module(self, resolved: ResolvedDependencies, module_bazel: Path): - patched = resolved.overwrite(module_bazel, module_under_test="score_persistency", write=False) + def test_skips_root_module(self, resolved: ResolvedDependencies, module_bazel: Path, flat_graph: DependencyGraph): + patched = resolved.overwrite(module_bazel, flat_graph, module_under_test="score_persistency", write=False) block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] assert 'module_name = "score_persistency"' not in block - def test_skips_unpinned_third_party(self, resolved: ResolvedDependencies, module_bazel: Path): - patched = resolved.overwrite(module_bazel, module_under_test="score_persistency", write=False) + def test_skips_unpinned_third_party( + self, resolved: ResolvedDependencies, module_bazel: Path, flat_graph: DependencyGraph + ): + patched = resolved.overwrite(module_bazel, flat_graph, module_under_test="score_persistency", write=False) block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] assert "score_unpinned" not in block assert "rules_cc" not in block - def test_idempotent(self, resolved: ResolvedDependencies, module_bazel: Path): - first = resolved.overwrite(module_bazel, module_under_test="score_persistency", write=True) - second = resolved.overwrite(module_bazel, module_under_test="score_persistency", write=True) + def test_idempotent(self, resolved: ResolvedDependencies, module_bazel: Path, flat_graph: DependencyGraph): + first = resolved.overwrite(module_bazel, flat_graph, module_under_test="score_persistency", write=True) + second = resolved.overwrite(module_bazel, flat_graph, module_under_test="score_persistency", write=True) assert first == second assert second.count(INJECTION_BEGIN) == 1 def test_warns_on_declared_dep_not_in_resolved_set( - self, resolved: ResolvedDependencies, module_bazel: Path, caplog: pytest.LogCaptureFixture + self, + resolved: ResolvedDependencies, + module_bazel: Path, + flat_graph: DependencyGraph, + caplog: pytest.LogCaptureFixture, ): # "score_unpinned" is declared in MODULE_BAZEL but has no known_good.json entry. # This is expected to be effectively impossible once the resolved set is sourced # from the full 'bazel mod graph' (a superset of any module's own graph), so it # must be surfaced as a warning rather than silently ignored. with caplog.at_level(logging.WARNING): - resolved.overwrite(module_bazel, module_under_test="score_persistency", write=False) + resolved.overwrite(module_bazel, flat_graph, module_under_test="score_persistency", write=False) assert "score_unpinned" in caplog.text - def test_overwrites_dep_with_existing_override(self, resolved: ResolvedDependencies, tmp_path: Path): + def test_overwrites_dep_with_existing_override( + self, resolved: ResolvedDependencies, tmp_path: Path, flat_graph: DependencyGraph + ): # ref_int always decides the version — a pre-existing override in the module is replaced. mod = tmp_path / "MODULE.bazel" mod.write_text( MODULE_BAZEL + '\ngit_override(\n module_name = "score_logging",\n commit = "deadbeef",\n' ' remote = "https://example.com/x.git",\n)\n' ) - patched = resolved.overwrite(mod, module_under_test="score_persistency", write=False) + patched = resolved.overwrite(mod, flat_graph, module_under_test="score_persistency", write=False) block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] # ref_int's resolved commit must appear in the injection block, overwriting "deadbeef" assert 'module_name = "score_logging"' in block @@ -240,6 +258,29 @@ def test_overwrites_dep_with_existing_override(self, resolved: ResolvedDependenc assert "deadbeef" not in patched assert patched.count('module_name = "score_logging"') == 1 + def test_overwrites_dep_whose_own_override_is_written_on_one_line( + self, resolved: ResolvedDependencies, tmp_path: Path, flat_graph: DependencyGraph + ): + # Bazel rejects two overrides for one module, so a single-line override must be stripped + # like a multi-line one -- otherwise ref_int's is the second and nothing resolves. + mod = tmp_path / "MODULE.bazel" + mod.write_text( + MODULE_BAZEL + '\ngit_override(module_name = "score_logging", commit = "deadbeef", remote = "u")\n' + ) + patched = resolved.overwrite(mod, flat_graph, module_under_test="score_persistency", write=False) + assert "deadbeef" not in patched + assert patched.count('module_name = "score_logging"') == 1 + + def test_strip_leaves_an_override_for_a_dep_it_does_not_inject( + self, resolved: ResolvedDependencies, tmp_path: Path, flat_graph: DependencyGraph + ): + # A dep with no entry in the resolved set is never injected, so the module's own override + # for it must survive -- a too-greedy strip would take neighbours with it. + mod = tmp_path / "MODULE.bazel" + mod.write_text(MODULE_BAZEL + '\nsingle_version_override(module_name = "score_unpinned", version = "9.9.9")\n') + patched = resolved.overwrite(mod, flat_graph, module_under_test="score_persistency", write=False) + assert 'module_name = "score_unpinned"' in patched + class TestOverwriteTransitive: """Closure injection: pin transitive deps the module never declares itself.""" @@ -263,9 +304,7 @@ def only_baselibs(self, tmp_path: Path) -> Path: return p def test_injects_transitive_dep_with_its_bazel_dep(self, resolved: ResolvedDependencies, only_baselibs: Path): - patched = resolved.overwrite( - only_baselibs, module_under_test="score_persistency", write=False, graph=self._graph() - ) + patched = resolved.overwrite(only_baselibs, self._graph(), module_under_test="score_persistency", write=False) block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] assert 'module_name = "score_baselibs"' in block assert 'module_name = "score_logging"' in block @@ -276,9 +315,7 @@ def test_injects_transitive_dep_with_its_bazel_dep(self, resolved: ResolvedDepen def test_declared_dep_gets_no_extra_bazel_dep(self, resolved: ResolvedDependencies, only_baselibs: Path): # score_baselibs is already declared above the block; re-declaring it would be a # duplicate declaration of the same module. - patched = resolved.overwrite( - only_baselibs, module_under_test="score_persistency", write=False, graph=self._graph() - ) + patched = resolved.overwrite(only_baselibs, self._graph(), module_under_test="score_persistency", write=False) block = patched.split(INJECTION_BEGIN)[1].split(INJECTION_END)[0] assert "bazel_dep" not in block.split('module_name = "score_baselibs"')[0] assert patched.count('bazel_dep(name = "score_baselibs")') == 1 @@ -293,29 +330,29 @@ def test_registry_dep_stub_repeats_the_resolved_version(self, resolved: Resolved _node("", "", [_node("score_persistency", deps=[_node("score_tooling"), _node("score_logging")])]) ) block = ( - resolved.overwrite(mod, module_under_test="score_persistency", write=False, graph=graph) + resolved.overwrite(mod, graph, module_under_test="score_persistency", write=False) .split(INJECTION_BEGIN)[1] .split(INJECTION_END)[0] ) assert 'bazel_dep(name = "score_tooling", version = "1.2.0")' in block assert 'bazel_dep(name = "score_logging")\n' in block - def test_without_graph_only_declared_deps_are_pinned(self, resolved: ResolvedDependencies, only_baselibs: Path): - patched = resolved.overwrite(only_baselibs, module_under_test="score_persistency", write=False) - assert 'module_name = "score_logging"' not in patched + def test_graph_is_required(self, resolved: ResolvedDependencies, only_baselibs: Path): + # Without a graph the closure cannot be computed, and pinning a module without the modules + # it needs is what produced "module lobster@0.0.0 not found in registries". Unrepresentable + # rather than merely discouraged. + with pytest.raises(TypeError): + resolved.overwrite(only_baselibs, module_under_test="score_persistency", write=False) def test_closure_member_absent_from_resolved_set_is_skipped( self, resolved: ResolvedDependencies, only_baselibs: Path, caplog: pytest.LogCaptureFixture ): - # A module's dev_dependency deps activate only when it is root — true in Stage 2 but - # not in Stage 1 — so ref_int's graph never saw them. Warn, never fail. + # ref_int resolved nothing for it, so there is nothing to impose. Warn, never fail. graph = DependencyGraph( _node("", "", [_node("score_persistency", deps=[_node("rules_doxygen")])]), ) with caplog.at_level(logging.WARNING): - patched = resolved.overwrite( - only_baselibs, module_under_test="score_persistency", write=False, graph=graph - ) + patched = resolved.overwrite(only_baselibs, graph, module_under_test="score_persistency", write=False) assert "rules_doxygen" not in patched assert "rules_doxygen" in caplog.text @@ -323,11 +360,109 @@ def test_module_under_test_inferred_from_module_declaration( self, resolved: ResolvedDependencies, only_baselibs: Path ): # module(name = "...") identifies the root, so --module-under-test is optional. - patched = resolved.overwrite(only_baselibs, write=False, graph=self._graph()) + patched = resolved.overwrite(only_baselibs, self._graph(), write=False) assert 'module_name = "score_persistency"' not in patched assert 'module_name = "score_baselibs"' in patched +class TestScopeIsDecidedByTheResolvedSet: + """Presence in the resolved set decides the pin scope. ``dev_dependency`` plays no part. + + The two properties are independent, measured on the real modules: ``score_baselibs`` declares + 11 dev-only deps ref_int *has* resolved, while ``score_baselibs_rust`` is a *public* dep of + ``score_communication`` that ref_int has *not*. So the flag predicts neither case and cannot + be the discriminator. + """ + + @pytest.fixture + def dev_and_public(self, tmp_path: Path) -> Path: + p = tmp_path / "MODULE.bazel" + p.write_text( + 'module(name = "score_persistency", version = "0.0.0")\n' + 'bazel_dep(name = "score_baselibs", version = "0.2.7")\n' + 'bazel_dep(name = "score_tooling", version = "1.0.0", dev_dependency = True)\n' + ) + return p + + @staticmethod + def _graph_with_tooling_closure() -> DependencyGraph: + # ref_int declares score_tooling publicly, so its closure is in the Stage-1 graph even + # though the module-under-test's edge to it is dev-only and therefore absent. + return DependencyGraph( + _node( + "", + "", + [ + _node("score_persistency", deps=[_node("score_baselibs")]), + _node("score_tooling", deps=[_node("trlc"), _node("lobster")]), + ], + ) + ) + + def test_dev_declared_dep_is_pinned_because_ref_int_resolved_it( + self, resolved: ResolvedDependencies, dev_and_public: Path + ): + # ref_int has a version for score_tooling, so it imposes it. How the module declares the + # edge is irrelevant. Excluding these left 11 of score_baselibs' deps unvalidated. + patched = resolved.overwrite( + dev_and_public, self._graph_with_tooling_closure(), module_under_test="score_persistency", write=False + ) + assert 'module_name = "score_tooling"' in patched + + def test_its_closure_is_pinned_with_it(self, resolved: ResolvedDependencies, dev_and_public: Path): + # What makes the rule safe rather than merely permissive: score_tooling must never arrive + # without the modules it needs, or the graph is unresolvable. + with_closure = ResolvedDependencies( + { + **resolved.modules, + "trlc": Module(name="trlc", hash="", repo="", version="2.0.0"), + "lobster": Module(name="lobster", hash="", repo="", version="0.9.0"), + } + ) + patched = with_closure.overwrite( + dev_and_public, self._graph_with_tooling_closure(), module_under_test="score_persistency", write=False + ) + assert 'module_name = "trlc"' in patched + assert 'module_name = "lobster"' in patched + + def test_public_dep_ref_int_did_not_resolve_is_left_alone( + self, resolved: ResolvedDependencies, tmp_path: Path, caplog: pytest.LogCaptureFixture + ): + # The case that disproves the flag as a discriminator: score_unpinned is declared with no + # dev flag at all and still has no resolved entry, exactly like score_baselibs_rust in + # score_communication. It must be reported and left untouched, not forced to anything. + p = tmp_path / "MODULE.bazel" + p.write_text( + 'module(name = "score_persistency", version = "0.0.0")\n' + 'bazel_dep(name = "score_unpinned", version = "9.9.9")\n' + ) + graph = DependencyGraph(_node("", "", [_node("score_persistency"), _node("score_unpinned")])) + with caplog.at_level(logging.WARNING): + patched = resolved.overwrite(p, graph, module_under_test="score_persistency", write=False) + assert 'module_name = "score_unpinned"' not in patched + assert "score_unpinned" in caplog.text + + def test_dev_declared_dep_ref_int_did_not_resolve_is_left_alone( + self, resolved: ResolvedDependencies, tmp_path: Path + ): + # Same outcome as the public case above, reached by the same rule rather than by the flag. + p = tmp_path / "MODULE.bazel" + p.write_text( + 'module(name = "score_persistency", version = "0.0.0")\n' + 'bazel_dep(name = "score_unpinned", version = "9.9.9", dev_dependency = True)\n' + ) + graph = DependencyGraph(_node("", "", [_node("score_persistency"), _node("score_unpinned")])) + patched = resolved.overwrite(p, graph, module_under_test="score_persistency", write=False) + assert 'module_name = "score_unpinned"' not in patched + + def test_public_dep_is_still_pinned(self, resolved: ResolvedDependencies, dev_and_public: Path): + # Widening scope must not disturb the public surface, or Stage 2 goes vacuously green. + patched = resolved.overwrite( + dev_and_public, self._graph_with_tooling_closure(), module_under_test="score_persistency", write=False + ) + assert 'module_name = "score_baselibs"' in patched + + class TestFromModGraph: @staticmethod def _graph() -> dict: @@ -401,32 +536,18 @@ def test_to_file_is_lean_and_roundtrips(self, tmp_path: Path, resolved: Resolved class TestFromResolvedArtifact: - def test_prefers_manifest(self, tmp_path: Path, resolved: ResolvedDependencies): + def test_reads_the_manifest(self, tmp_path: Path, resolved: ResolvedDependencies): art = tmp_path / "art" art.mkdir() resolved.to_file(art / "resolved_versions.json") - # With the manifest present, no lock / score_modules files are required. parsed = ResolvedDependencies.from_resolved_artifact(art) assert parsed.get("score_baselibs").hash == resolved.get("score_baselibs").hash assert parsed.get("score_tooling").version == "1.2.0" - def test_requires_manifest_or_lockfile(self, tmp_path: Path): + def test_missing_manifest_is_fatal(self, tmp_path: Path): + # A silently empty resolved set would make Stage 2 pass while validating nothing. The lock + # and the generated score_modules_*.MODULE.bazel files are not substitutes. + (tmp_path / "MODULE.bazel.lock").write_text("{}") (tmp_path / "score_modules_target_sw.MODULE.bazel").write_text("bazel_dep(name='x')\n") - with pytest.raises(FileNotFoundError): + with pytest.raises(FileNotFoundError, match=MANIFEST_NAME): ResolvedDependencies.from_resolved_artifact(tmp_path) - - def test_roundtrip_known_good_to_artifact(self, tmp_path: Path, resolved: ResolvedDependencies): - # Build an artifact dir mirroring stage1-resolved-deps (legacy format), then parse it back. - art = tmp_path / "art" - art.mkdir() - (art / "MODULE.bazel.lock").write_text("{}") - blocks = [] - for m in resolved.modules.values(): - directive = generate_override_directive(m) - if directive: - blocks.append(f'bazel_dep(name = "{m.name}")\n' + directive) - (art / "score_modules_target_sw.MODULE.bazel").write_text("\n".join(blocks)) - - parsed = ResolvedDependencies.from_resolved_artifact(art) - assert parsed.get("score_baselibs").hash == resolved.get("score_baselibs").hash - assert parsed.get("score_tooling").version == "1.2.0" diff --git a/scripts/tooling/BUILD b/scripts/tooling/BUILD index d989b765db4..c2088414897 100644 --- a/scripts/tooling/BUILD +++ b/scripts/tooling/BUILD @@ -81,13 +81,6 @@ py_binary( visibility = ["//visibility:public"], ) -# Alias: expose resolve_deps under //scripts/tooling for `bazel run //scripts/tooling:resolve_deps`. -alias( - name = "resolve_deps", - actual = "//scripts/known_good:resolve_deps", - visibility = ["//visibility:public"], -) - # Tests target score_py_pytest( name = "tooling_tests", From fb20ff4f7564e07c2be3d76ddbe64393b32d2efd Mon Sep 17 00:00:00 2001 From: subramaniak Date: Tue, 11 Aug 2026 10:01:13 +0000 Subject: [PATCH 11/14] feat: lock the versions of deps Stage 2 collects test artifacts from --- MODULE.bazel | 5 ++ MODULE.bazel.lock | 21 ------ .../score_test_artifact_versions.MODULE.bazel | 71 +++++++++++++++++++ 3 files changed, 76 insertions(+), 21 deletions(-) create mode 100644 bazel_common/score_test_artifact_versions.MODULE.bazel diff --git a/MODULE.bazel b/MODULE.bazel index df2e3a8995e..0b56e7058d4 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -41,6 +41,11 @@ include("//bazel_common:score_modules_target_sw.MODULE.bazel") # Score test images include("//bazel_common:score_images.MODULE.bazel") +# Single-version locks for the deps Stage 2 collects test artifacts from (GTest, the Rust +# test rules, the ferrocene coverage tooling). Read after the includes above so it pins the +# versions they bring in transitively. +include("//bazel_common:score_test_artifact_versions.MODULE.bazel") + bazel_dep(name = "rules_boost", repo_name = "com_github_nelhage_rules_boost") archive_override( module_name = "rules_boost", diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 345f5061d98..ee2861f76ce 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -5,7 +5,6 @@ "https://bcr.bazel.build/modules/abseil-cpp/20210324.2/MODULE.bazel": "7cd0312e064fde87c8d1cd79ba06c876bd23630c83466e9500321be55c96ace2", "https://bcr.bazel.build/modules/abseil-cpp/20211102.0/MODULE.bazel": "70390338f7a5106231d20620712f7cccb659cd0e9d073d1991c038eb9fc57589", "https://bcr.bazel.build/modules/abseil-cpp/20220623.1/MODULE.bazel": "73ae41b6818d423a11fd79d95aedef1258f304448193d4db4ff90e5e7a0f076c", - "https://bcr.bazel.build/modules/abseil-cpp/20230125.1/MODULE.bazel": "89047429cb0207707b2dface14ba7f8df85273d484c2572755be4bab7ce9c3a0", "https://bcr.bazel.build/modules/abseil-cpp/20230802.0.bcr.1/MODULE.bazel": "1c8cec495288dccd14fdae6e3f95f772c1c91857047a098fad772034264cc8cb", "https://bcr.bazel.build/modules/abseil-cpp/20230802.0/MODULE.bazel": "d253ae36a8bd9ee3c5955384096ccb6baf16a1b1e93e858370da0a3b94f77c16", "https://bcr.bazel.build/modules/abseil-cpp/20230802.1/MODULE.bazel": "fa92e2eb41a04df73cdabeec37107316f7e5272650f81d6cc096418fe647b915", @@ -263,14 +262,8 @@ "https://bcr.bazel.build/modules/googleapis/0.0.0-20240326-1c8d509c5/MODULE.bazel": "a4b7e46393c1cdcc5a00e6f85524467c48c565256b22b5fae20f84ab4a999a68", "https://bcr.bazel.build/modules/googleapis/0.0.0-20240819-fe8ba054a/MODULE.bazel": "117b7c7be7327ed5d6c482274533f2dbd78631313f607094d4625c28203cacdf", "https://bcr.bazel.build/modules/googleapis/0.0.0-20240819-fe8ba054a/source.json": "b31fc7eb283a83f71d2e5bfc3d1c562d2994198fa1278409fbe8caec3afc1d3e", - "https://bcr.bazel.build/modules/googletest/1.11.0/MODULE.bazel": "3a83f095183f66345ca86aa13c58b59f9f94a2f81999c093d4eeaa2d262d12f4", - "https://bcr.bazel.build/modules/googletest/1.13.0/MODULE.bazel": "369533f4a302dc7d9ad1cd9a09a9e820a1d9a4011fad2dfa636b5bb225b9a6c7", - "https://bcr.bazel.build/modules/googletest/1.14.0.bcr.1/MODULE.bazel": "22c31a561553727960057361aa33bf20fb2e98584bc4fec007906e27053f80c6", - "https://bcr.bazel.build/modules/googletest/1.14.0/MODULE.bazel": "cfbcbf3e6eac06ef9d85900f64424708cc08687d1b527f0ef65aa7517af8118f", - "https://bcr.bazel.build/modules/googletest/1.15.2/MODULE.bazel": "6de1edc1d26cafb0ea1a6ab3f4d4192d91a312fd2d360b63adaa213cd00b2108", "https://bcr.bazel.build/modules/googletest/1.17.0.bcr.2/MODULE.bazel": "827f54f492a3ce549c940106d73de332c2b30cebd0c20c0bc5d786aba7f116cb", "https://bcr.bazel.build/modules/googletest/1.17.0.bcr.2/source.json": "3664514073a819992320ffbce5825e4238459df344d8b01748af2208f8d2e1eb", - "https://bcr.bazel.build/modules/googletest/1.17.0/MODULE.bazel": "dbec758171594a705933a29fcf69293d2468c49ec1f2ebca65c36f504d72df46", "https://bcr.bazel.build/modules/grpc-java/1.62.2/MODULE.bazel": "99b8771e8c7cacb130170fed2a10c9e8fed26334a93e73b42d2953250885a158", "https://bcr.bazel.build/modules/grpc-java/1.66.0/MODULE.bazel": "86ff26209fac846adb89db11f3714b3dc0090fb2fb81575673cc74880cda4e7e", "https://bcr.bazel.build/modules/grpc-java/1.69.0/MODULE.bazel": "53887af6a00b3b406d70175d3d07e84ea9362016ff55ea90b9185f0227bfaf98", @@ -527,9 +520,6 @@ "https://bcr.bazel.build/modules/rules_python_gazelle_plugin/1.5.1/source.json": "c52e4d2229fbd92b658bf60a7638e79b96525e8f7ed6c59036b4827cade9e430", "https://bcr.bazel.build/modules/rules_robolectric/4.14.1.2/MODULE.bazel": "d44fec647d0aeb67b9f3b980cf68ba634976f3ae7ccd6c07d790b59b87a4f251", "https://bcr.bazel.build/modules/rules_robolectric/4.14.1.2/source.json": "37c10335f2361c337c5c1f34ed36d2da70534c23088062b33a8bdaab68aa9dea", - "https://bcr.bazel.build/modules/rules_rust/0.56.0/MODULE.bazel": "3295b00757db397122092322fe1e920be7f5c9fbfb8619138977e820f2cbbbae", - "https://bcr.bazel.build/modules/rules_rust/0.61.0/MODULE.bazel": "0318a95777b9114c8740f34b60d6d68f9cfef61e2f4b52424ca626213d33787b", - "https://bcr.bazel.build/modules/rules_rust/0.67.0/MODULE.bazel": "87c3816c4321352dcfd9e9e26b58e84efc5b21351ae3ef8fb5d0d57bde7237f5", "https://bcr.bazel.build/modules/rules_shell/0.1.2/MODULE.bazel": "66e4ca3ce084b04af0b9ff05ff14cab4e5df7503973818bb91cbc6cda08d32fc", "https://bcr.bazel.build/modules/rules_shell/0.2.0/MODULE.bazel": "fda8a652ab3c7d8fee214de05e7a9916d8b28082234e8d2c0094505c5268ed3c", "https://bcr.bazel.build/modules/rules_shell/0.3.0/MODULE.bazel": "de4402cd12f4cc8fda2354fce179fdb068c0b9ca1ec2d2b17b3e21b24c1a937b", @@ -589,7 +579,6 @@ "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/abseil-cpp/20210324.2/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/abseil-cpp/20211102.0/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/abseil-cpp/20220623.1/MODULE.bazel": "not found", - "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/abseil-cpp/20230125.1/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/abseil-cpp/20230802.0.bcr.1/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/abseil-cpp/20230802.0/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/abseil-cpp/20230802.1/MODULE.bazel": "not found", @@ -770,13 +759,7 @@ "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/google_benchmark/1.9.5/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/googleapis/0.0.0-20240326-1c8d509c5/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/googleapis/0.0.0-20240819-fe8ba054a/MODULE.bazel": "not found", - "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/googletest/1.11.0/MODULE.bazel": "not found", - "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/googletest/1.13.0/MODULE.bazel": "not found", - "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/googletest/1.14.0.bcr.1/MODULE.bazel": "not found", - "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/googletest/1.14.0/MODULE.bazel": "not found", - "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/googletest/1.15.2/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/googletest/1.17.0.bcr.2/MODULE.bazel": "not found", - "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/googletest/1.17.0/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/grpc-java/1.62.2/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/grpc-java/1.66.0/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/grpc-java/1.69.0/MODULE.bazel": "not found", @@ -992,10 +975,6 @@ "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/rules_python/1.8.5/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/rules_python_gazelle_plugin/1.5.1/MODULE.bazel": "not found", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/rules_robolectric/4.14.1.2/MODULE.bazel": "not found", - "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/rules_rust/0.56.0/MODULE.bazel": "not found", - "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/rules_rust/0.61.0/MODULE.bazel": "not found", - "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/rules_rust/0.67.0/MODULE.bazel": "not found", - "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/rules_rust/0.68.1-score/MODULE.bazel": "dc1c87d74ef6d32190e65c3c8aabfa7e7764e457bf9888312e0313c3c11fdb69", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/rules_rust/0.68.2-score/MODULE.bazel": "37be8dee6df19d666c1d4266e1266d82012aa83bd82de38b3100fd7f641d064b", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/rules_rust/0.68.2-score/source.json": "f88ad98dd08f296a546677e86ad42b20f61851e41a9fd3e0449971162fcaf784", "https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/rules_shell/0.1.2/MODULE.bazel": "not found", diff --git a/bazel_common/score_test_artifact_versions.MODULE.bazel b/bazel_common/score_test_artifact_versions.MODULE.bazel new file mode 100644 index 00000000000..1e9758a451b --- /dev/null +++ b/bazel_common/score_test_artifact_versions.MODULE.bazel @@ -0,0 +1,71 @@ +# ******************************************************************************* +# 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 +# ******************************************************************************* +# +# Single-version locks for the dependencies Stage 2 collects test artifacts from. +# +# The rule: if a dependency's output ends up inside a collected Stage-2 artifact, it gets +# ONE version, defined here, used by every downstream module execution. A dependency we +# collect nothing from is deliberately left alone, and each module keeps whatever it +# declares -- hedron_compile_commands is the reference case: never in ref_int's resolved +# set, never injected, and that is correct. +# +# Why an override rather than just a bazel_dep version. `bazel_dep(version = ...)` is only +# a FLOOR: MVS raises it whenever anything else in the graph asks for more, silently and +# without review. Five deps already drift that way today -- rules_cc (0.2.16 declared vs +# 0.2.17 resolved), rules_python (1.8.3/1.8.5), rules_shell (0.6.0/0.6.1), +# aspect_rules_lint (2.3.0/2.5.0), and rules_rust below. `single_version_override` is the +# ceiling as well as the floor, so the version Stage 1 writes into resolved_versions.json +# -- and therefore the version Stage 2 injects into all 8 modules -- is one ref_int +# asserted, not one MVS happened to land on. +# +# Every version below equals what MVS already resolves today, so this file changes no +# build now. It changes what happens the day a module bumps its own copy. + +# Every C++ test binary Stage 2 runs is a gtest binary, and every _coverage_report.dat +# that genhtml turns into a report comes out of one. Before this lock ref_int declared no +# googletest version anywhere: 1.17.0.bcr.2 was purely what MVS selected transitively. +single_version_override( + module_name = "googletest", + version = "1.17.0.bcr.2", +) + +# The Rust test rules that build the binaries emitting .profraw. This also closes an +# existing drift: score_rust_toolchains.MODULE.bazel declares 0.68.1-score while MVS +# resolves 0.68.2-score. That bazel_dep stays as the floor; this is the assertion. +single_version_override( + module_name = "rules_rust", + version = "0.68.2-score", +) + +# The ferrocene coverage tooling itself. `ferrocene.toolchain(coverage_tools_url = ...)` +# in score_rust_toolchains.MODULE.bazel is what produces the .profraw files that +# @score_tooling//coverage:ferrocene_report then reads. It is declared +# `dev_dependency = True`, which is correct and unrelated: dev_dependency scopes when an +# edge activates, not who owns the version. +single_version_override( + module_name = "score_toolchains_rust", + version = "0.8.0", +) + +# Considered and deliberately NOT locked here: +# +# * toolchains_llvm -- supplies libclang.so for the score_tooling C++ parser behind +# score_communication's code generation; the LLVM C++ toolchain is not registered at all +# (see score_llvm_libclang.MODULE.bazel). Nothing collects an artifact from it. It can +# reach a collected artifact only indirectly, by changing generated sources, so locking +# it would be a defensible *extension* of this rule rather than an instance of it. Open +# for the reviewer to include if that indirect path is judged in scope. +# * google_benchmark, aspect_rules_lint, score_qnx_unit_tests -- no collection path in +# scripts/quality_runners.py. +# * score_tooling, score_test_scenarios -- already exact, both pinned by git_override +# commit; a version lock would add nothing. From 66b4dc1c67674a717a1d47281c6d0239331b171a Mon Sep 17 00:00:00 2001 From: subramaniak Date: Wed, 12 Aug 2026 10:24:17 +0000 Subject: [PATCH 12/14] fix: pin rules_oci by commit and report every pin's provenance, conflicts, and drops --- .github/workflows/internal_tests.yml | 2 +- bazel_common/score_images.MODULE.bazel | 4 +- .../score_test_artifact_versions.MODULE.bazel | 52 +- scripts/known_good/BUILD | 12 +- scripts/known_good/resolved_dependencies.py | 445 ++++++++++++++++-- .../tests/test_resolved_dependencies.py | 438 +++++++++++++++++ 6 files changed, 858 insertions(+), 95 deletions(-) diff --git a/.github/workflows/internal_tests.yml b/.github/workflows/internal_tests.yml index c1d0f518313..ec62f248aa9 100644 --- a/.github/workflows/internal_tests.yml +++ b/.github/workflows/internal_tests.yml @@ -21,4 +21,4 @@ jobs: internal_tests: uses: eclipse-score/cicd-workflows/.github/workflows/tests.yml@main with: - bazel-target: "test //scripts/tooling:tooling_tests" + bazel-target: "test //scripts/tooling:tooling_tests //scripts/known_good:known_good_tests" diff --git a/bazel_common/score_images.MODULE.bazel b/bazel_common/score_images.MODULE.bazel index a68ab7227af..38cd721e690 100644 --- a/bazel_common/score_images.MODULE.bazel +++ b/bazel_common/score_images.MODULE.bazel @@ -10,11 +10,13 @@ # # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* +# Spelled as a commit, not `tag = "v2.3.1"`: the Stage-1 manifest carries immutable commits only, so +# a tag-pinned git_override was dropped from it silently and every module resolved its own rules_oci. bazel_dep(name = "rules_oci", version = "2.3.1") git_override( module_name = "rules_oci", + commit = "f214185dcf149090cb3212e878f692eb2c8c0d3d", # v2.3.1 remote = "https://github.com/bazel-contrib/rules_oci.git", - tag = "v2.3.1", ) oci = use_extension("@rules_oci//oci:extensions.bzl", "oci") diff --git a/bazel_common/score_test_artifact_versions.MODULE.bazel b/bazel_common/score_test_artifact_versions.MODULE.bazel index 1e9758a451b..e860cb7c186 100644 --- a/bazel_common/score_test_artifact_versions.MODULE.bazel +++ b/bazel_common/score_test_artifact_versions.MODULE.bazel @@ -11,61 +11,31 @@ # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* # -# Single-version locks for the dependencies Stage 2 collects test artifacts from. +# Deliberate ceilings for the deps Stage 2 collects test artifacts from. # -# The rule: if a dependency's output ends up inside a collected Stage-2 artifact, it gets -# ONE version, defined here, used by every downstream module execution. A dependency we -# collect nothing from is deliberately left alone, and each module keeps whatever it -# declares -- hedron_compile_commands is the reference case: never in ref_int's resolved -# set, never injected, and that is correct. +# ref_int's resolved set is imposed on every module under test whether a dependency is named here or +# not; this file does not decide *which* deps get pinned. It decides which of those pins are a +# decision rather than an inheritance. `bazel_dep(version = ...)` is only a floor that MVS raises +# silently, while `single_version_override` is also a ceiling, so the versions below are ones Stage 1 +# reports as `asserted` rather than `incidental`. # -# Why an override rather than just a bazel_dep version. `bazel_dep(version = ...)` is only -# a FLOOR: MVS raises it whenever anything else in the graph asks for more, silently and -# without review. Five deps already drift that way today -- rules_cc (0.2.16 declared vs -# 0.2.17 resolved), rules_python (1.8.3/1.8.5), rules_shell (0.6.0/0.6.1), -# aspect_rules_lint (2.3.0/2.5.0), and rules_rust below. `single_version_override` is the -# ceiling as well as the floor, so the version Stage 1 writes into resolved_versions.json -# -- and therefore the version Stage 2 injects into all 8 modules -- is one ref_int -# asserted, not one MVS happened to land on. -# -# Every version below equals what MVS already resolves today, so this file changes no -# build now. It changes what happens the day a module bumps its own copy. +# Every version here equals what MVS resolves today, so this changes no build now -- only what +# happens the day something in the graph asks for more. -# Every C++ test binary Stage 2 runs is a gtest binary, and every _coverage_report.dat -# that genhtml turns into a report comes out of one. Before this lock ref_int declared no -# googletest version anywhere: 1.17.0.bcr.2 was purely what MVS selected transitively. +# C++ test binaries and the coverage .dat files genhtml reads. single_version_override( module_name = "googletest", version = "1.17.0.bcr.2", ) -# The Rust test rules that build the binaries emitting .profraw. This also closes an -# existing drift: score_rust_toolchains.MODULE.bazel declares 0.68.1-score while MVS -# resolves 0.68.2-score. That bazel_dep stays as the floor; this is the assertion. +# Rust test rules that build the .profraw-emitting binaries. single_version_override( module_name = "rules_rust", version = "0.68.2-score", ) -# The ferrocene coverage tooling itself. `ferrocene.toolchain(coverage_tools_url = ...)` -# in score_rust_toolchains.MODULE.bazel is what produces the .profraw files that -# @score_tooling//coverage:ferrocene_report then reads. It is declared -# `dev_dependency = True`, which is correct and unrelated: dev_dependency scopes when an -# edge activates, not who owns the version. +# Ferrocene coverage tooling behind those .profraw files. single_version_override( module_name = "score_toolchains_rust", version = "0.8.0", ) - -# Considered and deliberately NOT locked here: -# -# * toolchains_llvm -- supplies libclang.so for the score_tooling C++ parser behind -# score_communication's code generation; the LLVM C++ toolchain is not registered at all -# (see score_llvm_libclang.MODULE.bazel). Nothing collects an artifact from it. It can -# reach a collected artifact only indirectly, by changing generated sources, so locking -# it would be a defensible *extension* of this rule rather than an instance of it. Open -# for the reviewer to include if that indirect path is judged in scope. -# * google_benchmark, aspect_rules_lint, score_qnx_unit_tests -- no collection path in -# scripts/quality_runners.py. -# * score_tooling, score_test_scenarios -- already exact, both pinned by git_override -# commit; a version lock would add nothing. diff --git a/scripts/known_good/BUILD b/scripts/known_good/BUILD index 11f18e00a6d..d782d0d2216 100644 --- a/scripts/known_good/BUILD +++ b/scripts/known_good/BUILD @@ -37,12 +37,16 @@ score_py_pytest( # Runnable binary for the resolve + inject workflow. # # Stage 1 (export) — 'bazel mod graph' is a prerequisite; run it first and pass the result: -# bazel mod graph --output=json > graph.json +# bazel mod graph --verbose --output=json > graph.json # bazel run //scripts/known_good:resolve_deps -- \ # --mod-graph graph.json --export _resolved_deps/resolved_versions.json -# Writes the manifest and stores graph.json next to it; both are published as the -# stage1-resolved-deps artifact. Paths are resolved against BUILD_WORKSPACE_DIRECTORY, -# so graph.json does not need to be listed in data = [...]. +# Writes the manifest, graph.json and resolved_pins_report.json side by side; all three are +# published as the stage1-resolved-deps artifact. Paths are resolved against +# BUILD_WORKSPACE_DIRECTORY, so graph.json does not need to be listed in data = [...]. +# +# '--verbose' adds 'originalVersion' to each edge, the only record of a consumer asking for a +# version other than the one ref_int imposes. It is a strict superset, so Stage 2 reads the same +# graph.json either way; without it the export still succeeds, with every verdict 'unknown'. # # Stage 2 (inject) — consumes that same directory: # bazel run //scripts/known_good:resolve_deps -- \ diff --git a/scripts/known_good/resolved_dependencies.py b/scripts/known_good/resolved_dependencies.py index f7609350d99..cfbebf1151a 100644 --- a/scripts/known_good/resolved_dependencies.py +++ b/scripts/known_good/resolved_dependencies.py @@ -204,6 +204,13 @@ def closure(self, module_name: str) -> set[str]: # third-party resolved versions, merged. The lock travels alongside only as evidence. MANIFEST_NAME = "resolved_versions.json" +# Sidecar beside the manifest: what a human needs to judge the pins, kept out of the manifest so +# that stays a lean Stage-2 input. +REPORT_NAME = "resolved_pins_report.json" + +# Stated in the report so a reader does not have to infer why the export is this broad. +POLICY = "ref_int's pins are authoritative and are forced onto every module under test." + # Built-in / non-registry modules that must not be given a single_version_override. _SKIP_MODULES = {"bazel_tools"} @@ -219,12 +226,119 @@ def closure(self, module_name: str) -> set[str]: _SINGLE_VERSION_BLOCK_RE = re.compile(r"single_version_override\((?P.*?)\)", re.S) _FIELD_RE = lambda field: re.compile(rf'{field}\s*=\s*"([^"]+)"') # noqa: E731 +# Every override directive Bazel accepts from a root module. +_OVERRIDE_KINDS = ( + "git_override", + "single_version_override", + "archive_override", + "local_path_override", + "multiple_version_override", +) +# Any override block. ``[^)]*`` suffices: override arguments are scalars and lists, never a call. +_ANY_OVERRIDE_BLOCK_RE = re.compile(r"(?P" + "|".join(_OVERRIDE_KINDS) + r")\((?P[^)]*)\)", re.S) + +_UNCARRIED_CONSEQUENCE = "ref_int imposes nothing for this module; each module under test resolves its own version" + + +def _declared_overrides(text: str, source: str) -> dict[str, dict[str, str]]: + """Every module ref_int declares an override for, with its kind and why it might not carry. + + The completeness half of the manifest guard: ``_parse_override_file`` says what can become a + Module, this says what ref_int actually wrote, and the difference is a pin ref_int believes it + imposes but does not. That gap hid ``rules_oci`` -- a ``git_override`` with ``tag`` instead of + ``commit``, rejected by the parser and missed by a warning that only matched archive/local_path, + so it produced no output at all. Enumerated from :data:`_OVERRIDE_KINDS` rather than a list of + known-bad cases, so an unforeseen kind cannot vanish quietly either. + """ + declared: dict[str, dict[str, str]] = {} + for match in _ANY_OVERRIDE_BLOCK_RE.finditer(text): + kind, body = match.group("kind"), match.group("body") + name = _field(body, "module_name") + if not name: + continue + declared[name] = { + "module": name, + "kind": kind, + "file": source, + "reason": _uncarried_reason(kind, body), + "consequence": _UNCARRIED_CONSEQUENCE, + } + return declared + + +def _uncarried_reason(kind: str, body: str) -> str: + """Why an override ref_int declares cannot become a manifest entry.""" + if kind == "git_override": + ref = _field(body, "tag") or _field(body, "branch") + if ref: + return ( + f"git_override pins the mutable ref {ref!r} rather than a commit; the manifest " + f"carries immutable commits only" + ) + if not _field(body, "commit"): + return "git_override declares no commit" + return "git_override declares no remote" + if kind == "single_version_override": + return "single_version_override declares no version" + if kind == "multiple_version_override": + return "multiple_version_override cannot be expressed as a single pin" + return f"{kind} cannot be expressed as a manifest directive" + + +# ``dev_dependency = True`` is a bare token, not a quoted value, so _FIELD_RE cannot match it. +_DEV_DEPENDENCY_RE = re.compile(r"dev_dependency\s*=\s*True") + + +def _declared_dep_specs(text: str) -> dict[str, dict[str, object]]: + """Every ``bazel_dep`` in ``text``, with the version it asks for and whether it is dev-scoped. + + Kept separate from :func:`_declared_deps` so ``overwrite()`` keeps its narrower contract. + ``dev_dependency`` is recorded for Stage 2, which needs it to attribute a conflict on a dev + edge; Stage 1 does not filter on it. + """ + specs: dict[str, dict[str, object]] = {} + for call in _BAZEL_DEP_CALL_RE.finditer(text): + body = call.group("body") + name = _FIELD_RE("name").search(body) + if name is None: + continue + version = _FIELD_RE("version").search(body) + specs[name.group(1)] = { + "version": version.group(1) if version else None, + "dev_dependency": bool(_DEV_DEPENDENCY_RE.search(body)), + } + return specs + + +def _internal_drift(resolved: dict[str, Module], declared: dict[str, dict[str, object]]) -> list[dict[str, object]]: + """Where ref_int's own declared ``bazel_dep`` version is not the one it ends up imposing. + + ref_int disagreeing with itself, invisible in review because ``bazel_dep(version = ...)`` reads + like a decision while MVS treats it as a floor and raises it silently. + """ + drift: list[dict[str, object]] = [] + for name, spec in sorted(declared.items()): + wanted, module = spec.get("version"), resolved.get(name) + if not wanted or module is None or not module.version or module.version == wanted: + continue + drift.append( + { + "module": name, + "declared": wanted, + "resolved": module.version, + "file": spec.get("file"), + "dev_dependency": spec.get("dev_dependency", False), + } + ) + return drift + def _declared_deps(text: str) -> set[str]: """Every dependency a module declares via ``bazel_dep``, dev-declared ones included. - The ``dev_dependency`` flag is deliberately not reported: it does not decide whether ref_int - pins a dependency -- presence in the resolved set does. See :meth:`ResolvedDependencies.overwrite`. + The ``dev_dependency`` flag is deliberately not reported -- policy, not omission: ref_int's pins + are authoritative, so presence in its resolved set decides a pin, not how the module scopes its + own declaration. See :func:`_declared_dep_specs` where the version and dev flag are needed. """ declared: set[str] = set() for call in _BAZEL_DEP_CALL_RE.finditer(text): @@ -261,8 +375,11 @@ class ResolvedDependencies: :meth:`overwrite` interface that pins a module's ``MODULE.bazel`` to those versions. """ - def __init__(self, resolved: dict[str, Module]): + def __init__(self, resolved: dict[str, Module], report: dict | None = None): self._resolved = resolved + # Only from_mod_graph sees both what ref_int declared and what Bazel resolved, so only it can + # populate the report. A manifest read back has the pins but not the evidence. + self._report = report if report is not None else _empty_report() # -- construction: "resolved deps versions from ref_int root" -------------------- @@ -302,26 +419,27 @@ def from_resolved_artifact(cls, artifact_dir: Path) -> ResolvedDependencies: @classmethod def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> ResolvedDependencies: - """Build the *complete* resolved set by merging two sources. - - * The override directives ref_int actually declares — parsed from its root - ``MODULE.bazel`` and the ``bazel_common/*.MODULE.bazel`` files it ``include()``s. - This carries every module ref_int pins by a non-registry source as its real - directive: ``git_override(commit, remote)`` for ``score_*`` plus third-party like - ``trlc`` / ``flatbuffers`` / ``rules_oci``, and ``single_version_override`` where - ref_int pins a registry version. The graph cannot supply these — it reports - overridden modules as version ``0.0.0``. + """Build the resolved set by merging two sources. + + A module keeps its own version only where ref_int has no representable pin, and each such + gap lands in the report's ``uncarried`` list rather than staying implicit. + + * The override directives ref_int declares — parsed from its root ``MODULE.bazel`` and the + ``bazel_common/*.MODULE.bazel`` files it ``include()``s. The graph cannot supply these: it + reports an overridden module at an empty version, or at whatever the overridden source + declares for itself, never at the version ref_int meant to impose. * ``bazel mod graph --output=json`` — the post-MVS resolved version of every other - (registry) module (protobuf, abseil, rules_rust, ...), emitted as - ``single_version_override`` so each module under test is forced to the exact - version ref_int resolved (MVS is graph-global, so a module's own subgraph could - otherwise select a different version). + (registry) module, emitted as ``single_version_override`` so each module under test is + forced to the exact version ref_int resolved (MVS is graph-global, so a module's own + subgraph could otherwise select a different version). - ``archive_override`` / ``local_path_override`` targets (e.g. ``rules_boost``) cannot - be represented and are logged as not carried. + Produce the graph with ``--verbose`` to populate ``originalVersion``, without which every + report verdict is ``unknown``. The export succeeds either way. """ resolved: dict[str, Module] = {} - unrepresentable: list[str] = [] + provenance: dict[str, str] = {} + declared_overrides: dict[str, dict[str, str]] = {} + declared_deps: dict[str, dict[str, object]] = {} for f in override_files: # Drop comment-only lines first: hand-written MODULE.bazel files contain # commented-out overrides (e.g. "# git_override(... rules_rpm ...)") that must @@ -329,33 +447,42 @@ def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> Res text = "\n".join(ln for ln in Path(f).read_text().splitlines() if not ln.lstrip().startswith("#")) for module in cls._parse_override_file(text): # git_override + single_version_override resolved[module.name] = module - for m in re.finditer(r'(archive_override|local_path_override)\(\s*module_name\s*=\s*"([^"]+)"', text): - unrepresentable.append(f"{m.group(2)} ({m.group(1)})") + provenance[module.name] = "asserted" + declared_overrides.update(_declared_overrides(text, Path(f).name)) + for name, spec in _declared_dep_specs(text).items(): + declared_deps[name] = {**spec, "file": Path(f).name} graph = json.loads(Path(mod_graph_json).read_text()) versions: dict[str, str] = {} _collect_resolved_versions(graph, versions) - skipped: list[str] = [] + declared_by: dict[str, set[str]] = {} + _collect_declared_versions(graph, declared_by) for name, version in versions.items(): if name in resolved or name in _SKIP_MODULES: continue # already carried by an override directive, or non-overridable - # 0.0.0 means ref_int pins it via an override this parser did not capture (an - # archive_override), which single_version_override cannot reproduce. Empty versions - # are already dropped by _collect_resolved_versions. + # A literal 0.0.0 means the module declares 0.0.0 itself and ref_int overrides it with + # nothing, so there is no version worth imposing. (Overridden modules report an empty + # version, already dropped upstream by _collect_resolved_versions.) if version == "0.0.0": - skipped.append(name) + declared_overrides.setdefault( + name, + { + "module": name, + "kind": "none", + "file": str(mod_graph_json), + "reason": "resolves to 0.0.0 in the graph and ref_int declares no override for it", + "consequence": _UNCARRIED_CONSEQUENCE, + }, + ) continue resolved[name] = Module(name=name, hash="", repo="", version=version) + provenance[name] = "incidental" - if unrepresentable: - logging.warning( - "Overrides not carried into manifest (need manual handling): %s", ", ".join(unrepresentable) - ) - if skipped: - logging.warning( - "Graph modules at version 0.0.0 with no carried override, skipped: %s", ", ".join(sorted(skipped)) - ) - return cls(resolved) + # The completeness guard: every override ref_int declared that did not become a manifest entry. + uncarried = [entry for name, entry in sorted(declared_overrides.items()) if name not in resolved] + report = _build_report(resolved, provenance, uncarried, declared_by, _internal_drift(resolved, declared_deps)) + _warn_report(report) + return cls(resolved, report) def to_file(self, path: Path) -> None: """Serialize the resolved set to the JSON manifest (Stage 1 -> Stage 2 handoff). @@ -374,6 +501,16 @@ def to_file(self, path: Path) -> None: modules[name] = entry Path(path).write_text(json.dumps({"modules": dict(sorted(modules.items()))}, indent=2) + "\n") + @property + def report(self) -> dict: + """Pin provenance, consumer conflicts and uncarried overrides. Empty unless built by + :meth:`from_mod_graph`.""" + return self._report + + def write_report(self, path: Path) -> None: + """Write the sidecar beside the manifest (see :data:`REPORT_NAME`).""" + Path(path).write_text(json.dumps(self._report, indent=2, sort_keys=False) + "\n") + @classmethod def from_file(cls, path: Path) -> ResolvedDependencies: """Load a resolved set previously written by :meth:`to_file`.""" @@ -426,7 +563,9 @@ def overwrite( ) -> str: """Overwrite a module's dependency versions with ref_int's resolved set. - The rule is *presence in the resolved set*, not how the module declares a dependency: + ref_int's version wins wherever ref_int has one -- never raised to the module's own, never + fallen back to it. The rule is *presence in the resolved set*, not how the module declares + a dependency: * ref_int resolved a version or commit for it -> pin it, whether the module declares it ``dev_dependency`` or not. ref_int has an answer, so it imposes it. @@ -526,15 +665,6 @@ def _strip_injection(text: str) -> str: return pattern.sub("", text).rstrip() + "\n" if pattern.search(text) else text -_OVERRIDE_KINDS = ( - "git_override", - "single_version_override", - "archive_override", - "local_path_override", - "multiple_version_override", -) - - def _strip_existing_overrides(text: str, names: list[str]) -> str: """Remove any ``*_override(module_name = "", ...)`` the module declares itself. @@ -570,9 +700,12 @@ def _field(body: str, field: str) -> str: def injected_override_names(module_bazel_text: str) -> set[str]: """Module names ref_int injected an override for, read back from a patched MODULE.bazel. - The authoritative answer to "did ref_int pin this?" — used by the Stage 2 verification - to tell an override that failed to take effect (ref_int's bug) from a dependency that - was never pinned at all (the module resolved it on its own). + The authoritative answer to "did ref_int pin this?" — it distinguishes an override that failed + to take effect (ref_int's bug) from a dependency that was never pinned at all (the module + resolved it on its own). + + Consumed by ``scripts/known_good/verify_stage2_resolution.py``, which lands with the Stage-2 + workflow; it has no caller inside Stage 1, where the manifest itself is the source of truth. """ if INJECTION_BEGIN not in module_bazel_text: return set() @@ -604,6 +737,204 @@ def _collect_resolved_versions(node: dict, acc: dict[str, str]) -> None: _collect_resolved_versions(dep, acc) +def _collect_declared_versions(node: dict, acc: dict[str, set[str]]) -> None: + """Walk the graph recording name -> the versions consumers asked for. + + ``--verbose`` adds ``originalVersion`` to an edge whenever MVS moved a module off the version its + dependent declared; without it, nothing is recorded. Incomplete by construction, since a non-root + ``dev_dependency`` edge is never loaded: ``score_crates`` and ``score_rules_imagefs`` are visible + but ``score_toolchains_rust``, ``score_itf`` and ``yq.bzl`` are not. See + :data:`_DEV_EDGE_LIMITATION`. + """ + for dep in node.get("dependencies", []): + name, original = dep.get("name"), dep.get("originalVersion") + if name and original: + acc.setdefault(name, set()).add(original) + _collect_declared_versions(dep, acc) + + +def _version_identifiers(version: str) -> tuple[list[str], str]: + """Split a Bazel module version into its release identifiers and its prerelease suffix.""" + release, _, prerelease = version.partition("-") + return release.split("."), prerelease + + +def _compare_identifier(left: str, right: str) -> int | None: + if left == right: + return 0 + if left.isdigit() and right.isdigit(): + return -1 if int(left) < int(right) else 1 + if left.isdigit() != right.isdigit(): + return -1 if left.isdigit() else 1 # Bazel orders numeric identifiers below alphanumeric + return None # two different alphanumerics have no defensible order + + +def _compare_versions(left: str, right: str) -> int | None: + """Order two Bazel module versions: -1, 0, 1, or ``None`` when undecidable. + + Not a string comparison: ``"0.0.10" < "0.0.6"`` holds lexically, which would report the real + ``score_crates`` conflict backwards. ``None`` rather than a guess when the two carry different + alphanumeric identifiers (``1.17.0.bcr.2`` vs ``1.17.0``). + """ + if left == right: + return 0 + left_ids, left_pre = _version_identifiers(left) + right_ids, right_pre = _version_identifiers(right) + for a, b in zip(left_ids, right_ids, strict=False): + order = _compare_identifier(a, b) + if order is None: + return None + if order: + return order + if len(left_ids) != len(right_ids): + shared = min(len(left_ids), len(right_ids)) + longer, sign = (left_ids, 1) if len(left_ids) > len(right_ids) else (right_ids, -1) + # 0.1 vs 0.1.1 -> the longer one is higher. 1.17.0 vs 1.17.0.bcr.2 -> undecidable. + return sign if longer[shared].isdigit() else None + if left_pre != right_pre: + if not left_pre: + return 1 + if not right_pre: + return -1 + return None + return 0 + + +def _highest(versions: list[str]) -> str | None: + """The greatest of ``versions``, or ``None`` if any pair is undecidable.""" + highest = versions[0] + for candidate in versions[1:]: + order = _compare_versions(candidate, highest) + if order is None: + return None + if order > 0: + highest = candidate + return highest + + +def _empty_report() -> dict: + return { + "schema": 1, + "policy": POLICY, + "counts": { + "pinned": 0, + "asserted": 0, + "incidental": 0, + "uncarried": 0, + "conflicts": 0, + "ref_int_internal_drift": 0, + }, + "pins": {}, + "uncarried": [], + "ref_int_internal_drift": [], + "limitations": [], + } + + +_DEV_EDGE_LIMITATION = ( + "A non-root dev_dependency edge is never loaded by Bazel, so a consumer's dev-declared version " + "cannot appear in declared_versions. Measured on ref_int: score_toolchains_rust (0.9.1), " + "score_itf (0.4.0) and score_rules_imagefs are dev-declared by modules under test. Stage 2 " + "completes this from each module's own MODULE.bazel." +) +_OUT_OF_GRAPH_LIMITATION = ( + "A dependency of a module outside ref_int's own graph is invisible here: rules_distroless " + "requires yq.bzl 0.3.1 and never appears in ref_int's graph, so ref_int's incidental yq.bzl " + "pin looks unconflicted while it breaks that module's load phase." +) +_NO_VERBOSE_LIMITATION = ( + "The graph carried no originalVersion for any module, so consumer requests were unavailable " + "and every verdict is 'unknown'. Produce the graph with " + "'bazel mod graph --verbose --output=json' to populate them." +) + + +def _build_report( + resolved: dict[str, Module], + provenance: dict[str, str], + uncarried: list[dict[str, str]], + declared_by: dict[str, set[str]], + internal_drift: list[dict[str, object]], +) -> dict: + """Assemble the sidecar: where each pin came from, and who disagrees with it. + + ``provenance`` separates the two: ``asserted`` means ref_int wrote an override, so the version + is a decision; ``incidental`` means it merely inherited whatever MVS selected. Both are imposed + with equal force -- the ``yq.bzl`` pin that breaks score_communication is incidental, since + nothing in ref_int declares yq.bzl at all. + """ + report = _empty_report() + conflicts: list[str] = [] + for name in sorted(resolved): + module = resolved[name] + wanted = sorted(declared_by.get(name, ())) + entry: dict[str, object] = { + "pin": {"version": module.version} if module.version else {"repo": module.repo, "hash": module.hash}, + "provenance": provenance.get(name, "incidental"), + "declared_versions": wanted, + "verdict": "unknown", + "direction": None, + } + if not wanted: + # Nothing asked for a different version, or the request rode an edge Bazel never loaded. + # Indistinguishable here, hence "unknown" rather than "agree". + entry["verdict"] = "by_commit" if not module.version else "unknown" + elif not module.version: + entry["verdict"] = "differs" # a commit replaced a version request: not comparable + elif all(v == module.version for v in wanted): + entry["verdict"] = "agree" + else: + entry["verdict"] = "differs" + highest = _highest(wanted) + order = None if highest is None else _compare_versions(module.version, highest) + if order is not None: + entry["direction"] = "ref_int_lower" if order < 0 else "ref_int_higher" + if entry["verdict"] == "differs": + conflicts.append(name) + report["pins"][name] = entry + + report["uncarried"] = uncarried + report["ref_int_internal_drift"] = internal_drift + asserted = sum(1 for v in provenance.values() if v == "asserted") + report["counts"] = { + "pinned": len(resolved), + "asserted": asserted, + "incidental": len(resolved) - asserted, + "uncarried": len(uncarried), + "conflicts": len(conflicts), + "ref_int_internal_drift": len(internal_drift), + } + # The first two are properties of what Bazel loads, not of this run, so they always apply. + report["limitations"] = [_DEV_EDGE_LIMITATION, _OUT_OF_GRAPH_LIMITATION] + if not declared_by: + report["limitations"].append(_NO_VERBOSE_LIMITATION) + return report + + +def _warn_report(report: dict) -> None: + """Surface the report's findings as GitHub annotations, without failing the export. + + ``::warning::`` rather than ``logging.warning``, which does not reach the job UI -- the reason + silent downgrades and the dropped ``rules_oci`` pin went unnoticed. The export always exits 0; + ref_int's version is imposed and reported rather than blocking the run. + """ + for entry in report["uncarried"]: + print( + f"::warning::resolved_dependencies - ref_int declares an override for " + f"{entry['module']} that the manifest cannot carry: {entry['reason']}. {entry['consequence']}." + ) + for name, pin in report["pins"].items(): + if pin["verdict"] != "differs": + continue + pinned = pin["pin"].get("version") or f"commit {pin['pin'].get('hash', '')[:12]}" + direction = f" ({pin['direction']})" if pin["direction"] else "" + print( + f"::warning::resolved_dependencies - ref_int pins {name} at {pinned}{direction}; " + f"consumers in ref_int's graph declare {', '.join(pin['declared_versions'])}. " + f"ref_int's version is imposed." + ) + + def _parse_args() -> argparse.Namespace: parser = argparse.ArgumentParser( description="Resolve (Stage 1) or inject (Stage 2) ref_int's resolved dependency set (DR-008 Option 4)." @@ -636,6 +967,16 @@ def _parse_args() -> argparse.Namespace: default=None, help=f"Export mode: write the merged resolved set to this {MANIFEST_NAME} manifest and exit.", ) + parser.add_argument( + "--report", + type=Path, + default=None, + help=( + f"Export mode: write the pin report to this path instead of {REPORT_NAME} beside the " + f"manifest. Produce the graph with 'bazel mod graph --verbose --output=json' so the " + f"report can name the consumers that asked for a different version." + ), + ) parser.add_argument( "--module-under-test", default=None, @@ -674,8 +1015,16 @@ def main() -> None: # to, the graph says which of them a given module actually depends on. graph_copy = export.parent / GRAPH_NAME graph_copy.write_text(mod_graph.read_text()) + report_path = args.report or export.parent / REPORT_NAME + resolved.write_report(report_path) + counts = resolved.report["counts"] print(f"Wrote resolved dependency manifest ({len(resolved.names)} modules) to {export}") print(f"Stored dependency graph for Stage 2 at {graph_copy}") + print( + f"Wrote pin report to {report_path} " + f"({counts['asserted']} asserted, {counts['incidental']} incidental, " + f"{counts['conflicts']} conflicting, {counts['uncarried']} uncarried)" + ) return # Inject mode (Stage 2): overwrite a module's MODULE.bazel with the resolved set. diff --git a/scripts/known_good/tests/test_resolved_dependencies.py b/scripts/known_good/tests/test_resolved_dependencies.py index a0968ee0994..fb7fce9de3c 100644 --- a/scripts/known_good/tests/test_resolved_dependencies.py +++ b/scripts/known_good/tests/test_resolved_dependencies.py @@ -33,8 +33,12 @@ INJECTION_BEGIN, INJECTION_END, MANIFEST_NAME, + REPORT_NAME, DependencyGraph, ResolvedDependencies, + _compare_versions, + _declared_dep_specs, + _declared_deps, ) KNOWN_GOOD = { @@ -523,6 +527,395 @@ def test_ignores_commented_out_overrides(self, tmp_path: Path): assert rd.get("rules_rpm") is None # commented-out override must not be carried +class TestUncarriedOverridesAreNeverSilent: + """Every override ref_int declares reaches the manifest, or is named in the report. + + A silently dropped override is ref_int's authority quietly failing: the pin looks present in its + MODULE.bazel while every module resolves its own version. ``rules_oci`` was exactly that, and + logged nothing at all. The guard is a set difference over every override kind. + """ + + @staticmethod + def _graph(*names: str) -> dict: + # Overridden modules report an empty version, so the override file is the only source. + return { + "key": "", + "name": "ref_int", + "version": "", + "dependencies": [{"name": n, "version": ""} for n in names], + } + + def _export(self, tmp_path: Path, override_text: str, *graph_names: str) -> ResolvedDependencies: + graph = tmp_path / "graph.json" + graph.write_text(json.dumps(self._graph(*graph_names))) + root = tmp_path / "MODULE.bazel" + root.write_text(override_text) + return ResolvedDependencies.from_mod_graph(graph, [root]) + + @staticmethod + def _uncarried(rd: ResolvedDependencies) -> dict[str, dict]: + return {entry["module"]: entry for entry in rd.report["uncarried"]} + + def test_git_override_with_tag_and_no_commit_is_reported(self, tmp_path: Path): + # The real rules_oci shape: a mutable ref cannot be carried, but must not vanish silently. + rd = self._export( + tmp_path, + 'git_override(\n module_name = "rules_oci",\n' + ' remote = "https://github.com/bazel-contrib/rules_oci.git",\n tag = "v2.3.1",\n)\n', + "rules_oci", + ) + assert rd.get("rules_oci") is None + entry = self._uncarried(rd)["rules_oci"] + assert entry["kind"] == "git_override" + assert "v2.3.1" in entry["reason"] + assert "commit" in entry["reason"] + + def test_archive_override_is_reported_with_its_kind(self, tmp_path: Path): + rd = self._export( + tmp_path, + 'archive_override(\n module_name = "rules_boost",\n urls = ["https://e/master.tar.gz"],\n)\n', + "rules_boost", + ) + assert rd.get("rules_boost") is None + assert self._uncarried(rd)["rules_boost"]["kind"] == "archive_override" + + def test_local_path_override_is_reported(self, tmp_path: Path): + # A third kind, so the guard is kind-agnostic rather than a list of known-bad cases. + rd = self._export( + tmp_path, + 'local_path_override(\n module_name = "some_dep",\n path = "../some_dep",\n)\n', + "some_dep", + ) + assert self._uncarried(rd)["some_dep"]["kind"] == "local_path_override" + + def test_single_version_override_without_a_version_is_reported(self, tmp_path: Path): + rd = self._export( + tmp_path, + 'single_version_override(\n module_name = "googletest",\n patch_strip = 1,\n)\n', + "googletest", + ) + assert rd.get("googletest") is None + assert "version" in self._uncarried(rd)["googletest"]["reason"] + + def test_commented_out_override_is_not_reported_as_uncarried(self, tmp_path: Path): + # A commented-out override is not a pin ref_int is making: neither manifest nor report. + rd = self._export( + tmp_path, + '# git_override(\n# module_name = "rules_rpm",\n# tag = "v1",\n# )\n', + ) + assert rd.get("rules_rpm") is None + assert "rules_rpm" not in self._uncarried(rd) + + def test_a_carried_override_is_not_reported(self, tmp_path: Path): + rd = self._export( + tmp_path, + 'git_override(\n module_name = "trlc",\n commit = "abc1234",\n' + ' remote = "https://github.com/x/trlc.git",\n)\n', + "trlc", + ) + assert rd.get("trlc").hash == "abc1234" + assert self._uncarried(rd) == {} + + def test_every_declared_override_is_pinned_or_uncarried(self, tmp_path: Path): + # The completeness invariant as a set identity, rather than a claim in a comment. + rd = self._export( + tmp_path, + 'git_override(\n module_name = "trlc",\n commit = "abc1234",\n' + ' remote = "https://github.com/x/trlc.git",\n)\n' + 'git_override(\n module_name = "rules_oci",\n remote = "https://e/o.git",\n' + ' tag = "v2.3.1",\n)\n' + 'archive_override(\n module_name = "rules_boost",\n urls = ["https://e/x.tar"],\n)\n', + "trlc", + "rules_oci", + "rules_boost", + ) + declared = {"trlc", "rules_oci", "rules_boost"} + assert declared - rd.names == set(self._uncarried(rd)) + assert declared & rd.names == {"trlc"} + + +class TestPinProvenance: + """Which pins ref_int decided, and which it merely inherited from MVS. + + Both are imposed equally, so the manifest cannot tell them apart, though fixing a bad pin needs + to know which it is. The yq.bzl pin that breaks score_communication is incidental: nothing in + ref_int declares yq.bzl, so nobody ever chose 0.1.1. + """ + + @staticmethod + def _graph() -> dict: + return { + "key": "", + "name": "ref_int", + "version": "", + "dependencies": [ + {"name": "trlc", "version": ""}, + {"name": "protobuf", "version": "29.1"}, + {"name": "googletest", "version": "1.17.0.bcr.2"}, + ], + } + + def _report(self, tmp_path: Path, override_text: str) -> dict: + graph = tmp_path / "graph.json" + graph.write_text(json.dumps(self._graph())) + root = tmp_path / "MODULE.bazel" + root.write_text(override_text) + return ResolvedDependencies.from_mod_graph(graph, [root]).report + + def test_override_declared_by_ref_int_is_asserted(self, tmp_path: Path): + report = self._report( + tmp_path, + 'git_override(\n module_name = "trlc",\n commit = "abc1234",\n' + ' remote = "https://github.com/x/trlc.git",\n)\n', + ) + assert report["pins"]["trlc"]["provenance"] == "asserted" + + def test_registry_version_only_from_the_graph_is_incidental(self, tmp_path: Path): + report = self._report(tmp_path, "") + assert report["pins"]["protobuf"]["provenance"] == "incidental" + + def test_bare_single_version_override_promotes_a_graph_pin_to_asserted(self, tmp_path: Path): + # The score_test_artifact_versions shape: a bare override for a module already in the graph + # at that version. A no-op for manifest content, so provenance is what shows it doing anything. + report = self._report( + tmp_path, + 'single_version_override(\n module_name = "googletest",\n version = "1.17.0.bcr.2",\n)\n', + ) + assert report["pins"]["googletest"]["provenance"] == "asserted" + assert report["pins"]["googletest"]["pin"] == {"version": "1.17.0.bcr.2"} + + def test_counts_split_the_set(self, tmp_path: Path): + report = self._report( + tmp_path, + 'git_override(\n module_name = "trlc",\n commit = "abc1234",\n' + ' remote = "https://github.com/x/trlc.git",\n)\n', + ) + counts = report["counts"] + assert counts["asserted"] + counts["incidental"] == counts["pinned"] + assert counts["asserted"] == 1 + + +class TestVersionComparison: + """Version ordering, because a lexical compare silently inverts the whole report. + + ``"0.0.10" < "0.0.6"`` holds for strings, and score_crates is exactly that pair -- so a naive + compare would report ref_int as ahead of its consumers on the case that matters most. + """ + + def test_numeric_identifiers_compare_numerically_not_lexically(self): + assert _compare_versions("0.0.10", "0.0.6") == 1 + assert _compare_versions("0.0.6", "0.0.10") == -1 + + @pytest.mark.parametrize( + ("higher", "lower"), + [ + ("0.4.0", "0.2.0"), # score_itf + ("0.9.1", "0.8.0"), # score_toolchains_rust + ("0.0.3", "0.0.1"), # score_rules_imagefs + ("0.3.1", "0.1.1"), # yq.bzl + ("0.68.2-score", "0.68.1-score"), # rules_rust, prerelease suffix on both sides + ("1.3.1.bcr.8", "1.3.1.bcr.5"), # zlib, four-identifier registry version + ("0.1.1", "0.1"), # a longer numeric identifier list outranks a shorter one + ], + ) + def test_real_manifest_versions(self, higher: str, lower: str): + assert _compare_versions(higher, lower) == 1 + assert _compare_versions(lower, higher) == -1 + + def test_equal_versions_compare_equal(self): + assert _compare_versions("1.17.0.bcr.2", "1.17.0.bcr.2") == 0 + + def test_undecidable_pair_returns_none(self): + # These differ by an alphanumeric identifier with no defensible order. + assert _compare_versions("1.17.0.bcr.2", "1.17.0") is None + assert _compare_versions("1.17.0", "1.17.0.bcr.2") is None + + def test_prerelease_sorts_below_the_same_release(self): + assert _compare_versions("0.51.0", "0.51.0-rc2") == 1 + + +class TestConflictReportDoesNotAbort: + """ref_int's version is imposed, the disagreement is reported, and the export never aborts. + + Never raised to the consumer's version, never fallen back to. All that changes is visibility: an + override suppresses Bazel's own --check_direct_dependencies warning, so before this report all + five measured downgrades produced no output at all. + """ + + @staticmethod + def _graph(name: str, resolved_version: str, *declared: str) -> dict: + # --verbose records the version a dependent asked for whenever MVS moved the module off it. + return { + "key": "", + "name": "ref_int", + "version": "", + "dependencies": [{"name": name, "version": resolved_version, "originalVersion": d} for d in declared] + or [{"name": name, "version": resolved_version}], + } + + def _export(self, tmp_path: Path, graph: dict, override_text: str = "") -> ResolvedDependencies: + graph_file = tmp_path / "graph.json" + graph_file.write_text(json.dumps(graph)) + root = tmp_path / "MODULE.bazel" + root.write_text(override_text) + return ResolvedDependencies.from_mod_graph(graph_file, [root]) + + def test_a_downgrade_is_reported_and_the_export_still_writes_the_manifest(self, tmp_path: Path): + rd = self._export(tmp_path, self._graph("score_rules_imagefs", "0.0.1", "0.0.3")) + pin = rd.report["pins"]["score_rules_imagefs"] + assert pin["verdict"] == "differs" + assert pin["direction"] == "ref_int_lower" + assert pin["declared_versions"] == ["0.0.3"] + # ref_int's version is imposed, not raised to the consumer's. + assert rd.get("score_rules_imagefs").version == "0.0.1" + manifest = tmp_path / "resolved_versions.json" + rd.to_file(manifest) + assert json.loads(manifest.read_text())["modules"]["score_rules_imagefs"] == {"version": "0.0.1"} + + def test_an_upgrade_is_reported_as_the_other_direction(self, tmp_path: Path): + rd = self._export(tmp_path, self._graph("stardoc", "0.7.2", "0.7.1")) + assert rd.report["pins"]["stardoc"]["direction"] == "ref_int_higher" + + def test_agreement_is_not_a_conflict(self, tmp_path: Path): + rd = self._export(tmp_path, self._graph("protobuf", "29.1", "29.1")) + assert rd.report["pins"]["protobuf"]["verdict"] == "agree" + assert rd.report["counts"]["conflicts"] == 0 + + def test_commit_pin_reports_the_conflict_with_null_direction(self, tmp_path: Path): + # The score_crates shape: no comparable version, but the disagreement must stay visible -- + # otherwise the obvious "fix" is dropping commit pins from the report and losing 18 of them. + rd = self._export( + tmp_path, + self._graph("score_crates", "0.0.6", "0.0.10", "0.0.9"), + 'git_override(\n module_name = "score_crates",\n commit = "a5f4f57",\n' + ' remote = "https://github.com/eclipse-score/score-crates.git",\n)\n', + ) + pin = rd.report["pins"]["score_crates"] + assert pin["verdict"] == "differs" + assert pin["direction"] is None + assert pin["declared_versions"] == ["0.0.10", "0.0.9"] + assert rd.report["counts"]["conflicts"] == 1 + + def test_undecidable_comparison_reports_no_direction(self, tmp_path: Path): + rd = self._export(tmp_path, self._graph("googletest", "1.17.0.bcr.2", "1.17.0")) + pin = rd.report["pins"]["googletest"] + assert pin["verdict"] == "differs" + assert pin["direction"] is None + + def test_a_dev_declared_conflict_is_not_claimed_to_be_detected(self, tmp_path: Path): + # A non-root dev edge leaves no originalVersion, so an empty declared_versions must not + # read as "nobody disagrees". + rd = self._export(tmp_path, self._graph("score_toolchains_rust", "0.8.0")) + assert rd.report["pins"]["score_toolchains_rust"]["verdict"] == "unknown" + assert any("dev_dependency" in limitation for limitation in rd.report["limitations"]) + + def test_graph_without_verbose_says_so(self, tmp_path: Path): + # No requests at all must not read as universal agreement. + rd = self._export(tmp_path, self._graph("protobuf", "29.1")) + assert any("originalVersion" in limitation for limitation in rd.report["limitations"]) + + def test_verbose_graph_drops_the_no_requests_caveat(self, tmp_path: Path): + rd = self._export(tmp_path, self._graph("protobuf", "29.1", "28.0")) + assert not any("originalVersion" in limitation for limitation in rd.report["limitations"]) + # The structural caveats describe what Bazel loads, not this run, so they always apply. + assert any("dev_dependency" in limitation for limitation in rd.report["limitations"]) + + +class TestInternalDrift: + """ref_int disagreeing with itself: a declared bazel_dep version it does not actually impose. + + The declaration reads like a decision but is only a floor, so the version in review is not the + one Stage 2 imposes. Five are live today. + """ + + def test_declared_floor_below_the_resolved_version_is_reported(self, tmp_path: Path): + graph = tmp_path / "graph.json" + graph.write_text( + json.dumps( + { + "key": "", + "name": "ref_int", + "version": "", + "dependencies": [{"name": "rules_cc", "version": "0.2.17"}], + } + ) + ) + root = tmp_path / "MODULE.bazel" + root.write_text('bazel_dep(name = "rules_cc", version = "0.2.16")\n') + report = ResolvedDependencies.from_mod_graph(graph, [root]).report + assert report["ref_int_internal_drift"] == [ + { + "module": "rules_cc", + "declared": "0.2.16", + "resolved": "0.2.17", + "file": "MODULE.bazel", + "dev_dependency": False, + } + ] + + def test_dev_scoped_declaration_is_recorded_as_such(self, tmp_path: Path): + graph = tmp_path / "graph.json" + graph.write_text( + json.dumps( + { + "key": "", + "name": "ref_int", + "version": "", + "dependencies": [{"name": "score_toolchains_rust", "version": "0.9.1"}], + } + ) + ) + root = tmp_path / "MODULE.bazel" + root.write_text('bazel_dep(name = "score_toolchains_rust", version = "0.8.0", dev_dependency = True)\n') + drift = ResolvedDependencies.from_mod_graph(graph, [root]).report["ref_int_internal_drift"] + assert drift[0]["dev_dependency"] is True + + def test_declaration_matching_the_resolved_version_is_not_drift(self, tmp_path: Path): + graph = tmp_path / "graph.json" + graph.write_text( + json.dumps( + { + "key": "", + "name": "ref_int", + "version": "", + "dependencies": [{"name": "rules_pkg", "version": "1.2.0"}], + } + ) + ) + root = tmp_path / "MODULE.bazel" + root.write_text('bazel_dep(name = "rules_pkg", version = "1.2.0")\n') + report = ResolvedDependencies.from_mod_graph(graph, [root]).report + assert report["ref_int_internal_drift"] == [] + + +class TestDeclaredDepSpecs: + def test_captures_version_and_dev_flag(self): + # Real score_communication declarations, including a repo_name after the version. + text = ( + 'bazel_dep(name = "score_crates", version = "0.0.10", repo_name = "score_communication_crate_index")\n' + 'bazel_dep(name = "score_itf", version = "0.4.0", dev_dependency = True)\n' + 'bazel_dep(name = "score_toolchains_rust", version = "0.9.1", dev_dependency = True)\n' + ) + specs = _declared_dep_specs(text) + assert specs["score_crates"] == {"version": "0.0.10", "dev_dependency": False} + assert specs["score_itf"] == {"version": "0.4.0", "dev_dependency": True} + assert specs["score_toolchains_rust"]["dev_dependency"] is True + + def test_dep_without_version_is_captured_with_none(self): + # The non-registry idiom: the override supplies the source, so no version is declared. + assert _declared_dep_specs('bazel_dep(name = "score_tooling")\n') == { + "score_tooling": {"version": None, "dev_dependency": False} + } + + def test_declared_deps_set_is_unchanged(self): + # The narrower contract overwrite() depends on: names only, dev-declared included. + text = ( + 'bazel_dep(name = "score_crates", version = "0.0.10")\n' + 'bazel_dep(name = "score_itf", version = "0.4.0", dev_dependency = True)\n' + ) + assert _declared_deps(text) == {"score_crates", "score_itf"} + + class TestManifestRoundtrip: def test_to_file_is_lean_and_roundtrips(self, tmp_path: Path, resolved: ResolvedDependencies): manifest = tmp_path / "resolved_versions.json" @@ -534,6 +927,51 @@ def test_to_file_is_lean_and_roundtrips(self, tmp_path: Path, resolved: Resolved assert loaded.get("score_baselibs").hash == resolved.get("score_baselibs").hash assert loaded.get("score_tooling").version == "1.2.0" + def test_report_is_a_sidecar_and_the_manifest_schema_is_unchanged(self, tmp_path: Path): + # The manifest is the Stage1->Stage2 contract, so new fields go in the sidecar instead. + graph = tmp_path / "graph.json" + graph.write_text( + json.dumps( + { + "key": "", + "name": "ref_int", + "version": "", + "dependencies": [ + {"name": "protobuf", "version": "29.1", "originalVersion": "28.0"}, + {"name": "trlc", "version": ""}, + ], + } + ) + ) + root = tmp_path / "MODULE.bazel" + root.write_text( + 'git_override(\n module_name = "trlc",\n commit = "abc1234",\n' + ' remote = "https://github.com/x/trlc.git",\n)\n' + ) + rd = ResolvedDependencies.from_mod_graph(graph, [root]) + + manifest = tmp_path / MANIFEST_NAME + rd.to_file(manifest) + modules = json.loads(manifest.read_text())["modules"] + # Exactly the two legal shapes; nothing leaks in from the report. + assert modules == { + "protobuf": {"version": "29.1"}, + "trlc": {"repo": "https://github.com/x/trlc.git", "hash": "abc1234"}, + } + assert ResolvedDependencies.from_file(manifest).get("trlc").hash == "abc1234" + + report = tmp_path / REPORT_NAME + rd.write_report(report) + parsed = json.loads(report.read_text()) + assert parsed["schema"] == 1 + assert parsed["pins"]["protobuf"]["declared_versions"] == ["28.0"] + + def test_a_manifest_read_back_carries_no_report(self, tmp_path: Path, resolved: ResolvedDependencies): + # The pins survive a round trip; the evidence behind them is not faked. + manifest = tmp_path / MANIFEST_NAME + resolved.to_file(manifest) + assert ResolvedDependencies.from_file(manifest).report["pins"] == {} + class TestFromResolvedArtifact: def test_reads_the_manifest(self, tmp_path: Path, resolved: ResolvedDependencies): From 2761dccd3c8bfd664e6c18d2adaae27e7ab1bffe Mon Sep 17 00:00:00 2001 From: subramaniak Date: Wed, 12 Aug 2026 14:10:10 +0000 Subject: [PATCH 13/14] fix: correct stale claims in comments --- scripts/known_good/BUILD | 3 +- scripts/known_good/resolved_dependencies.py | 34 +++++++------------ .../tests/test_resolved_dependencies.py | 14 ++++---- 3 files changed, 23 insertions(+), 28 deletions(-) diff --git a/scripts/known_good/BUILD b/scripts/known_good/BUILD index d782d0d2216..cfdff652c00 100644 --- a/scripts/known_good/BUILD +++ b/scripts/known_good/BUILD @@ -14,7 +14,8 @@ load("@rules_python//python:defs.bzl", "py_binary", "py_library") load("@score_tooling//python_basics:defs.bzl", "score_py_pytest") # Library target: the known_good package (models + generators). -# Used as a dep by //scripts/tooling and by the resolve_deps binary below. +# Depended on by the test and binary targets below. Note //scripts/tooling has its own separate +# lib/known_good package and does not use this one. py_library( name = "known_good", srcs = glob( diff --git a/scripts/known_good/resolved_dependencies.py b/scripts/known_good/resolved_dependencies.py index cfbebf1151a..0e8cf8b3834 100644 --- a/scripts/known_good/resolved_dependencies.py +++ b/scripts/known_good/resolved_dependencies.py @@ -66,22 +66,17 @@ def _repo_root() -> Path: INJECTION_END = "# --- END ref_int resolved-deps injection ---" -def generate_override_directive(module: Module, repo_commit_dict: dict[str, str] | None = None) -> str | None: +def generate_override_directive(module: Module) -> str | None: """Return the override directive (single_version_override / git_override) for a module. - Returns just the override call without a preceding ``bazel_dep(...)`` line, so the - same logic can be reused both to build ref_int's score_modules_*.MODULE.bazel files - and to inject overrides into a module's own MODULE.bazel where bazel_dep is already - declared (see :meth:`ResolvedDependencies.overwrite`). + Returns just the override call without a preceding ``bazel_dep(...)`` line, since + :meth:`ResolvedDependencies.overwrite` injects into a module's own MODULE.bazel where the + ``bazel_dep`` is already declared, and adds one itself only for a closure member that is not. Returns ``None`` when the module has neither a usable version nor a valid repo+commit. """ - repo_commit_dict = repo_commit_dict or {} commit = module.hash - if module.repo in repo_commit_dict: - commit = repo_commit_dict[module.repo] - patches_lines = "" if module.bazel_patches: patches_lines = " patches = [\n" @@ -214,8 +209,6 @@ def closure(self, module_name: str) -> set[str]: # Built-in / non-registry modules that must not be given a single_version_override. _SKIP_MODULES = {"bazel_tools"} -# Capture the module name from any ``bazel_dep(name = "...")`` call (name is the first arg). -_BAZEL_DEP_RE = re.compile(r'bazel_dep\(\s*name\s*=\s*"([^"]+)"') # The whole ``bazel_dep(...)`` argument list. ``[^)]*`` is sufficient: bazel_dep takes only # scalar keyword arguments, never a nested call. _BAZEL_DEP_CALL_RE = re.compile(r"bazel_dep\((?P[^)]*)\)", re.S) @@ -460,9 +453,9 @@ def from_mod_graph(cls, mod_graph_json: Path, override_files: list[Path]) -> Res for name, version in versions.items(): if name in resolved or name in _SKIP_MODULES: continue # already carried by an override directive, or non-overridable - # A literal 0.0.0 means the module declares 0.0.0 itself and ref_int overrides it with - # nothing, so there is no version worth imposing. (Overridden modules report an empty - # version, already dropped upstream by _collect_resolved_versions.) + # Defensive: a module declaring 0.0.0 itself that ref_int does not override has no + # version worth imposing. Every 0.0.0 module in ref_int's graph today (trlc, + # score_tooling, the score_* target modules) is already carried by an override above. if version == "0.0.0": declared_overrides.setdefault( name, @@ -572,13 +565,12 @@ def overwrite( * ref_int resolved nothing for it -> leave the module's own declaration untouched and log it. There is nothing to impose. - ``dev_dependency`` is not the discriminator and is never read. The two properties are - independent: ``score_baselibs`` declares 11 dev-only deps ref_int *has* resolved - (``score_tooling``, ``score_docs_as_code``, ``toolchains_llvm``, ...), while - ``score_baselibs_rust`` is a public dep ref_int has *not* resolved. A dependency is absent - from the resolved set because nothing in ref_int's own graph reaches it -- usually it is - only reachable through some module's dev edge, inactive while ref_int is root -- or because - ref_int pins it with an ``archive_override`` the manifest cannot express. + ``dev_dependency`` is not the discriminator and is never read: it does not predict whether + ref_int resolved a dependency. ``score_baselibs`` at 0.2.9 declares 13 dev-only deps that + ref_int *has* resolved and therefore pins (``score_tooling``, ``score_docs_as_code``, + ``toolchains_llvm``, ...). A dependency is absent from the resolved set for an unrelated + reason: nothing in ref_int's own graph reaches it, or ref_int pins it with an override the + manifest cannot express (``rules_boost``, an ``archive_override``). Scope is the module's declared deps plus ``closure()`` of the module and of each declared dep. The closure is what makes the rule above safe rather than merely permissive: pinning diff --git a/scripts/known_good/tests/test_resolved_dependencies.py b/scripts/known_good/tests/test_resolved_dependencies.py index fb7fce9de3c..31d0466d826 100644 --- a/scripts/known_good/tests/test_resolved_dependencies.py +++ b/scripts/known_good/tests/test_resolved_dependencies.py @@ -470,15 +470,17 @@ def test_public_dep_is_still_pinned(self, resolved: ResolvedDependencies, dev_an class TestFromModGraph: @staticmethod def _graph() -> dict: - # Mirrors 'bazel mod graph --output=json': overridden modules report version 0.0.0. + # Mirrors 'bazel mod graph --output=json': an overridden module reports whatever version its + # own MODULE.bazel declares, which is empty when it declares none -- never the version + # ref_int meant to impose. All three values below are the ones ref_int's real graph reports. return { "key": "", "name": "ref_int", "version": "", "dependencies": [ - {"name": "trlc", "version": "0.0.0"}, # git_override (carried from file) - {"name": "rules_boost", "version": "0.0.0"}, # archive_override (not representable) - {"name": "score_baselibs", "version": "0.0.0"}, # git_override (carried from file) + {"name": "trlc", "version": "0.0.0"}, # git_override; trlc declares 0.0.0 itself + {"name": "rules_boost", "version": ""}, # archive_override; declares no version + {"name": "score_baselibs", "version": "0.2.9"}, # git_override; declares a real version { "name": "protobuf", "version": "29.1", @@ -505,13 +507,13 @@ def test_merges_overrides_and_registry_versions(self, tmp_path: Path): ) rd = ResolvedDependencies.from_mod_graph(graph, [root, scoremods]) - # Overridden modules carried as their real git_override (graph's 0.0.0 ignored). + # Overridden modules carried as their real git_override; the graph's version is ignored. assert rd.get("trlc").hash == "abc1234" assert rd.get("score_baselibs").hash == "def5678" # Registry modules carried from the resolved graph version. assert rd.get("protobuf").version == "29.1" assert rd.get("abseil-cpp").version == "20250512.1" - # archive_override target at 0.0.0 is not representable -> not carried. + # An archive_override cannot be expressed as a manifest directive -> not carried. assert rd.get("rules_boost") is None def test_ignores_commented_out_overrides(self, tmp_path: Path): From f56cf806c0ef6f51ac45ec71a6f26ba77aa83002 Mon Sep 17 00:00:00 2001 From: subramaniak Date: Fri, 14 Aug 2026 03:07:42 +0000 Subject: [PATCH 14/14] fix: match Bazel's version ordering exactly and surface non-verbose graphs --- scripts/known_good/resolved_dependencies.py | 110 ++++++++++-------- .../tests/test_resolved_dependencies.py | 48 ++++++-- 2 files changed, 98 insertions(+), 60 deletions(-) diff --git a/scripts/known_good/resolved_dependencies.py b/scripts/known_good/resolved_dependencies.py index 0e8cf8b3834..197675eb8a4 100644 --- a/scripts/known_good/resolved_dependencies.py +++ b/scripts/known_good/resolved_dependencies.py @@ -745,63 +745,64 @@ def _collect_declared_versions(node: dict, acc: dict[str, set[str]]) -> None: _collect_declared_versions(dep, acc) -def _version_identifiers(version: str) -> tuple[list[str], str]: - """Split a Bazel module version into its release identifiers and its prerelease suffix.""" - release, _, prerelease = version.partition("-") - return release.split("."), prerelease +# Bazel's own module-version grammar: a mandatory release part, an optional ``-prerelease``, and +# optional ``+build`` metadata that is deliberately not captured because it does not affect ordering. +_VERSION_RE = re.compile(r"^(?P[a-zA-Z0-9.]+)(?:-(?P[a-zA-Z0-9.-]+))?(?:\+[a-zA-Z0-9.-]+)?$") -def _compare_identifier(left: str, right: str) -> int | None: - if left == right: - return 0 - if left.isdigit() and right.isdigit(): - return -1 if int(left) < int(right) else 1 - if left.isdigit() != right.isdigit(): - return -1 if left.isdigit() else 1 # Bazel orders numeric identifiers below alphanumeric - return None # two different alphanumerics have no defensible order +def _identifier_key(identifier: str) -> tuple[int, int, str]: + """Sort key for one version identifier, matching Bazel's ``Identifier.COMPARATOR``. + Digits-only identifiers sort *below* alphanumeric ones; two numeric identifiers compare + numerically; two alphanumeric ones compare lexicographically. + """ + if identifier.isdigit(): + return (0, int(identifier), "") + return (1, 0, identifier) -def _compare_versions(left: str, right: str) -> int | None: - """Order two Bazel module versions: -1, 0, 1, or ``None`` when undecidable. - Not a string comparison: ``"0.0.10" < "0.0.6"`` holds lexically, which would report the real - ``score_crates`` conflict backwards. ``None`` rather than a guess when the two carry different - alphanumeric identifiers (``1.17.0.bcr.2`` vs ``1.17.0``). +def _version_key(version: str) -> tuple[list[tuple[int, int, str]], int, list[tuple[int, int, str]]]: + """Sort key for a whole version, mirroring Bazel's ``Version.COMPARATOR`` chain. + + Release identifiers first (list comparison is lexicographic, so a shorter list sorts lower -- + ``1.17.0`` is below ``1.17.0.bcr.2``), then presence of a prerelease (a prerelease sorts *below* + the same release without one), then the prerelease identifiers, which are dot-split like the + release part rather than compared as one opaque string. + """ + match = _VERSION_RE.match(version) + if match is None: + # Not a version Bazel would accept; order it as a single alphanumeric identifier so the + # comparison stays total rather than raising on data we only ever report on. + return ([_identifier_key(version)], 1, []) + release = [_identifier_key(i) for i in match.group("release").split(".")] + prerelease = match.group("prerelease") + if not prerelease: + return (release, 1, []) + return (release, 0, [_identifier_key(i) for i in prerelease.split(".")]) + + +def _compare_versions(left: str, right: str) -> int: + """Order two Bazel module versions: -1, 0 or 1. + + Ports Bazel's own ordering (``Version.java``) rather than approximating it, because the report + claims to say whether ref_int's pin is behind what a consumer asked for -- saying "unknown" + where Bazel has a definite answer is misinformation, not caution. The ordering is total; Bazel + has no incomparable pair. + + Deliberately not a string comparison: ``"0.0.10" < "0.0.6"`` holds lexically, which would report + the real ``score_crates`` conflict backwards. """ if left == right: return 0 - left_ids, left_pre = _version_identifiers(left) - right_ids, right_pre = _version_identifiers(right) - for a, b in zip(left_ids, right_ids, strict=False): - order = _compare_identifier(a, b) - if order is None: - return None - if order: - return order - if len(left_ids) != len(right_ids): - shared = min(len(left_ids), len(right_ids)) - longer, sign = (left_ids, 1) if len(left_ids) > len(right_ids) else (right_ids, -1) - # 0.1 vs 0.1.1 -> the longer one is higher. 1.17.0 vs 1.17.0.bcr.2 -> undecidable. - return sign if longer[shared].isdigit() else None - if left_pre != right_pre: - if not left_pre: - return 1 - if not right_pre: - return -1 - return None - return 0 + left_key, right_key = _version_key(left), _version_key(right) + if left_key == right_key: + return 0 + return -1 if left_key < right_key else 1 -def _highest(versions: list[str]) -> str | None: - """The greatest of ``versions``, or ``None`` if any pair is undecidable.""" - highest = versions[0] - for candidate in versions[1:]: - order = _compare_versions(candidate, highest) - if order is None: - return None - if order > 0: - highest = candidate - return highest +def _highest(versions: list[str]) -> str: + """The greatest of ``versions`` under Bazel's ordering.""" + return max(versions, key=_version_key) def _empty_report() -> dict: @@ -877,10 +878,8 @@ def _build_report( entry["verdict"] = "agree" else: entry["verdict"] = "differs" - highest = _highest(wanted) - order = None if highest is None else _compare_versions(module.version, highest) - if order is not None: - entry["direction"] = "ref_int_lower" if order < 0 else "ref_int_higher" + order = _compare_versions(module.version, _highest(wanted)) + entry["direction"] = "ref_int_lower" if order < 0 else "ref_int_higher" if entry["verdict"] == "differs": conflicts.append(name) report["pins"][name] = entry @@ -910,6 +909,15 @@ def _warn_report(report: dict) -> None: silent downgrades and the dropped ``rules_oci`` pin went unnoticed. The export always exits 0; ref_int's version is imposed and reported rather than blocking the run. """ + if _NO_VERBOSE_LIMITATION in report["limitations"]: + # Without this the conflict half of the report is silently a constant: every verdict is + # "unknown" and no consumer disagreement can be detected, which reads identically to a + # graph in which nobody disagrees. + print( + "::warning::resolved_dependencies - the graph carries no originalVersion, so no " + "consumer version conflicts can be detected. Produce it with " + "'bazel mod graph --verbose --output=json' to enable them." + ) for entry in report["uncarried"]: print( f"::warning::resolved_dependencies - ref_int declares an override for " diff --git a/scripts/known_good/tests/test_resolved_dependencies.py b/scripts/known_good/tests/test_resolved_dependencies.py index 31d0466d826..8222d876461 100644 --- a/scripts/known_good/tests/test_resolved_dependencies.py +++ b/scripts/known_good/tests/test_resolved_dependencies.py @@ -372,9 +372,9 @@ def test_module_under_test_inferred_from_module_declaration( class TestScopeIsDecidedByTheResolvedSet: """Presence in the resolved set decides the pin scope. ``dev_dependency`` plays no part. - The two properties are independent, measured on the real modules: ``score_baselibs`` declares - 11 dev-only deps ref_int *has* resolved, while ``score_baselibs_rust`` is a *public* dep of - ``score_communication`` that ref_int has *not*. So the flag predicts neither case and cannot + The two properties are independent, measured on the real modules: ``score_baselibs`` at 0.2.9 + declares 13 dev-only deps ref_int *has* resolved, while ``score_baselibs_rust`` is a *public* + dep of ``score_logging`` that ref_int has *not*. So the flag predicts neither case and cannot be the discriminator. """ @@ -727,14 +727,31 @@ def test_real_manifest_versions(self, higher: str, lower: str): def test_equal_versions_compare_equal(self): assert _compare_versions("1.17.0.bcr.2", "1.17.0.bcr.2") == 0 - def test_undecidable_pair_returns_none(self): - # These differ by an alphanumeric identifier with no defensible order. - assert _compare_versions("1.17.0.bcr.2", "1.17.0") is None - assert _compare_versions("1.17.0", "1.17.0.bcr.2") is None + def test_a_longer_release_list_outranks_its_prefix(self): + # Bazel compares release identifiers lexicographically, so the shorter list sorts lower -- + # a BCR re-release of 1.17.0 is above it. This is ref_int's own googletest pin. + assert _compare_versions("1.17.0.bcr.2", "1.17.0") == 1 + assert _compare_versions("1.17.0", "1.17.0.bcr.2") == -1 + + def test_alphanumeric_identifiers_compare_lexicographically(self): + # Bazel's ordering is total: two differing alphanumeric identifiers still have an order. + assert _compare_versions("1.0.alpha", "1.0.beta") == -1 + assert _compare_versions("1.0.beta", "1.0.alpha") == 1 + + def test_numeric_identifiers_sort_below_alphanumeric_ones(self): + assert _compare_versions("1.0.2", "1.0.rc") == -1 def test_prerelease_sorts_below_the_same_release(self): assert _compare_versions("0.51.0", "0.51.0-rc2") == 1 + def test_prerelease_identifiers_are_dot_split(self): + # Compared identifier-wise like the release part, not as one opaque string -- otherwise + # "rc.10" would sort below "rc.9". + assert _compare_versions("1.0.0-rc.10", "1.0.0-rc.9") == 1 + + def test_build_metadata_does_not_affect_ordering(self): + assert _compare_versions("1.0.0+build1", "1.0.0+build2") == 0 + class TestConflictReportDoesNotAbort: """ref_int's version is imposed, the disagreement is reported, and the export never aborts. @@ -798,11 +815,13 @@ def test_commit_pin_reports_the_conflict_with_null_direction(self, tmp_path: Pat assert pin["declared_versions"] == ["0.0.10", "0.0.9"] assert rd.report["counts"]["conflicts"] == 1 - def test_undecidable_comparison_reports_no_direction(self, tmp_path: Path): + def test_a_bcr_rerelease_is_reported_as_higher(self, tmp_path: Path): + # ref_int's real googletest pin. Bazel orders 1.17.0.bcr.2 above 1.17.0, so the direction + # is knowable and must be stated -- reporting "unknown" here would be misinformation. rd = self._export(tmp_path, self._graph("googletest", "1.17.0.bcr.2", "1.17.0")) pin = rd.report["pins"]["googletest"] assert pin["verdict"] == "differs" - assert pin["direction"] is None + assert pin["direction"] == "ref_int_higher" def test_a_dev_declared_conflict_is_not_claimed_to_be_detected(self, tmp_path: Path): # A non-root dev edge leaves no originalVersion, so an empty declared_versions must not @@ -816,6 +835,17 @@ def test_graph_without_verbose_says_so(self, tmp_path: Path): rd = self._export(tmp_path, self._graph("protobuf", "29.1")) assert any("originalVersion" in limitation for limitation in rd.report["limitations"]) + def test_a_non_verbose_graph_is_announced_not_just_recorded(self, tmp_path: Path, capsys): + # The conflict half of the report is a constant without originalVersion -- every verdict + # "unknown", no disagreement detectable, which reads exactly like a graph nobody disagrees + # in. A limitations entry alone is invisible in a CI log, so it must be annotated too. + self._export(tmp_path, self._graph("protobuf", "29.1")) + assert "::warning::" in capsys.readouterr().out + + def test_a_verbose_graph_is_not_announced(self, tmp_path: Path, capsys): + self._export(tmp_path, self._graph("protobuf", "29.1", "28.0")) + assert "originalVersion" not in capsys.readouterr().out + def test_verbose_graph_drops_the_no_requests_caveat(self, tmp_path: Path): rd = self._export(tmp_path, self._graph("protobuf", "29.1", "28.0")) assert not any("originalVersion" in limitation for limitation in rd.report["limitations"])