Skip to content

Commit e031d63

Browse files
committed
Fix query-failure fallback for bzlmod-only workspaces
The per-repo rdeps query-failure fallback filtered out every `@@` label on the assumption that the hash set was mostly workspace-local `//...` targets. That assumption does not hold on bzlmod-only workspaces where `generate-hashes` emits almost entirely `@@canonical` labels plus a small number of `//external:<apparent>` bzlmod-synthetic bridges, with zero workspace-local targets. On such workspaces, a single failed `rdeps(//..., @@<repo>//...)` query (for example, one caused by an unrelated loading error elsewhere in the dep graph) left the filter with only the `//external:*` bridges, which the downstream `excludeExternalTargets=true` default (#334) then strips entirely — producing silently-empty impacted output on what should be a "conservatively rebuild everything" signal. Mirror the outer catch at line 401 and mark every hashed target as impacted on per-repo query failure. Downstream filters (`targetTypes`, `excludeExternalTargets`) remain the caller's responsibility, which is the whole point of surfacing the full set upstream. Add a regression test in CalculateImpactedTargetsInteractorModuleQueryTest that constructs a bzlmod-shaped hash set (two `@@abseil-cpp+` targets + one `//external:*` bridge, no workspace targets), stubs `queryService.query` to throw, and asserts all three labels appear in the output.
1 parent a61697d commit e031d63

2 files changed

Lines changed: 50 additions & 1 deletion

File tree

cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,18 @@ class CalculateImpactedTargetsInteractor : KoinComponent {
374374
} catch (e: Exception) {
375375
logger.w { "Failed to query rdeps for @@$repoName: ${e.message}" }
376376
logger.w { "Conservatively marking all targets as impacted for this module" }
377-
impactedTargets.addAll(allTargets.keys.filter { !it.startsWith("@@") })
377+
// Mirror the outer catch: mark every hashed target as impacted.
378+
//
379+
// Previously this filtered out `@@...` labels on the assumption that
380+
// the hash set was mostly workspace-local `//...` targets. That
381+
// assumption breaks on bzlmod-only workspaces where `generate-hashes`
382+
// emits almost entirely `@@` canonical-repo labels and
383+
// `//external:<apparent>` bridges, with zero `//...` workspace
384+
// targets. The filter then left nothing, which — after the downstream
385+
// `excludeExternalTargets=true` default strip of `//external:*` —
386+
// produced silently-empty impacted output on query failure instead of
387+
// the intended "conservatively rebuild" signal.
388+
impactedTargets.addAll(allTargets.keys)
378389
}
379390
}
380391

cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorModuleQueryTest.kt

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,44 @@ class CalculateImpactedTargetsInteractorModuleQueryTest : KoinTest {
312312
verify(queryService).query(eq("rdeps(//..., @@abseil-cpp+//...)"), eq(false))
313313
} }
314314

315+
@Test
316+
fun queryFailureMarksAllHashedTargetsImpactedIncludingExternal() { runBlocking {
317+
// On bzlmod-only workspaces the hash set is almost entirely `@@canonical`
318+
// and `//external:<apparent>` bridges with zero workspace-local `//...`
319+
// targets. If the per-repo rdeps query fails, the conservative fallback
320+
// must still surface a non-empty impacted set — otherwise the downstream
321+
// `excludeExternalTargets=true` default strips the only remaining labels
322+
// and the CLI reports zero impacted targets on what is really a
323+
// "rebuild everything" condition.
324+
val hashes = mapOf(
325+
"@@abseil-cpp+//absl:strings" to TargetHash("Rule", "a", "a"),
326+
"@@abseil-cpp+//absl:base" to TargetHash("Rule", "b", "b"),
327+
"//external:com_google_absl" to TargetHash("Rule", "c", "c"))
328+
329+
whenever(queryService.discoverRepoMapping())
330+
.thenReturn(mapOf("abseil-cpp" to "abseil-cpp+"))
331+
whenever(queryService.query(any(), any()))
332+
.thenThrow(RuntimeException("simulated bazel query failure"))
333+
334+
val writer = StringWriter()
335+
CalculateImpactedTargetsInteractor().execute(
336+
from = hashes,
337+
to = hashes,
338+
outputWriter = writer,
339+
targetTypes = null,
340+
fromModuleGraphJson = graph("abseil-cpp", "20240116.2"),
341+
toModuleGraphJson = graph("abseil-cpp", "20240722.0"))
342+
343+
verify(queryService).query(eq("rdeps(//..., @@abseil-cpp+//...)"), eq(false))
344+
// Every hashed label must appear in the fallback output, including `@@`
345+
// and `//external:*`. Downstream filters (targetTypes, excludeExternalTargets)
346+
// are the caller's responsibility.
347+
assertThat(outputLines(writer)).containsExactlyInAnyOrder(
348+
"@@abseil-cpp+//absl:strings",
349+
"@@abseil-cpp+//absl:base",
350+
"//external:com_google_absl")
351+
} }
352+
315353
@Test
316354
fun executeWithDistancesRunsModuleQueryPath() { runBlocking {
317355
// Covers the parallel module-query call site in `executeWithDistances`.

0 commit comments

Comments
 (0)