fix(docs): detect Bazel scopes from impacted targets, not changed files - #12476
Conversation
Merge Protections🔴 2 of 6 protections blocking · waiting on 👀 reviews
🔴 👀 Review RequirementsWaiting for
This rule is failing.
🔴 🔎 ReviewsWaiting for
This rule is failing.
Show 4 satisfied protections🟢 🤖 Continuous Integration
🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 📕 PR description
🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
b7cef70 to
5d9cffb
Compare
Revision history
|
There was a problem hiding this comment.
Pull request overview
Updates the Bazel scopes documentation to compute merge-queue scopes from impacted Bazel targets (via bazel-diff) instead of inferring scopes from changed files/packages, aligning the “dependency-graph-based scopes” promise with how affected-graph detection is described elsewhere in the docs.
Changes:
- Replaces
bazel query buildfiles(set(<changed files>))guidance with abazel-diff-based impacted-targets workflow. - Adds an explanation of why changed-file/package queries under-report impact and can produce misleading scope lists.
- Updates GitHub Actions, Buildkite, and CLI recipes and adds guidance on performance/cost and “graph-invisible” changes.
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
All three CI recipes on the Bazel scopes page ran the same detection:
bazel query --output=package "buildfiles(set(<changed files>))"
`buildfiles()` returns the BUILD and .bzl files needed to *load* the
packages the changed files live in. It walks the graph away from the
dependents, so a change to a shared library tags that library and
nothing downstream — while the Nx page next door uses
`nx show projects --affected`, which does include dependents. Two
sibling pages promised the same guarantee and only one delivered it.
That is a correctness bug rather than imprecision. Scopes are what makes
the queue serialize pull requests that touch the same area; an
under-reported list lets it batch and parallelize pull requests that
genuinely conflict, and lets scope-gated CI jobs skip work the change
really did affect.
The page now teaches bazel-diff (github.com/Tinder/bazel-diff), which
hashes every target over its content, rule attributes and transitive
deps at two revisions and reports the targets whose hash changed.
Verified by running both detections against two Bazel workspaces:
- a four-package fixture (//app/server -> //lib/util -> //lib/core,
every BUILD loading a macro from //tools), across six scenarios;
- Tinder/bazel-diff itself at HEAD~10..HEAD, on Bazel 8.5.1.
What the runs showed, beyond the known missing dependents:
- `buildfiles()` names the wrong side of the graph. `tools` — the
package that *defines* the macro — appears for every change, while
the packages that consume it never do.
- On a real workspace it emits junk. On Tinder/bazel-diff it returned
~60 external-repository pseudo-packages (`@rules_kotlin//kotlin`,
`@bazel_tools//tools/build_defs/repo`, ...) alongside 7 real ones,
because it also returns the .bzl files of every ruleset loaded.
Those get uploaded verbatim as scope names. It also missed //proto,
//release and //tools/go/sample, which bazel-diff found.
No product work was needed: `source: manual`, `scopes-git-refs`,
`scopes-upload` and `mergify ci scopes-send` already carry this.
The page keeps no "quick but approximate" build-graph option, because
there isn't one. `rdeps(//..., set(<changed files>))` was measured too:
it returns *nothing at all* for a pull request that only edits BUILD or
.bzl files, since those are not targets and `rdeps` of an empty set is
empty. Zero scopes reads as "impacts nothing", which is worse than the
status quo. Readers who do not want a JVM and a second checkout are
pointed at file-pattern scopes instead — coarse, but honest about it.
The recipe also documents the traps the runs surfaced: `--targetType
Rule` throws unless `generate-hashes` ran with `--includeTargetType`;
bazel writes MODULE.bazel.lock so the mid-recipe checkout needs
`--force`; `fetch-depth: 0` is now mandatory because the base revision
must exist locally; and hashing both revisions in one directory keeps
the Bazel server warm (15.4s cold then 4.5s warm on the real repo).
This builds on #12468, which made these snippets actually run: it fixed
the ref quoting, added fetch-depth: 0 and stopped the query failing
silently into an empty scope list. That work was about the plumbing
around `bazel query`; this one changes the question the query asks, so
the quoting, exit-3 and empty-diff handling it added go away with the
`git diff | bazel query` pipeline they guarded. The fetch-depth: 0 it
added stays, and is now load-bearing for a different reason: bazel-diff
checks the base revision out rather than diffing against it.
The GitHub Actions and Buildkite wirings around the shell are unchanged
in shape from the existing page and were not executed on a real runner.
MRGFY-8808
Change-Id: I432e27032980e1106d41e1380233177bb3c0560e
5d9cffb to
a56d493
Compare
|
Force-pushed — The review asked for a supply-chain note on the There is also a new Verifying the download section: recomputing the digest on a version bump, Everything in it was executed, not written from memory — the digest matches the real v42.0.0 artifact, |
All three CI recipes on the Bazel scopes page ran the same detection:
buildfiles()returns the BUILD and .bzl files needed to load thepackages the changed files live in. It walks the graph away from the
dependents, so a change to a shared library tags that library and
nothing downstream — while the Nx page next door uses
nx show projects --affected, which does include dependents. Twosibling pages promised the same guarantee and only one delivered it.
That is a correctness bug rather than imprecision. Scopes are what makes
the queue serialize pull requests that touch the same area; an
under-reported list lets it batch and parallelize pull requests that
genuinely conflict, and lets scope-gated CI jobs skip work the change
really did affect.
The page now teaches bazel-diff (github.com/Tinder/bazel-diff), which
hashes every target over its content, rule attributes and transitive
deps at two revisions and reports the targets whose hash changed.
Verified by running both detections against two Bazel workspaces:
every BUILD loading a macro from //tools), across six scenarios;
What the runs showed, beyond the known missing dependents:
buildfiles()names the wrong side of the graph.tools— thepackage that defines the macro — appears for every change, while
the packages that consume it never do.
~60 external-repository pseudo-packages (
@rules_kotlin//kotlin,@bazel_tools//tools/build_defs/repo, ...) alongside 7 real ones,because it also returns the .bzl files of every ruleset loaded.
Those get uploaded verbatim as scope names. It also missed //proto,
//release and //tools/go/sample, which bazel-diff found.
No product work was needed:
source: manual,scopes-git-refs,scopes-uploadandmergify ci scopes-sendalready carry this.The page keeps no "quick but approximate" build-graph option, because
there isn't one.
rdeps(//..., set(<changed files>))was measured too:it returns nothing at all for a pull request that only edits BUILD or
.bzl files, since those are not targets and
rdepsof an empty set isempty. Zero scopes reads as "impacts nothing", which is worse than the
status quo. Readers who do not want a JVM and a second checkout are
pointed at file-pattern scopes instead — coarse, but honest about it.
The recipe also documents the traps the runs surfaced:
--targetType Rulethrows unlessgenerate-hashesran with--includeTargetType;bazel writes MODULE.bazel.lock so the mid-recipe checkout needs
--force;fetch-depth: 0is now mandatory because the base revisionmust exist locally; and hashing both revisions in one directory keeps
the Bazel server warm (15.4s cold then 4.5s warm on the real repo).
This builds on #12468, which made these snippets actually run: it fixed
the ref quoting, added fetch-depth: 0 and stopped the query failing
silently into an empty scope list. That work was about the plumbing
around
bazel query; this one changes the question the query asks, sothe quoting, exit-3 and empty-diff handling it added go away with the
git diff | bazel querypipeline they guarded. The fetch-depth: 0 itadded stays, and is now load-bearing for a different reason: bazel-diff
checks the base revision out rather than diffing against it.
The GitHub Actions and Buildkite wirings around the shell are unchanged
in shape from the existing page and were not executed on a real runner.
MRGFY-8808