Finish the Bazel module consolidation in reviewable steps
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 218
- Forks
- 72
- Avg merge
- 1d 12h
- Merged PRs (30d)
- 427
Description
Tracks finishing the Bazel module consolidation after NVIDIA/nvcf#593 was closed
as too large to review (1608 files). The work was correct and is preserved on
the preserve/consolidation-full branch; this issue re-lands it in reviewable
pieces.
Where things stand
tools/ci/nested-modules.txt lists 17 services that still carry their own
MODULE.bazel. Everything else already builds from the root module.
Two things are already done and merged:
- NVIDIA/nvcf#600 makes
bazel query 'deps(//...)'resolve and documents
dependency scanning. - The Java components and nvsnap migrated earlier.
Why the previous attempt was too large
Of the 1608 files in #593:
| what | files |
|---|---|
| nvca vendored BUILD relabels | 1113 |
| nvca's own BUILD files | 119 |
| the other 16 services | 376 |
nvca is the whole reviewability problem, and for a reason worth fixing on its
own merits.
Step 1: move nvca off vendored BUILD files
nvca is the only service that resolves Go dependencies through //vendor/...
labels. That requires a committed BUILD.bazel beside every vendored package:
1261 of them today. Adding a dependency means committing a BUILD file per
package in its transitive closure, so a single library bump churns hundreds of
files.
Every other service resolves through go_deps from go.mod, with vendor/
kept on disk for the non-Bazel path and listed in .bazelignore.
image-credential-helper already does exactly this.
Scope:
- delete 1261 vendored
BUILD.bazelfiles - rewrite 119 of nvca's own BUILD files from
//vendor/xto@go_depslabels - add
nvca/vendorto.bazelignore, add nvca togo.work.bazel
Two facts established before proposing this:
- Only one vendored BUILD file does anything a generated one would not
(x_defsstamping a version into the DRA driver'sinfopackage). It is
redundant: the same stamp is applied from nvca's own targets using the
fully-qualified import path, which works regardless of how the dependency
resolves. - Joining nvca's dependencies to the shared graph moves 10 versions, all minor
or patch:golang.org/x/{sync,term,net,sys}, two otel exporters, otelgrpc,
genproto, klauspost/compress, samber/lo. No majors, nothing in the
containerd, moby or k8s families, andotelitself does not move, so the
semconv schema conflict seen in #593 does not arise.
This lands independently of consolidation and is worth doing regardless.
Step 2: shared rules parity
Root's rules/oci is a strict subset of what the services use. Every migration
fails at load time until it is a superset. Additive, touches no service.
Needs: binary_path (used by 15 of 17), extra_layers, env,
extra_registries, cmd, go_oci_multi_binary_image, rust_oci_image, and
layer visibility following the image so entrypoint-mode tests can inspect it.
Step 3: migrate the remaining 16 services
About 376 files, in batches of four. Each batch: delete the nested module and
its duplicated scaffold, join go.work.bazel, leave .bazelignore, rewrite
subtree-relative labels, convert the CI matrix row to root-scoped.
The first batch must also carry the CI partition fix, because that is when
.bazelignore stops separating the root row from the service rows.
Step 4: paired release-config change
The private release repo builds each service from a source.subtree with
subtree-relative target labels. Those only resolve inside a nested module. The
matching change is written and open, and must land in the same window as the
migration: on its own it points release builds at labels that do not exist, and
the migration without it points them at a subtree with no module. Neither
repository's CI catches the mismatch, because the failure is a broken release
rather than a failed check.
Findings that must carry forward
Recorded because each cost real time to find and none is obvious:
- A
replacedirective is not scoped to its own module once that module joins
the rootgo.work. Two services carried 19 between them for modules they
never import, which re-pinned the whole workspace and broke unrelated
subtrees with missing-sum errors. - Bazel honours the
manualtag only when expanding wildcard patterns. CI
resolves an explicit label list, so ten deliberately quarantined tests run in
CI whilebazel test //...skips them locally. .bazelignoreimplicitly enforces the one-row-per-target CI partition.
Removing entries makes the root row a superset of every service row, which
doubles the work and lets 68 fixed-port test packages contend on one machine.cargo_build_scripttargets carry no edition and take the toolchain default.
One root module registers one default, so a crate whose build script needs a
newer edition fails.- Relative paths in
build_script_envbreak when a package moves deeper.
Nothing in analysis catches it; the build script fails at execution. - Nested
.bazelignorefiles carry load-bearing exclusions that must be lifted
with a path prefix.
Known defects, not in scope here
- 68 test packages bind literal ports, overlapping heavily across services.
Safe while each has a CI row to itself. Should move to ephemeral binding. image-push-manualand the matrix build key their Bazel cache per row, while
the cached content is identical for every root-module row. Fixed on the
preserved branch; needs re-landing with step 3.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with tools/ci/nested-modules.txt and the preserved preserve/consolidation-full branch, then review nvca's BUILD files, .bazelignore, and go.work.bazel. Work through the numbered migration steps and their listed CI and release-config constraints. Done means the remaining services use the root module, CI partitions remain correct, and the paired release change lands without broken release targets.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- build-system, ci-cd
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100