Add reproducer for #301 (#313)

* Add reproducer for #301

Starts a reproducer for this issue

* Attempt to fix

* bump version

Author: tinder-maxwellelliott

MODULE.bazel

@@ -1,6 +1,6 @@
 module(
     name = "bazel-diff",
-    version = "15.0.5",
+    version = "16.0.0",
     compatibility_level = 0,
 )
 

MODULE.bazel.lock

@@ -187,6 +187,13 @@ workspace.
                           Additional space separated Bazel command options used
                             when invoking `bazel cquery`. This flag is has no
                             effect if `--useCquery`is false.
+      --cqueryExpression=<cqueryExpression>
+                          Custom cquery expression to use instead of the default
+                            'deps(//...:all-targets)'. This allows you to exclude
+                            problematic targets (e.g., analysis_test targets that
+                            are designed to fail). Example: 'deps(//:target1) +
+                            deps(//:target2)'. This flag has no effect if
+                            `--useCquery` is false.
       --fineGrainedHashExternalRepos=<fineGrainedHashExternalRepos>
                           Comma separate list of external repos in which
                             fine-grained hashes are computed for the targets.
@@ -239,6 +246,37 @@ workspace.
 of Bazel. You may want to fallback to use normal query mode in that case.
 See <https://github.com/bazelbuild/bazel/issues/17743> for more details.
 
+#### Handling Failing Analysis Targets with `--cqueryExpression`
+
+When using `--useCquery`, Bazel's `cquery` command analyzes all targets (executes their implementation functions). This can cause issues with targets that are intentionally designed to fail during analysis, such as:
+
+- `analysis_test` targets from the Bazel `rules_testing` library
+- Other validation targets that verify build failures
+
+With regular `bazel query`, these targets don't cause problems because `query` doesn't execute implementation functions. However, `cquery` will fail when it encounters these targets.
+
+**Solution**: Use the `--cqueryExpression` flag to specify a custom query expression that excludes the problematic targets:
+
+```bash
+bazel-diff generate-hashes \
+  --useCquery \
+  --cqueryExpression "deps(//:target1) + deps(//:target2)" \
+  output.json
+```
+
+**Important**: When crafting custom cquery expressions:
+
+- ❌ **Don't use**: `deps(//...:all-targets) except //:failing_target`
+  - This still analyzes the failing target during pattern expansion
+
+- ✅ **Do use**: Explicitly specify which targets or packages to include:
+  ```bash
+  --cqueryExpression "deps(//:target1) + deps(//:target2)"
+  --cqueryExpression "deps(//src/...:*) + deps(//lib/...:*)"
+  ```
+
+See [GitHub Issue #301](https://github.com/Tinder/bazel-diff/issues/301) for more details.
+
 ### What does the SHA256 value of `generate-hashes` represent?
 
 `generate-hashes` is a canonical SHA256 value representing all attributes and inputs into a target. These inputs

README.md

@@ -7,6 +7,7 @@ import org.koin.core.component.inject
 
 class BazelClient(
     private val useCquery: Boolean,
+    private val cqueryExpression: String?,
     private val fineGrainedHashExternalRepos: Set<String>,
     private val excludeExternalTargets: Boolean,
 ) : KoinComponent {
@@ -42,7 +43,8 @@ class BazelClient(
           // In addition, we must include all source dependencies in this query in order for them to
           // show up in
           // `configuredRuleInput`. Hence, one must not filter them out with `kind(rule, deps(..))`.
-          val mainTargets = queryService.query("deps(//...:all-targets)", useCquery = true)
+          val expression = cqueryExpression ?: "deps(//...:all-targets)"
+          val mainTargets = queryService.query(expression, useCquery = true)
           val repoTargets =
               if (repoTargetsQuery.isNotEmpty()) {
                 queryService.query(repoTargetsQuery.joinToString(" + ") { "'$it'" })

cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt

@@ -124,6 +124,14 @@ class GenerateHashesCommand : Callable<Int> {
   )
   var cqueryCommandOptions: List<String> = emptyList()
 
+  @CommandLine.Option(
+      names = ["--cqueryExpression"],
+      description =
+          [
+              "Custom cquery expression to use instead of the default 'deps(//...:all-targets)'. This allows you to exclude problematic targets (e.g., analysis_test targets that are designed to fail). Example: 'deps(//...:all-targets) except //path/to/failing:target'. This flag has no effect if `--useCquery` is false."],
+      scope = CommandLine.ScopeType.INHERIT)
+  var cqueryExpression: String? = null
+
   @CommandLine.Option(
       names = ["-k", "--keep_going"],
       negatable = true,
@@ -200,6 +208,7 @@ class GenerateHashesCommand : Callable<Int> {
               bazelCommandOptions,
               cqueryCommandOptions,
               useCquery,
+              cqueryExpression,
               keepGoing,
               depsMappingJSONPath != null,
               fineGrainedHashExternalRepos,

cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt

@@ -28,6 +28,7 @@ fun hasherModule(
     commandOptions: List<String>,
     cqueryOptions: List<String>,
     useCquery: Boolean,
+    cqueryExpression: String?,
     keepGoing: Boolean,
     trackDeps: Boolean,
     fineGrainedHashExternalRepos: Set<String>,
@@ -75,7 +76,7 @@ fun hasherModule(
   single {
     BazelModService(workingDirectory, bazelPath, startupOptions, debug)
   }
-  single { BazelClient(useCquery, updatedFineGrainedHashExternalRepos, excludeExternalTargets) }
+  single { BazelClient(useCquery, cqueryExpression, updatedFineGrainedHashExternalRepos, excludeExternalTargets) }
   single { BuildGraphHasher(get()) }
   single { TargetHasher() }
   single { RuleHasher(useCquery, trackDeps, updatedFineGrainedHashExternalRepos) }

cli/src/main/kotlin/com/bazel_diff/di/Modules.kt

@@ -14,7 +14,7 @@ fun testModule(): Module = module {
   val outputBase = Paths.get("output-base")
   val workingDirectory = Paths.get("working-directory")
   single<Logger> { SilentLogger }
-  single { BazelClient(false, emptySet(), false) }
+  single { BazelClient(false, null, emptySet(), false) }
   single { BuildGraphHasher(get()) }
   single { TargetHasher() }
   single { RuleHasher(false, true, emptySet()) }

cli/src/test/kotlin/com/bazel_diff/Modules.kt

@@ -720,6 +720,101 @@ class E2ETest {
     }
   }
 
+  @Test
+  fun testCqueryWithFailingAnalysisTargets() {
+    // Reproducer for https://github.com/Tinder/bazel-diff/issues/301
+    // This test demonstrates the issue where cquery executes implementation functions
+    // for all repository targets, causing targets designed to fail (like analysis_test
+    // from rules_testing) to fail during cquery execution.
+    //
+    // The workspace contains:
+    // - normal_target: A regular target that works fine
+    // - dependent_target: Another regular target
+    // - failing_analysis_target: A target designed to fail during analysis
+    //
+    // Expected behavior:
+    // - With query: All targets are found without executing implementation functions
+    // - With cquery: The failing_analysis_target causes analysis to fail
+    // - With cquery + keep_going: Partial results should be returned (only the non-failing targets)
+    //
+    // This test verifies the current behavior and demonstrates the issue.
+
+    val workspace = copyTestWorkspace("cquery_failing_target")
+    val outputDir = temp.newFolder()
+    val from = File(outputDir, "starting_hashes.json")
+
+    val cli = CommandLine(BazelDiff())
+
+    // First, verify that generate-hashes works without --useCquery
+    val exitCodeWithoutCquery = cli.execute(
+        "generate-hashes",
+        "-w",
+        workspace.absolutePath,
+        "-b",
+        "bazel",
+        from.absolutePath)
+
+    assertThat(exitCodeWithoutCquery).isEqualTo(0)
+
+    // Now, verify that generate-hashes fails with --useCquery due to the failing target
+    // This demonstrates the issue described in #301
+    val exitCodeWithCquery = cli.execute(
+        "generate-hashes",
+        "-w",
+        workspace.absolutePath,
+        "-b",
+        "bazel",
+        "--useCquery",
+        from.absolutePath)
+
+    // The cquery should fail because it tries to analyze the failing_analysis_target
+    // which is designed to fail during analysis
+    assertThat(exitCodeWithCquery).isEqualTo(1)
+
+    // Test with --keep_going enabled (default behavior)
+    // With keep_going, cquery returns partial results but still exits with code 1
+    // The current implementation allows exit codes 0 and 3, but cquery with keep_going
+    // returns exit code 1 when some targets fail analysis
+    val exitCodeWithCqueryKeepGoing = cli.execute(
+        "generate-hashes",
+        "-w",
+        workspace.absolutePath,
+        "-b",
+        "bazel",
+        "--useCquery",
+        "--keep_going",
+        from.absolutePath)
+
+    // This currently fails (exit code 1) because bazel-diff only allows exit codes 0 and 3
+    // but cquery with --keep_going returns exit code 1 when partial results are available
+    assertThat(exitCodeWithCqueryKeepGoing).isEqualTo(1)
+
+    // Test with custom cquery expression to exclude the failing target
+    // Note: We use explicit target selection instead of wildcard + except because
+    // cquery analyzes targets during pattern expansion, so "//...:all-targets except X"
+    // would still try to analyze X. The solution is to explicitly specify which targets to query.
+    val exitCodeWithCustomExpression = cli.execute(
+        "generate-hashes",
+        "-w",
+        workspace.absolutePath,
+        "-b",
+        "bazel",
+        "--useCquery",
+        "--cqueryExpression",
+        "deps(//:normal_target) + deps(//:dependent_target)",
+        from.absolutePath)
+
+    // With the custom expression that explicitly lists only the non-failing targets, this should succeed
+    assertThat(exitCodeWithCustomExpression).isEqualTo(0)
+
+    // Verify the hashes were generated successfully and contain the expected targets
+    val hashes = from.readText()
+    assertThat(hashes.contains("normal_target")).isEqualTo(true)
+    assertThat(hashes.contains("dependent_target")).isEqualTo(true)
+    // The failing target should not be in the hashes since it wasn't included in the query
+    assertThat(hashes.contains("failing_analysis_target")).isEqualTo(false)
+  }
+
   private fun copyTestWorkspace(path: String): File {
     val testProject = temp.newFolder()
 

cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt

@@ -0,0 +1,22 @@
+load(":failing_rule.bzl", "failing_rule")
+load("@bazel_skylib//rules:write_file.bzl", "write_file")
+
+# A normal target that should work fine
+write_file(
+    name = "normal_target",
+    out = "normal.txt",
+    content = ["This is a normal target"],
+)
+
+# A target designed to fail during analysis
+# This simulates analysis_test targets that are meant to validate build failures
+failing_rule(
+    name = "failing_analysis_target",
+)
+
+# Another normal target that depends on the normal target
+write_file(
+    name = "dependent_target",
+    out = "dependent.txt",
+    content = ["This depends on normal_target"],
+)

cli/src/test/resources/workspaces/cquery_failing_target/BUILD

@@ -0,0 +1,6 @@
+module(
+    name = "cquery_failing_target_test",
+    version = "0.0.0",
+)
+
+bazel_dep(name = "bazel_skylib", version = "1.9.0")