Refactor root cause reporting in ConfiguredTargetFunction

We now track Causes instead of plain Labels, which will allow us to do better reporting in the future. Add basic tests.

PiperOrigin-RevId: 198380468

1 files changed, 44 insertions, 41 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index 29ffee6abf..1e39c7693c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -38,10 +38,13 @@ import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget;
 import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget.DuplicateException;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId.ConfigurationId;
+import com.google.devtools.build.lib.causes.AnalysisFailedCause;
+import com.google.devtools.build.lib.causes.Cause;
+import com.google.devtools.build.lib.causes.LabelCause;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.Aspect;
@@ -149,7 +152,8 @@ public final class AspectFunction implements SkyFunction {
       throw new AspectCreationException(
           String.format(
               "%s from %s is not a skylark-defined aspect",
-              skylarkValueName, extensionLabel.toString()));
+              skylarkValueName, extensionLabel.toString()),
+          extensionLabel);
     } else {
       return (SkylarkDefinedAspect) skylarkAspect;
     }
@@ -202,7 +206,7 @@ public final class AspectFunction implements SkyFunction {
         | ConversionException
         | InconsistentFilesystemException e) {
       env.getListener().handle(Event.error(e.getMessage()));
-      throw new AspectCreationException(e.getMessage());
+      throw new AspectCreationException(e.getMessage(), extensionLabel);
     }
   }
 
@@ -211,7 +215,7 @@ public final class AspectFunction implements SkyFunction {
   public SkyValue compute(SkyKey skyKey, Environment env)
       throws AspectFunctionException, InterruptedException {
     SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
-    NestedSetBuilder<Label> transitiveRootCauses = NestedSetBuilder.stableOrder();
+    NestedSetBuilder<Cause> transitiveRootCauses = NestedSetBuilder.stableOrder();
     AspectKey key = (AspectKey) skyKey.argument();
     ConfiguredAspectFactory aspectFactory;
     Aspect aspect;
@@ -274,7 +278,8 @@ public final class AspectFunction implements SkyFunction {
       baseConfiguredTargetValue =
           (ConfiguredTargetValue) baseAndAspectValues.get(key.getBaseConfiguredTargetKey()).get();
     } catch (ConfiguredValueCreationException e) {
-      throw new AspectFunctionException(new AspectCreationException(e.getRootCauses()));
+      throw new AspectFunctionException(
+          new AspectCreationException(e.getMessage(), e.getRootCauses()));
     }
 
     if (aspectHasConfiguration) {
@@ -364,7 +369,8 @@ public final class AspectFunction implements SkyFunction {
                     associatedConfiguredTargetAndData.getTarget().getLocation(), e.getMessage()));
 
         throw new AspectFunctionException(
-            new AspectCreationException(e.getMessage(), associatedTarget.getLabel()));
+            new AspectCreationException(
+                e.getMessage(), associatedTarget.getLabel(), aspectConfiguration));
       }
     }
     associatedConfiguredTargetAndData =
