Fix #326: filter //external:* labels from impacted-targets output on bzlmod (#334)

tinder-maxwellelliott · claude · web-flow · commit 2f6b3414248b · 2026-04-27T09:44:25.000-04:00
Synthetic //external:&lt;apparent_name&gt; targets created by `bazel mod show_repo`
are needed during generate-hashes (so dep changes are detected) but are not
buildable in bzlmod-only mode (Bazel 8.6.0+ with WORKSPACE disabled), causing
downstream `bazel build` of the impacted-targets file to fail with
"no such package 'external'".

Adds --excludeExternalTargets to get-impacted-targets (negatable, auto-defaults
to true when bzlmod is detected), filters those labels at the output stage,
and adds unit + e2e coverage.

Co-authored-by: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/cli/src/main/kotlin/com/bazel_diff/cli/GetImpactedTargetsCommand.kt b/cli/src/main/kotlin/com/bazel_diff/cli/GetImpactedTargetsCommand.kt
@@ -105,6 +105,22 @@ class GetImpactedTargetsCommand : Callable<Int> {
       scope = CommandLine.ScopeType.LOCAL)
   var noBazelrc = false
 
+  @CommandLine.Option(
+      names = ["--excludeExternalTargets"],
+      negatable = true,
+      description =
+          [
+              "If true, drop labels starting with '//external:' from the impacted-targets output. " +
+                  "These synthetic labels are produced for bzlmod-managed external repos so " +
+                  "generate-hashes can detect dep changes, but they are not buildable in " +
+                  "bzlmod-only mode (Bazel 8.6.0+ with --enable_workspace=false) and will fail " +
+                  "downstream `bazel build`. See https://github.com/Tinder/bazel-diff/issues/326. " +
+                  "When unset, defaults to true if Bzlmod is detected (via `bazel mod graph`), " +
+                  "false otherwise."],
+      scope = CommandLine.ScopeType.LOCAL,
+      defaultValue = CommandLine.Parameters.NULL_VALUE)
+  var excludeExternalTargets: Boolean? = null
+
   @CommandLine.Spec lateinit var spec: CommandLine.Model.CommandSpec
 
   override fun call(): Int {
@@ -141,6 +157,15 @@ class GetImpactedTargetsCommand : Callable<Int> {
       val fromData = deserialiser.executeTargetHashWithMetadata(startingHashesJSONPath)
       val toData = deserialiser.executeTargetHashWithMetadata(finalHashesJSONPath)
 
+      // If the user did not pass --[no-]excludeExternalTargets, default to true when bzlmod is
+      // enabled (synthetic //external:* labels are not buildable in bzlmod-only mode — #326),
+      // otherwise false to preserve WORKSPACE-mode behavior.
+      val resolvedExcludeExternalTargets =
+          excludeExternalTargets
+              ?: org.koin.java.KoinJavaComponent.get<com.bazel_diff.bazel.BazelModService>(
+                      com.bazel_diff.bazel.BazelModService::class.java)
+                  .isBzlmodEnabled
+
       val outputWriter =
           BufferedWriter(
               when (val path = outputPath) {
@@ -159,7 +184,8 @@ class GetImpactedTargetsCommand : Callable<Int> {
                   outputWriter,
                   targetType,
                   fromData.moduleGraphJson,
-                  toData.moduleGraphJson)
+                  toData.moduleGraphJson,
+                  resolvedExcludeExternalTargets)
         } else {
           CalculateImpactedTargetsInteractor()
               .execute(
@@ -168,7 +194,8 @@ class GetImpactedTargetsCommand : Callable<Int> {
                   outputWriter,
                   targetType,
                   fromData.moduleGraphJson,
-                  toData.moduleGraphJson)
+                  toData.moduleGraphJson,
+                  resolvedExcludeExternalTargets)
         }
         CommandLine.ExitCode.OK
       } catch (e: IOException) {
diff --git a/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt b/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt
@@ -35,7 +35,8 @@ class CalculateImpactedTargetsInteractor : KoinComponent {
       outputWriter: Writer,
       targetTypes: Set<String>?,
       fromModuleGraphJson: String? = null,
-      toModuleGraphJson: String? = null
+      toModuleGraphJson: String? = null,
+      excludeExternalTargets: Boolean = false,
   ) {
     /** This call might be faster if end hashes is a sorted map */
     val typeFilter = TargetTypeFilter(targetTypes, to)
@@ -59,6 +60,7 @@ class CalculateImpactedTargetsInteractor : KoinComponent {
 
     impactedTargets
         .filter { typeFilter.accepts(it) }
+        .filter { !excludeExternalTargets || !it.startsWith("//external:") }
         .sortedWith(impactedTargetOrdering(to, from))
         .let { filtered ->
           outputWriter.use { writer -> filtered.forEach { writer.write("$it\n") } }
@@ -87,7 +89,8 @@ class CalculateImpactedTargetsInteractor : KoinComponent {
       outputWriter: Writer,
       targetTypes: Set<String>?,
       fromModuleGraphJson: String? = null,
-      toModuleGraphJson: String? = null
+      toModuleGraphJson: String? = null,
+      excludeExternalTargets: Boolean = false,
   ) {
     val typeFilter = TargetTypeFilter(targetTypes, to)
 
@@ -114,6 +117,7 @@ class CalculateImpactedTargetsInteractor : KoinComponent {
     val ordering = impactedTargetOrdering(to, from)
     impactedTargets
         .filterKeys { typeFilter.accepts(it) }
+        .filterKeys { !excludeExternalTargets || !it.startsWith("//external:") }
         .toSortedMap(ordering)
         .let { filtered ->
           outputWriter.use { writer ->
diff --git a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt
@@ -1210,6 +1210,96 @@ class E2ETest {
         .isEqualTo(false)
   }
 
+  @Test
+  fun testExcludeExternalTargetsFiltersBzlmodSyntheticLabels() {
+    // Validates the fix for https://github.com/Tinder/bazel-diff/issues/326.
+    //
+    // On Bazel 8.6.0+ with bzlmod, BazelClient queries `bazel mod show_repo` to produce synthetic
+    // //external:<apparent_name> targets so generate-hashes can detect bzlmod dep changes. Those
+    // labels are unbuildable in bzlmod-only mode (no //external package), so users hit
+    // "no such package 'external'" when feeding the impacted-targets file to `bazel build`.
+    //
+    // Expected behavior:
+    //   - get-impacted-targets defaults --excludeExternalTargets to TRUE when bzlmod is detected
+    //     => no //external:* lines in the output.
+    //   - --no-excludeExternalTargets opts back into the legacy behavior so the synthetic labels
+    //     reappear (proving they're really in the hash maps and the filter is what removes them).
+    val version = getBazelVersion()
+    org.junit.Assume.assumeNotNull(version)
+    val v = version!!
+    val comparator = compareBy<Triple<Int, Int, Int>> { it.first }.thenBy { it.second }.thenBy { it.third }
+    val hasModShowRepo = comparator.compare(v, Triple(8, 6, 0)) >= 0 && v != Triple(9, 0, 0)
+    org.junit.Assume.assumeTrue(
+        "Requires Bazel 8.6.0+ or 9.0.1+ (current: ${v.first}.${v.second}.${v.third})",
+        hasModShowRepo)
+
+    val projectA = extractFixtureProject("/fixture/bzlmod-show-repo-test-1.zip")
+    val projectB = extractFixtureProject("/fixture/bzlmod-show-repo-test-2.zip")
+
+    val bazelPath = "bazel"
+    val outputDir = temp.newFolder()
+    val from = File(outputDir, "starting_hashes.json")
+    val to = File(outputDir, "final_hashes.json")
+    val defaultOutput = File(outputDir, "impacted_default.txt")
+    val optOutOutput = File(outputDir, "impacted_optout.txt")
+
+    val cli = CommandLine(BazelDiff())
+
+    assertThat(
+        cli.execute(
+            "generate-hashes",
+            "-w", projectA.absolutePath,
+            "-b", bazelPath,
+            from.absolutePath))
+        .isEqualTo(0)
+    assertThat(
+        cli.execute(
+            "generate-hashes",
+            "-w", projectB.absolutePath,
+            "-b", bazelPath,
+            to.absolutePath))
+        .isEqualTo(0)
+
+    // Default invocation: bzlmod is detected, so --excludeExternalTargets auto-defaults to true.
+    assertThat(
+        cli.execute(
+            "get-impacted-targets",
+            "-w", projectB.absolutePath,
+            "-b", bazelPath,
+            "-sh", from.absolutePath,
+            "-fh", to.absolutePath,
+            "-o", defaultOutput.absolutePath))
+        .isEqualTo(0)
+
+    val defaultLines = defaultOutput.readLines().filter { it.isNotBlank() }
+    val leakedExternal = defaultLines.filter { it.startsWith("//external:") }
+    assertThat(leakedExternal.isEmpty())
+        .transform(
+            "default impacted-targets output should not contain //external:* labels, but found: $leakedExternal") { it }
+        .isEqualTo(true)
+
+    // Opt-out: --no-excludeExternalTargets reproduces the pre-#326 behavior so the synthetic
+    // labels show up. This proves the labels really exist in the hashes (so the filter is doing
+    // real work) and gives users an escape hatch.
+    assertThat(
+        cli.execute(
+            "get-impacted-targets",
+            "-w", projectB.absolutePath,
+            "-b", bazelPath,
+            "-sh", from.absolutePath,
+            "-fh", to.absolutePath,
+            "--no-excludeExternalTargets",
+            "-o", optOutOutput.absolutePath))
+        .isEqualTo(0)
+
+    val optOutLines = optOutOutput.readLines().filter { it.isNotBlank() }
+    val externalsWithOptOut = optOutLines.filter { it.startsWith("//external:") }
+    assertThat(externalsWithOptOut.isNotEmpty())
+        .transform(
+            "with --no-excludeExternalTargets, the impacted-targets output should expose synthetic //external:* labels (none found in: $optOutLines)") { it }
+        .isEqualTo(true)
+  }
+
   private fun copyTestWorkspace(path: String): File {
     val testProject = temp.newFolder()
 
diff --git a/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt b/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt
@@ -533,6 +533,70 @@ class CalculateImpactedTargetsInteractorTest : KoinTest {
     assertThat(output).containsExactly("//:target1")
   }
 
+  @Test
+  fun testExecuteFiltersExternalLabelsWhenExcludeFlagSet() {
+    // Unit-level guard for https://github.com/Tinder/bazel-diff/issues/326. The synthetic
+    // //external:<apparent_name> labels produced for bzlmod repos must drop out of the
+    // impacted-targets output when --excludeExternalTargets is set, while real workspace
+    // labels (//foo:bar) and canonical bzlmod labels (@@repo//pkg:tgt) remain.
+    val from =
+        mapOf(
+            "//foo:bar" to TargetHash("Rule", "h1", "h1"),
+            "//external:boost.assert" to TargetHash("Rule", "h2", "h2"),
+            "//external:guava" to TargetHash("Rule", "h3", "h3"),
+            "@@some_repo//pkg:tgt" to TargetHash("Rule", "h4", "h4"),
+        )
+    val to = from.mapValues { (_, v) -> v.copy(hash = v.hash + "-changed") }
+
+    val filteredWriter = StringWriter()
+    CalculateImpactedTargetsInteractor()
+        .execute(
+            from = from,
+            to = to,
+            outputWriter = filteredWriter,
+            targetTypes = null,
+            excludeExternalTargets = true)
+    val filteredLines = filteredWriter.toString().trimEnd('\n').split("\n").toSet()
+    assertThat(filteredLines).containsOnly("//foo:bar", "@@some_repo//pkg:tgt")
+
+    val unfilteredWriter = StringWriter()
+    CalculateImpactedTargetsInteractor()
+        .execute(
+            from = from,
+            to = to,
+            outputWriter = unfilteredWriter,
+            targetTypes = null,
+            excludeExternalTargets = false)
+    val unfilteredLines = unfilteredWriter.toString().trimEnd('\n').split("\n").toSet()
+    assertThat(unfilteredLines)
+        .containsOnly(
+            "//foo:bar", "//external:boost.assert", "//external:guava", "@@some_repo//pkg:tgt")
+  }
+
+  @Test
+  fun testExecuteWithDistancesFiltersExternalLabelsWhenExcludeFlagSet() {
+    // Same guarantee for the distance-metrics output path used when --depEdgesFile is provided.
+    val from =
+        mapOf(
+            "//foo:bar" to TargetHash("Rule", "h1", "h1"),
+            "//external:boost.assert" to TargetHash("Rule", "h2", "h2"),
+        )
+    val to = from.mapValues { (_, v) -> v.copy(hash = v.hash + "-changed", directHash = v.directHash + "-changed") }
+
+    val filteredWriter = StringWriter()
+    CalculateImpactedTargetsInteractor()
+        .executeWithDistances(
+            from = from,
+            to = to,
+            depEdges = mapOf(),
+            outputWriter = filteredWriter,
+            targetTypes = null,
+            excludeExternalTargets = true)
+    val filteredJson = filteredWriter.toString()
+    assertThat(filteredJson).contains("//foo:bar")
+    assertThat(filteredJson).doesNotContain("//external:boost.assert")
+  }
+
   @Test
   fun testIdenticalModuleGraphsSkipsParsing() {
     // When module graphs are identical, should skip parsing and use normal hash comparison
