Reproducer test for #359: cquery rule inputs drop configuration_checksum#361
Merged
Merged
Conversation
…hecksum Under --useCquery, BazelRule.ruleInputList() reads from configured_rule_input but maps each entry to `.label` only, discarding `.configurationChecksum`. Two configured graphs that share dep labels but differ in per-edge configuration (cfg="exec" transitions, --platforms swap, --config=..., --define toggling a select() in a dep) collapse to identical ruleInputList() output, so RuleHasher cannot tell them apart and bazel-diff under cquery reports an empty impacted set. Three tests pin this down at the lowest level: * testCqueryRuleInputListDistinguishesConfigurationChecksum_reproducerForIssue359 -- @ignore'd reproducer asserting the desired post-fix invariant (inputsA != inputsB). Verified to fail today with "expected to not be equal to: <[//:dep]>". * testCqueryRuleInputListCollapsesConfigurationChecksum_currentBehaviourBuggy_issue359 -- pins the current (buggy) behaviour: both lists collapse to [//:dep]. * testBazelRuleDigestIgnoresConfigurationChecksum_currentBehaviourBuggy_issue359 -- pins that BazelRule.digest() does not touch configured_rule_input at all. The two `_currentBehaviourBuggy_*` tests keep CI green and give a regression signal: if they ever start failing, the `@Ignore` on the reproducer should be flipped off as part of the fix. Refs #359 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
tinder-maxwellelliott
added a commit
that referenced
this pull request
May 22, 2026
Under --useCquery, BazelRule.ruleInputList() previously folded each
ConfiguredRuleInput to its bare label via `.map { it.label }`, dropping
`configurationChecksum`. Two configured graphs that shared dep labels
but differed in per-edge configuration -- a `cfg = "exec"` transition
flip, a --platforms swap, a --config=... change, or a `--define`
toggling a select() inside a dep -- collapsed to identical
ruleInputList() output, so RuleHasher's transitive walk produced
identical digests and bazel-diff returned an empty impacted set under
cquery mode.
This change folds each dep edge's `configurationChecksum` into the
rule-input string using `|` -- a character that is invalid in a Bazel
label, so the encoding can never collide with a real input and survives
the existing `List<String>` return type. The full encoded string is
what gets mixed into RuleHasher's transitive hash, so two configured
graphs differing only by per-edge configuration now produce distinct
parent digests. RuleHasher splits on the separator to recover the bare
label for `allRulesMap` / `sourceDigests` lookups and dep tracking, so
the user-visible deps in the generated JSON stay clean.
The empty-checksum fallback (a known Bazel quirk where cquery's
`transitions=lite` output sometimes omits the checksum for an edge)
returns the bare label unchanged, preserving pre-#359 behaviour for
that path.
This is a hash-format change: hashes generated by this version are not
comparable to hashes from earlier versions under --useCquery, which is
the intended behaviour -- the old hashes were missing the
configuration signal.
Tests:
* Unignore the PR #361 reproducer
(`testCqueryRuleInputListDistinguishesConfigurationChecksum_issue359`)
and drop the two `_currentBehaviourBuggy_*` companions.
* Add `testCqueryRuleInputListEncodesLabelAndChecksum_issue359` to pin
the on-the-wire encoding.
* Add `testCqueryRuleInputListEmptyChecksumFallsBackToBareLabel_issue359`
to cover the Bazel-quirk fallback.
* Add `testNonCqueryRuleInputListIgnoresConfiguredRuleInput_issue359`
to confirm the encoding does not leak into the non-cquery path.
* Add `testDecodeConfiguredRuleInputLabelRoundTrip_issue359` to pin the
round-trip behaviour RuleHasher relies on.
This is the minimal symptom-fixing slice of #359 -- the parent target's
hash now reflects per-edge configuration changes, so the empty-impacted
set under cquery is gone. Full target-determinator parity (per-(label,
configuration) digest entries, `bazel config --output=json`
enumeration, empty-checksum inheritance from the depending target) is
intentionally out of scope here; those would change the on-disk JSON
schema and warrant a separate design discussion on the issue.
Refs #359, #361
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #359 (reproducer) — this PR is tests only, no behaviour change.
BazelRuleTestpinning down the cquery configuration-invariance bug described in Configuration-aware hashing under --useCquery (parity with target-determinator) #359 at the lowest possible level — directly onBazelRule.ruleInputList()/BazelRule.digest(), no fixture workspace or live Bazel call needed.@Ignored testtestCqueryRuleInputListDistinguishesConfigurationChecksum_reproducerForIssue359documents the desired post-fix invariant. I verified locally that flipping the@Ignoreoff makes it fail withexpected to not be equal to: <[//:dep]>— exactly the collapse the issue describes.*_currentBehaviourBuggy_issue359) lock in the current behaviour so CI stays green AND so we get a regression signal: if either ever starts failing, the@Ignoreshould be flipped off as part of the fix.Why this is the right reproducer shape
The issue identifies
cli/src/main/kotlin/com/bazel_diff/hash/RuleHasher.ktandcli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.ktas the loss site —configuredRuleInputList.map { it.label }discardsconfiguration_checksum. A unit test that builds twoBuild.Ruleprotos differing only inConfiguredRuleInput.configuration_checksumand asserts thatruleInputList(useCquery=true)distinguishes them isolates the bug exactly at that code line. An E2E test using a real Bazel workspace +--platformsswap would also work but is far more fragile (platforms infra, Bazel-version assumptions) and isn't necessary to make the bug visible.The companion tests also pin down a useful invariant for the fix: today
BazelRule.digest(...)doesn't readconfigured_rule_inputat all (seeBazelRule.kt:14-26) — so a fix that just mixesconfiguration_checksuminto the digest would also flip the third test.Test plan
bazel test //cli:BazelRuleTestpasses with the reproducer @ignore'd (5 tests, 1 ignored)@Ignoreoff locally →AssertionFailedError: expected to not be equal to: <[//:dep]>🤖 Generated with Claude Code