Fix #365: attribute .bzl seed per-package instead of workspace-wide#367
Merged
Merged
Conversation
The #259/#227 fix (createSeedForBzlFiles) rolled the union of every main-repo `.bzl` digest into a single workspace-wide seed that was mixed into every target's hash. As its own doc comment acknowledged ("a single `.bzl` edit re-hashes every target"), adding or editing any `.bzl` -- even in an otherwise-empty BUILD that nothing else loads -- flipped the seed and marked every target in the repo impacted, not just the targets that load it. Replace the single seed with `createPackageBzlSeeds`, which groups each BUILD file's loaded `.bzl` digests by the package that loads them (via the `subinclude` proto field on each package's BUILD SourceFile target). Each rule and source file then mixes in only its own package's seed, looked up by `labelToPackage(name)`. Because every target looks up its own package (never the caller's), this stays consistent under RuleHasher's memoized, depth-first recursion. Effects: - A macro added/edited in package X re-hashes only targets in packages that `load()` it, fixing the #365 over-triggering. - #259/#227 stays fixed: a `.bzl` edit still re-hashes every target in the packages that load it (e.g. macro_invalidation's `//:logo_miniature`). - Workspaces that load no tracked `.bzl` are byte-for-byte stable: the per-package map is empty, so nothing is mixed in (golden-hash unit tests unchanged). Flips the @ignore'd post-fix invariant test on and removes the companion test that pinned the buggy behaviour. Co-Authored-By: Claude Opus 4.8 <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
Fixes #365 — after the #259/#227 fix, adding (or editing) any
.bzlmacro andload()ing it, even in an otherwise-empty BUILD that nothing else loads, marked every target in the repository as impacted rather than just the targets that load the macro.Root cause:
BuildGraphHasher.createSeedForBzlFilesrolled the union of every main-repo.bzldigest into a single workspace-wide seed that was then mixed into every target's hash. Its own doc comment acknowledged the tradeoff — "a single.bzledit re-hashes every target". So introducing a brand-new, unrelated.bzlflipped the seed and changed the hash of completely unrelated targets.Fix: Replace the single seed with
createPackageBzlSeeds, which attributes each BUILD file's loaded.bzldigests to the package that loads them — keyed off thesubincludeproto field carried on each package's BUILDSourceFiletarget. Each rule and source file then mixes in only its own package's seed (looked up vialabelToPackage(name)). Because every target always looks up its own package and never the caller's, this stays consistent underRuleHasher's memoized, depth-first recursion.Behaviour
Xre-hashes only targets in packages thatload()it. Unrelated packages (e.g.//pkg:a,//pkg:bin thebzl_seed_overtriggerfixture) are untouched..bzledit still re-hashes every target in the packages that load it — e.g.macro_invalidation's//:logo_miniaturewhenminiature.bzlchanges..bzl-free workspaces: the per-package map is empty when no tracked.bzlis loaded, so nothing is mixed in. The golden-hash unit tests (BuildGraphHasherTest,TargetHashTest,SourceFileHasherTest) are unchanged.Tests
@Ignore'd post-fix invarianttestAddingUnrelatedMacroDoesNotImpactExistingTargets_reproducerForIssue365on — it now passes.testAddingUnrelatedMacroImpactsExistingTargets_currentBehaviourBuggy_issue365that pinned the now-fixed over-triggering behaviour.Test plan
BuildGraphHasherTest/TargetHashTest/SourceFileHasherTestpass (golden hashes byte-identical)testAddingUnrelatedMacroDoesNotImpactExistingTargets_reproducerForIssue365passestestMacroBzlChangeImpactsCallers_regressionForIssue259And227still passesRefs #365. Builds on the reproducer from #366.
🤖 Generated with Claude Code