@@ -416,7 +422,8 @@ public final class AspectFunction implements SkyFunction {
                 key.getAspectConfigurationKey());
       } catch (ToolchainContextException e) {
         // TODO(katre): better error handling
-        throw new AspectCreationException(e.getMessage());
+        throw new AspectCreationException(
+            e.getMessage(), new LabelCause(key.getLabel(), e.getMessage()));
       }
       if (env.valuesMissing()) {
         return null;
@@ -438,7 +445,7 @@ public final class AspectFunction implements SkyFunction {
                 transitiveRootCauses,
                 defaultBuildOptions);
       } catch (ConfiguredTargetFunctionException e) {
-        throw new AspectCreationException(e.getMessage());
+        throw new AspectCreationException(e.getMessage(), key.getLabel(), aspectConfiguration);
       }
       if (depValueMap == null) {
         return null;
@@ -463,17 +470,18 @@ public final class AspectFunction implements SkyFunction {
     } catch (DependencyEvaluationException e) {
       if (e.getCause() instanceof ConfiguredValueCreationException) {
         ConfiguredValueCreationException cause = (ConfiguredValueCreationException) e.getCause();
-        throw new AspectFunctionException(new AspectCreationException(
-            cause.getMessage(), cause.getAnalysisRootCause()));
+        throw new AspectFunctionException(
+            new AspectCreationException(cause.getMessage(), cause.getRootCauses()));
       } else if (e.getCause() instanceof InconsistentAspectOrderException) {
         InconsistentAspectOrderException cause = (InconsistentAspectOrderException) e.getCause();
-        throw new AspectFunctionException(new AspectCreationException(
-            cause.getMessage()));
+        throw new AspectFunctionException(
+            new AspectCreationException(cause.getMessage(), key.getLabel(), aspectConfiguration));
       } else {
         // Cast to InvalidConfigurationException as a consistency check. If you add any
         // DependencyEvaluationException constructors, you may need to change this code, too.
         InvalidConfigurationException cause = (InvalidConfigurationException) e.getCause();
-        throw new AspectFunctionException(new AspectCreationException(cause.getMessage()));
+        throw new AspectFunctionException(
+            new AspectCreationException(cause.getMessage(), key.getLabel(), aspectConfiguration));
       }
     } catch (AspectCreationException e) {
       throw new AspectFunctionException(e);
@@ -623,11 +631,11 @@ public final class AspectFunction implements SkyFunction {
     events.replayOn(env.getListener());
     if (events.hasErrors()) {
       analysisEnvironment.disable(associatedTarget.getTarget());
+      String msg = "Analysis of target '"
+          + associatedTarget.getTarget().getLabel()
+          + "' failed; build aborted";
       throw new AspectFunctionException(
-          new AspectCreationException(
-              "Analysis of target '"
-                  + associatedTarget.getTarget().getLabel()
-                  + "' failed; build aborted"));
+          new AspectCreationException(msg, key.getLabel(), aspectConfiguration));
     }
     Preconditions.checkState(!analysisEnvironment.hasErrors(),
         "Analysis environment hasError() but no errors reported");
@@ -672,41 +680,36 @@ public final class AspectFunction implements SkyFunction {
    * An exception indicating that there was a problem creating an aspect.
    */
   public static final class AspectCreationException extends Exception {
-    /** Targets in the transitive closure that failed to load. May be empty. */
-    private final NestedSet<Label> loadingRootCauses;
-
-    /**
-     * The target for which analysis failed, if any. We can't represent aspects with labels, so if
-     * the aspect analysis fails, this will be {@code null}.
-     */
-    @Nullable private final Label analysisRootCause;
-
-    public AspectCreationException(String message, Label analysisRootCause) {
-      super(message);
-      this.loadingRootCauses = NestedSetBuilder.<Label>emptySet(Order.STABLE_ORDER);
-      this.analysisRootCause = analysisRootCause;
+    private static ConfigurationId toId(BuildConfiguration config) {
+      return config == null ? null : config.getEventId().asStreamProto().getConfiguration();
     }
 
-    public AspectCreationException(String message, NestedSet<Label> loadingRootCauses) {
+    private final NestedSet<Cause> causes;
+
+    public AspectCreationException(String message, NestedSet<Cause> causes) {
       super(message);
-      this.loadingRootCauses = loadingRootCauses;
-      this.analysisRootCause = null;
+      this.causes = causes;
     }
 
-    public AspectCreationException(NestedSet<Label> loadingRootCauses) {
-      this("Loading failed", loadingRootCauses);
+    public AspectCreationException(
+        String message, Label currentTarget, @Nullable BuildConfiguration configuration) {
+      this(
+          message,
+          NestedSetBuilder.<Cause>stableOrder()
+              .add(new AnalysisFailedCause(currentTarget, toId(configuration), message))
+              .build());
     }
 
-    public AspectCreationException(String message) {
-      this(message, (Label) null);
+    public AspectCreationException(String message, Label currentTarget) {
+      this(message, currentTarget, null);
     }
 
-    public NestedSet<Label> getRootCauses() {
-      return loadingRootCauses;
+    public AspectCreationException(String message, Cause cause) {
+      this(message, NestedSetBuilder.<Cause>stableOrder().add(cause).build());
     }
 
-    @Nullable public Label getAnalysisRootCause() {
-      return analysisRootCause;
+    public NestedSet<Cause> getCauses() {
+      return causes;
     }
   }