Skip to content

Commit 876b186

Browse files
committed
Fix query-failure fallback for bzlmod-only workspaces
The unioned 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 query failure (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 (Tinder#334) then strips entirely — producing silently-empty impacted output on what should be a "conservatively rebuild everything" signal. Mark every hashed target as impacted on 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 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 f40009d commit 876b186

2 files changed

Lines changed: 47 additions & 2 deletions

File tree

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,8 +373,16 @@ class CalculateImpactedTargetsInteractor : KoinComponent {
373373
logger.i { "Found ${rdepLabels.size} workspace targets depending on changed modules" }
374374
impactedTargets.addAll(rdepLabels)
375375
} catch (e: Exception) {
376-
logger.e(e) { "Unioned rdeps query failed - conservatively marking all workspace targets impacted" }
377-
impactedTargets.addAll(allTargets.keys.filter { !it.startsWith("@@") })
376+
logger.e(e) { "Unioned rdeps query failed - conservatively marking all targets impacted" }
377+
// Mark every hashed target as impacted. Filtering out `@@` labels would
378+
// produce silently-empty output on bzlmod-only workspaces where the hash
379+
// set is almost entirely `@@canonical` + `//external:<apparent>` bridges
380+
// with zero workspace-local `//...` targets; the downstream
381+
// `excludeExternalTargets=true` default (#334) would then strip the
382+
// remaining `//external:*`, leaving nothing. Downstream filters
383+
// (`targetTypes`, `excludeExternalTargets`) remain the caller's
384+
// responsibility.
385+
impactedTargets.addAll(allTargets.keys)
378386
}
379387
}
380388

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,43 @@ class CalculateImpactedTargetsInteractorModuleQueryTest : KoinTest {
328328
assertThat(queryCaptor.firstValue).contains("@@abseil-cpp+//...")
329329
} }
330330

331+
@Test
332+
fun queryFailureMarksAllHashedTargetsImpactedIncludingExternal() { runBlocking {
333+
// On bzlmod-only workspaces the hash set is almost entirely `@@canonical`
334+
// and `//external:<apparent>` bridges with zero workspace-local `//...`
335+
// targets. If the unioned rdeps query fails, the conservative fallback
336+
// must still surface a non-empty impacted set — otherwise the downstream
337+
// `excludeExternalTargets=true` default strips the only remaining labels
338+
// and the CLI reports zero impacted targets on what is really a
339+
// "rebuild everything" condition.
340+
val hashes = mapOf(
341+
"@@abseil-cpp+//absl:strings" to TargetHash("Rule", "a", "a"),
342+
"@@abseil-cpp+//absl:base" to TargetHash("Rule", "b", "b"),
343+
"//external:com_google_absl" to TargetHash("Rule", "c", "c"))
344+
345+
whenever(queryService.discoverRepoMapping())
346+
.thenReturn(mapOf("abseil-cpp" to "abseil-cpp+"))
347+
whenever(queryService.query(any(), any()))
348+
.thenThrow(RuntimeException("simulated bazel query failure"))
349+
350+
val writer = StringWriter()
351+
CalculateImpactedTargetsInteractor().execute(
352+
from = hashes,
353+
to = hashes,
354+
outputWriter = writer,
355+
targetTypes = null,
356+
fromModuleGraphJson = graph("abseil-cpp", "20240116.2"),
357+
toModuleGraphJson = graph("abseil-cpp", "20240722.0"))
358+
359+
// Every hashed label must appear in the fallback output, including `@@`
360+
// and `//external:*`. Downstream filters (targetTypes, excludeExternalTargets)
361+
// are the caller's responsibility.
362+
assertThat(outputLines(writer)).containsExactlyInAnyOrder(
363+
"@@abseil-cpp+//absl:strings",
364+
"@@abseil-cpp+//absl:base",
365+
"//external:com_google_absl")
366+
} }
367+
331368
@Test
332369
fun executeWithDistancesRunsModuleQueryPath() { runBlocking {
333370
// Covers the parallel module-query call site in `executeWithDistances`.

0 commit comments

Comments
 (0)