Fail gracefully on conflicting actions generated by an aspect. These can come from Skylark, so we shouldn't crash. As a safety measure, subclasses of ActionLookupValue are now responsible for detecting action conflicts themselves.

PiperOrigin-RevId: 187095271

1 files changed, 16 insertions, 10 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 9d3f49cb26..0f3b752d4e 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
@@ -19,7 +19,9 @@ import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.Actions;
+import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.analysis.AliasProvider;
 import com.google.devtools.build.lib.analysis.AspectResolver;
 import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment;
@@ -274,7 +276,6 @@ public final class AspectFunction implements SkyFunction {
     if (baseConfiguredTargetValue.getConfiguredTarget().getProvider(AliasProvider.class) != null) {
       return createAliasAspect(
           env,
-          view.getActionKeyContext(),
           associatedConfiguredTargetAndTarget.getTarget(),
           aspect,
           key,
@@ -394,7 +395,6 @@ public final class AspectFunction implements SkyFunction {
 
       return createAspect(
           env,
-          view.getActionKeyContext(),
           key,
           aspectPath,
           aspect,
@@ -478,7 +478,6 @@ public final class AspectFunction implements SkyFunction {
 
   private SkyValue createAliasAspect(
       Environment env,
-      ActionKeyContext actionKeyContext,
       Target originalTarget,
       Aspect aspect,
       AspectKey originalKey,
@@ -513,8 +512,7 @@ public final class AspectFunction implements SkyFunction {
         originalTarget.getLabel(),
         originalTarget.getLocation(),
         ConfiguredAspect.forAlias(real.getConfiguredAspect()),
-        actionKeyContext,
-        ImmutableList.of(),
+        GeneratingActions.EMPTY,
         transitivePackagesForPackageRootResolution,
         removeActionsAfterEvaluation.get());
   }
@@ -522,7 +520,6 @@ public final class AspectFunction implements SkyFunction {
   @Nullable
   private AspectValue createAspect(
       Environment env,
-      ActionKeyContext actionKeyContext,
       AspectKey key,
       ImmutableList<Aspect> aspectPath,
       Aspect aspect,
@@ -587,14 +584,23 @@ public final class AspectFunction implements SkyFunction {
     analysisEnvironment.disable(associatedTarget.getTarget());
     Preconditions.checkNotNull(configuredAspect);
 
+    GeneratingActions generatingActions;
+    // Check for conflicting actions within this aspect (indicates a bug in the implementation).
+    try {
+      generatingActions =
+          Actions.filterSharedActionsAndThrowActionConflict(
+              analysisEnvironment.getActionKeyContext(),
+              analysisEnvironment.getRegisteredActions());
+    } catch (ActionConflictException e) {
+      throw new AspectFunctionException(e);
+    }
     return new AspectValue(
         key,
         aspect,
         associatedTarget.getTarget().getLabel(),
         associatedTarget.getTarget().getLocation(),
         configuredAspect,
-        actionKeyContext,
-        ImmutableList.copyOf(analysisEnvironment.getRegisteredActions()),
+        generatingActions,
         transitivePackagesForPackageRootResolution == null
             ? null
             : transitivePackagesForPackageRootResolution.build(),
@@ -661,7 +667,7 @@ public final class AspectFunction implements SkyFunction {
       super(e, Transience.PERSISTENT);
     }
 
-    public AspectFunctionException(InconsistentAspectOrderException cause) {
+    public AspectFunctionException(ActionConflictException cause) {
       super(cause, Transience.PERSISTENT);
     }
   }