Skip to content

Bugfix: Scope macro digest cache to each hash invocation - #446

Open
dgollahon-plaid wants to merge 1 commit into
Tinder:masterfrom
dgollahon-plaid:dgollahon-fix-query-service-bzl-cache
Open

Bugfix: Scope macro digest cache to each hash invocation#446
dgollahon-plaid wants to merge 1 commit into
Tinder:masterfrom
dgollahon-plaid:dgollahon-fix-query-service-bzl-cache

Conversation

@dgollahon-plaid

Copy link
Copy Markdown

The query service can omit targets affected by a macro-only .bzl change. It hashes the base and head revisions in one process, while RuleHasher is a singleton. PR #436 added a .bzl digest 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 .bzl cache state into a HashInvocationContext created for each BuildGraphHasher invocation. 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 700780a in #436. The same two-revision query-service reproduction:

  • passes at its parent, 07b6d54, where //:generated is impacted;
  • fails at 700780a, where the service returns no impacted targets;
  • passes with this change.

The regression coverage also verifies that .bzl digests are recalculated between hash invocations and still deduplicated within one parallel invocation.

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.
@dgollahon-plaid

Copy link
Copy Markdown
Author

This was discovered by A/B testing old/new bazel-diff on Plaid's codebase for a representative set of changes. The new version of bazel-diff omitted changes that should have been triggered. I had Codex produce a regression test and a fix. I am not personally familiar with this codebase so happy to implement any feedback or alter approach. You can also view this PR as a bug report instead if you prefer to implement something different.

Thanks for a valuable tool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant