[7.4.0] Don't mask error in cquery transitions output formatter (#2…

…3351) All errors used to be wrapped in an `InterruptedException` instead of being reported with a message. Also show a warning when an incompatible target is skipped instead of failing the build. Work towards #21010 Closes #23210. PiperOrigin-RevId: 660116656 Change-Id: I22651110984e471a6b31dcc59a387f3f85ad49bc 300c586 Co-authored-by: Fabian Meumertzheim <[email protected]>
bazelbuild · Sep 11, 2024 · 06f99bb · 06f99bb
1 parent 9e3fa2a
commit 06f99bb
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 5 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryTransitionResolver.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryTransitionResolver.java
@@ -125,8 +125,8 @@ public CqueryTransitionResolver(
    * @see ResolvedTransition for more details.
    * @param configuredTarget the configured target whose dependencies are being looked up.
    */
-  public ImmutableSet<ResolvedTransition> dependencies(CqueryNode configuredTarget)
-      throws EvaluateException, InterruptedException {
+  ImmutableSet<ResolvedTransition> dependencies(CqueryNode configuredTarget)
+      throws EvaluateException, InterruptedException, IncompatibleTargetException {
     if (!(configuredTarget instanceof RuleConfiguredTarget)) {
       return ImmutableSet.of();
     }
@@ -154,7 +154,7 @@ public ImmutableSet<ResolvedTransition> dependencies(CqueryNode configuredTarget
           eventHandler)) {
         throw new EvaluateException("DependencyResolver.evaluate did not complete");
       }
-    } catch (ReportedException | UnreportedException | IncompatibleTargetException e) {
+    } catch (ReportedException | UnreportedException e) {
       throw new EvaluateException(e.getMessage());
     }
 

diff --git a/.../java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java b/.../java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java
@@ -24,6 +24,7 @@
 import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
 import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
+import com.google.devtools.build.lib.analysis.constraints.IncompatibleTargetChecker;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
@@ -105,8 +106,18 @@ public void processOutput(Iterable<CqueryNode> partialResult) throws Interrupted
                     eventHandler, accessor, this, ruleClassProvider, transitionCache)
                 .dependencies(keyedConfiguredTarget);
       } catch (EvaluateException e) {
-        // This is an abuse of InterruptedException.
-        throw new InterruptedException(e.getMessage());
+        eventHandler.handle(
+            Event.error(
+                String.format(
+                    "Failed to evaluate %s: %s", keyedConfiguredTarget.getOriginalLabel(), e)));
+        return;
+      } catch (IncompatibleTargetChecker.IncompatibleTargetException e) {
+        eventHandler.handle(
+            Event.warn(
+                String.format(
+                    "Skipping dependencies of incompatible target %s",
+                    keyedConfiguredTarget.getOriginalLabel())));
+        return;
       }
       for (ResolvedTransition dep : dependencies) {
         addResult(

diff --git a/src/test/shell/integration/configured_query_test.sh b/src/test/shell/integration/configured_query_test.sh
@@ -217,6 +217,38 @@ function test_transitions_full() {
   assert_contains "host_dep#//$pkg:host#(exec + (TestTrimmingTransition + ConfigFeatureFlagTaggedTrimmingTransition))" output
 }
 
+function test_transitions_incompatible_target() {
+  local -r pkg=$FUNCNAME
+
+  mkdir -p $pkg
+  cat > $pkg/BUILD <<EOF
+constraint_setting(name = "incompatible_setting")
+constraint_value(
+    name = "incompatible",
+    constraint_setting = ":incompatible_setting",
+)
+genrule(
+    name = "gr",
+    srcs = [":input"],
+    cmd = "touch $@",
+    outs = ["out"],
+    target_compatible_with = [":incompatible"],
+)
+genrule(
+    name = "input",
+    cmd = "echo hi > $@",
+    outs = ["in"],
+)
+EOF
+
+    bazel cquery "deps(//$pkg:gr)" --transitions=lite \
+      > output 2>"$TEST_log" || fail "Excepted success"
+
+    assert_contains "//$pkg:gr" output
+    assert_not_contains "//$pkg:input" output
+    expect_log "WARNING: Skipping dependencies of incompatible target //$pkg:gr"
+}
+
 function write_test_targets() {
   mkdir -p $pkg
   cat > $pkg/rule.bzl <<'EOF'