Skip to content

Fix #197: auto-expand fineGrainedHashExternalRepos through bzlmod wrappers#343

Open
tinder-maxwellelliott wants to merge 2 commits into
masterfrom
claude/reproducer-issue-197
Open

Fix #197: auto-expand fineGrainedHashExternalRepos through bzlmod wrappers#343
tinder-maxwellelliott wants to merge 2 commits into
masterfrom
claude/reproducer-issue-197

Conversation

@tinder-maxwellelliott
Copy link
Copy Markdown
Collaborator

@tinder-maxwellelliott tinder-maxwellelliott commented May 12, 2026

Summary

  • Fixes #197 — specifically the wrapper shape @Ahajha called out in his comment: a target inside one external repo is re-wrapped by another external repo, and the main repo consumes only the wrapping repo. Before this fix, changing the inner repo's source file did not invalidate the main consumer unless every wrapping repo was manually enumerated in --fineGrainedHashExternalRepos.
  • The fix walks bazel mod graph --output=json once at startup, builds the apparent-name dep graph, and auto-adds every bzlmod module that transitively depends on a user-listed repo to the fine-grained set. Listing only @inner_repo is now enough; @middle_repo is added automatically so its alias rule is queried for real and its actual attribute carries the dep chain down to the leaf source file.
  • The expansion is a no-op when the user supplies no repos, when bzlmod is disabled, or when no wrapper depends on a listed repo, so existing flows that don't involve wrapping repos behave exactly as before.
  • The reproducer test added in the previous revision of this PR is now a non-@Ignore'd regression test under the new wrapped_external_repo fixture.

Mechanism

  • New entry point: expandFineGrainedHashExternalReposWithBzlmodDependents in cli/src/main/kotlin/com/bazel_diff/di/Modules.kt.
  • New parser helpers on ModuleGraphParser (parseModuleGraphEdges, findTransitiveDependents) extract the dep edges from the JSON form of bazel mod graph and run reverse-BFS from the user-listed apparent names, excluding the root module (which is always queried via //...:all-targets anyway).

Manual verification

With the fixture from this PR:

  • --fineGrainedHashExternalRepos=@inner_repo after editing inner_repo/data.txt:
    • Before: impacted = {@inner_repo//:all_files, @inner_repo//:data.txt}//:consumer is not impacted (bug).
    • After: impacted = {//:consumer, //:consumer.out, @inner_repo//:all_files, @inner_repo//:data.txt, @middle_repo//:wrapped}//:consumer is impacted (fixed).
  • --fineGrainedHashExternalRepos=@inner_repo,@middle_repo still works (the manual-enumeration workaround is unaffected).

Test plan

  • bazel test //cli:ModuleGraphParserTest — passes (5 new tests for the edge + dependents helpers).
  • bazel test //cli:E2ETest --test_filter=".*WrappedExternalRepo.*" — the regression test for No impacted targets when a file in an external repository changes #197 now passes without @Ignore.
  • Full E2E suite locally on Bazel 9.1.0: same 4 pre-existing failures as the PR base (testFineGrainedHashExternalRepo, testBzlmodTransitiveDepsCquery, testUseCqueryWithExternalDependencyChange, testUseCqueryWithAndroidCodeChange — all fail on the unmodified base too due to a Found duplicate artifact versions com.google.guava:guava issue in the existing maven fixture zips; CI runs on Linux and passes them).
  • Manual run with the locally built CLI confirms the fix (see "Manual verification" above).

🤖 Generated with Claude Code

@tinder-maxwellelliott tinder-maxwellelliott force-pushed the claude/reproducer-issue-197 branch from 311be6f to 6b91f03 Compare May 13, 2026 19:50
tinder-maxwellelliott and others added 2 commits May 15, 2026 18:15
…tected

The simple case (a main-repo target consuming a file from a single
external repo) works on current bazel-diff. The unfixed shape is the one
@Ahajha called out: a target inside one external repo is re-wrapped by
ANOTHER external repo, and the main repo consumes only the wrapping
repo. When the inner repo's source file changes, the main-repo consumer
should be reported as impacted -- today it is not, unless the user
manually enumerates every wrapping external repo in
--fineGrainedHashExternalRepos.

The new `wrapped_external_repo` fixture wires this up with bzlmod:
  inner_repo  -> filegroup wrapping data.txt
  middle_repo -> alias re-exporting @inner_repo//:all_files
  //:consumer -> genrule consuming @middle_repo//:wrapped

Confirmed by hand against the built CLI:
  --fineGrainedHashExternalRepos=@inner_repo
    -> impacted: @inner_repo//:all_files, @inner_repo//:data.txt
    -> //:consumer NOT impacted (bug)
  --fineGrainedHashExternalRepos=@inner_repo,@middle_repo
    -> impacted includes //:consumer (workaround proves the chain works
       when every wrapper is enumerated)

The reproducer test asserts that listing only the source-of-truth repo
is enough. It is @ignore'd until propagation through alias/wrapper
external repos works.

Refs #197

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Without this, a main-repo target consuming `@middle_repo//:wrapped`
(itself an alias for `@inner_repo//:all_files`) is not reported as
impacted when `inner_repo/data.txt` changes, unless the user manually
enumerates every wrapping external repo in `--fineGrainedHashExternalRepos`.
The dep chain stops at the unhashed `//external:middle_repo` blob because
`transformRuleInput` collapses any external label whose repo isn't in
the fine-grained set.

The fix walks `bazel mod graph --output=json` once at startup, builds
the apparent-name dependency graph, and adds every bzlmod module that
transitively depends on a user-listed repo to the fine-grained set.
For the reproducer fixture, listing only `@inner_repo` now auto-includes
`@middle_repo`, so its alias target is queried as a real rule and the
chain propagates down to the leaf source file.

The expansion is a no-op when the user supplies no repos, when bzlmod
is disabled, or when no wrapper depends on a listed repo, so existing
fine-grained flows that don't involve wrapping repos are unchanged.

Flips the `@Ignore`'d reproducer in #343 to a passing regression test
and adds `ModuleGraphParser` unit coverage for the new edge / dependents
helpers.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@tinder-maxwellelliott tinder-maxwellelliott force-pushed the claude/reproducer-issue-197 branch from 6b91f03 to c826840 Compare May 16, 2026 02:13
@tinder-maxwellelliott tinder-maxwellelliott changed the title Reproducer test for #197 wrapped external repo file change not detected Fix #197: auto-expand fineGrainedHashExternalRepos through bzlmod wrappers May 16, 2026
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.

No impacted targets when a file in an external repository changes

1 participant