Add resolved-dependency override mechanism for DR-008 - #278
Add resolved-dependency override mechanism for DR-008#278Subramanian-K812 wants to merge 7 commits into
Conversation
6d78316 to
fbc58a3
Compare
|
The created documentation from the pull request is available at: docu-html |
|
|
||
| # Alias: expose resolve_deps under //scripts/tooling for `bazel run //scripts/tooling:resolve_deps`. | ||
| alias( | ||
| name = "resolve_deps", |
There was a problem hiding this comment.
Why this alias? This is usually for backwards-compatibility but since this is new functionality it should not be necessary.
There was a problem hiding this comment.
The alias under //scripts/tooling provides reachability alongside other entry points. The implementation itself remains in //scripts/known_good since that’s where its models and tests live. The pipeline continues to call //scripts/known_good:resolve_deps directly; the alias simply exposes it without duplicating the target.
| """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. |
There was a problem hiding this comment.
I like this self-contained aspect. It allows to use this in an hermetic way.
However, the tradeoff is that we cannot parallelize the followup actions (build, test, docs, etc) via GitHub actions, can we?
There was a problem hiding this comment.
No, they can still run in parallel. Stage 1 resolves once and uploads resolved_versions.json as an artifact; each Stage 2 module job downloads that same artifact and does its own local override + build/test independently, still as a normal parallel matrix. The hermetic is only about the unit tests not needing real clones/bazel to validate scan()/overwrite() — it doesn't force any job serialization in the actual pipeline.
… markers, simplify imports
…directive inline, add BUILD for bazel run
…wn_good.py, and tooling BUILD format
…ing it, and make resolved_dependencies importable standalone
e32b107 to
cff77c1
Compare
OliverHeilwagen
left a comment
There was a problem hiding this comment.
Please fix transitive dependency pinning gap
| ) | ||
|
|
||
| # Runnable binary for the resolve + inject workflow. | ||
| # Stage 1 (export): bazel run //scripts/known_good:resolve_deps -- \ |
There was a problem hiding this comment.
Please extend description for Stage 1.
bazel mod graph --output json > graph.json is required as prerequisite.
In the script please make use of BUILD_WORKING_DIRECTORY or BUILD_WORKSPACE_DIRECTORY environment variable so the graph.json file can be found without requiring to be in data = ["..."]
| # 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 -- \ |
There was a problem hiding this comment.
Please keep Stage 1 and Stage 2 in sync.
--resolved-deps artifacts/ or _resolved_deps in both.
| import sys | ||
| from pathlib import Path | ||
|
|
||
| _HERE = Path(__file__).resolve().parent |
There was a problem hiding this comment.
Please make use of BUILD_WORKING_DIRECTORY or BUILD_WORKSPACE_DIRECTORY environment variable instead.
| version = _field(body, "version") | ||
| if name and version: | ||
| modules.append(Module(name=name, hash="", repo="", version=version)) | ||
|
|
There was a problem hiding this comment.
Please add support for multiple_version_override .
| 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()) |
There was a problem hiding this comment.
Please check file existence before using it.
| 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()] |
There was a problem hiding this comment.
| 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()] | |
| override_files = [ | |
| f for f in [repo_root / "MODULE.bazel", *sorted((repo_root / "bazel_common").glob("*.MODULE.bazel"))] | |
| if f.is_file() | |
| ] |
| if args.resolved_deps: | ||
| resolved = ResolvedDependencies.from_resolved_artifact(args.resolved_deps) | ||
| else: | ||
| resolved = ResolvedDependencies.from_known_good(args.known_good_path) |
There was a problem hiding this comment.
Transitive dependency pinning gap
overwrite() only injects overrides for modules the module-under-test directly declares
via bazel_dep. Transitive dependencies silently fall through to MVS resolution, which may
pick a different version than what ref_int validated against.
Example:
ref_int (resolved: flatbuffers @ 25.12.19)
└── communication (module under test)
└── bazel_dep(name = "score_baselibs") ← override injected ✅
└── flatbuffers ← transitive, NO override injected ❌
MVS picks its own version
Fix: For every module in the resolved set that is not directly declared by the
module-under-test, inject both a bazel_dep stub and the override into the injection block:
# brings the transitive dep into the root graph so the override is valid
bazel_dep(name = "flatbuffers", version = "0.0.0")
git_override(module_name = "flatbuffers", commit = "...", remote = "...")This also means from_known_good is insufficient for the inject mode, as it only carries
first-party score modules and has no transitive registry versions; --resolved-deps (from the
Stage-1 manifest) must be the required source for Stage 2 injection.
In Stage-1 we should also takeover and store the given graph.json as it is needed for Stage-2 to be aware of the necessary transitive overrides for a given MODULE.bazel file. Identification can be done via regex for module(name = "...").
DR-008: resolved-dependency resolve + override mechanism (PR 1 of 2)
Part of #264. This is the first of two PRs that split the original DR-008 Option 4
change. This PR adds only the mechanism; the follow-up
PR wires it into the test workflow.
What this PR does
Adds
ResolvedDependencies— the mechanism DR-008 Option 4 needs to (1) resolve the fullintegrated dependency set at the
reference_integrationroot and (2) override thoseresolved versions directly into an individual module.
(
MODULE.bazel+bazel_common/*.MODULE.bazel) with the post-MVS registry versionsfrom
bazel mod graph --output=json, and serializes the result to a singleresolved_versions.jsonmanifest (--export/from_mod_graph/to_file).MODULE.bazelfor every declaredbazel_depand appends agit_override/single_version_overridefor each one wehave a resolved version for, so the module builds/tests against the resolved set
(
scan/overwrite). Injection is append-only and idempotent (marked with begin/endcomments), always skips the module-under-test itself (the root is never overridden),
always overwrites any dependency the module already declares an override for (ref_int's
resolved version always wins), and is never committed back to module sources. A declared
dependency with no entry in the resolved set is left to resolve on its own and logs a
warning — this is expected to be effectively impossible once the resolved set comes from
the full
bazel mod graph(a superset of any module's own graph).Files
scripts/known_good/resolved_dependencies.pyResolvedDependenciesmechanism (resolve + override) + CLIscripts/known_good/tests/test_resolved_dependencies.pyscripts/known_good/BUILDpy_library :known_good,py_binary :resolve_deps(Bazel-runnable), andscore_py_pytest :known_good_testsso the unit tests run under Bazelscripts/tooling/BUILDalias :resolve_depsexposing it asbazel run //scripts/tooling:resolve_depsFollow-up (PR 2)
The two-stage
test_and_docs.yml(Stage 1 resolves + exports the manifest; Stage 2 checksout each module, overrides deps via this mechanism, and runs its UT + coverage; then
aggregates), the module-context
quality_runners.py, the dependency pin bumps, and theassociated config all land in the follow-up PR, which is stacked on this one.