Bugfix: Scope macro digest cache to each hash invocation - #446
Bugfix: Scope macro digest cache to each hash invocation#446dgollahon-plaid wants to merge 2 commits into
Conversation
The query service reuses RuleHasher across base and head checkouts. Its path-only .bzl cache could therefore reuse a base digest at head and hide macro-only changes from impacted targets. Make the cache invocation-local while retaining within-run parallel deduplication. Add unit coverage for cache lifetime and a two-commit query-service regression.
|
This was discovered by A/B testing old/new Thanks for a valuable tool! |
tinder-maxwellelliott
left a comment
There was a problem hiding this comment.
This is great, thanks!
|
@tinder-maxwellelliott Thanks for the review! Will you merge this or re-approve CI -- looks like it's waiting for an approval after I hit update branch? Github didn't show me a merge button I assume because the branch was out of date or I just don't have permissions even with an approval. |
The query service can omit targets affected by a macro-only
.bzlchange. It hashes the base and head revisions in one process, whileRuleHasheris a singleton. PR #436 added a.bzldigest cache keyed only by workspace-relative path, so hashing the head revision could reuse the base revision’s digest and produce identical target hashes.Move revision-derived
.bzlcache state into aHashInvocationContextcreated for eachBuildGraphHasherinvocation. The context is shared across parallel target hashing, which preserves same-run digest deduplication, but it cannot leak across base and head checkouts. This also avoids an unbounded process-lifetime cache and remains safe if hash generation becomes concurrent.The regression was introduced by
700780ain #436. The same two-revision query-service reproduction:07b6d54, where//:generatedis impacted;700780a, where the service returns no impacted targets;The regression coverage also verifies that
.bzldigests are recalculated between hash invocations and still deduplicated within one parallel invocation.