Expose aspect actions from Skylark.

Like with providers, consumers get a merged view of all actions from the merged configured target (all other aspects + the base target).

I had to rejig the aspect value / configured aspect to be symmetric with rule configured targets.

I do not expect significant memory bloat from this. All lists / maps already existed, only extra fields have been added.

RELNOTES: Expose aspect actions provider to Skylark.
PiperOrigin-RevId: 201697923

18 files changed, 177 insertions, 63 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
index a33e6fefd5..e99d134c83 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
@@ -18,10 +18,14 @@ import static com.google.devtools.build.lib.analysis.ExtraActionUtils.createExtr
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+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.ActionAnalysisMetadata;
+import com.google.devtools.build.lib.actions.Actions;
+import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -57,12 +61,20 @@ import javax.annotation.Nullable;
 @Immutable
 @AutoCodec
 public final class ConfiguredAspect {
-  private final TransitiveInfoProviderMap providers;
   private final AspectDescriptor descriptor;
+  private final ImmutableList<ActionAnalysisMetadata> actions;
+  private final ImmutableMap<Artifact, Integer> generatingActionIndex;
+  private final TransitiveInfoProviderMap providers;
 
   @AutoCodec.VisibleForSerialization
-  ConfiguredAspect(AspectDescriptor descriptor, TransitiveInfoProviderMap providers) {
+  ConfiguredAspect(
+      AspectDescriptor descriptor,
+      ImmutableList<ActionAnalysisMetadata> actions,
+      ImmutableMap<Artifact, Integer> generatingActionIndex,
+      TransitiveInfoProviderMap providers) {
     this.descriptor = descriptor;
+    this.actions = actions;
+    this.generatingActionIndex = generatingActionIndex;
     this.providers = providers;
   }
 
@@ -80,6 +92,18 @@ public final class ConfiguredAspect {
     return descriptor;
   }
 
+  public ImmutableList<ActionAnalysisMetadata> getActions() {
+    return actions;
+  }
+
+  /**
+   * Returns a map where keys are artifacts that are action outputs of this rule, and values are the
+   * index of the action that generates that artifact.
+   */
+  public ImmutableMap<Artifact, Integer> getGeneratingActionIndex() {
+    return generatingActionIndex;
+  }
+
   /** Returns the providers created by the aspect. */
   public TransitiveInfoProviderMap getProviders() {
     return providers;
@@ -112,11 +136,16 @@ public final class ConfiguredAspect {
   }
 
   public static ConfiguredAspect forAlias(ConfiguredAspect real) {
-    return new ConfiguredAspect(real.descriptor, real.getProviders());
+    return new ConfiguredAspect(
+        real.descriptor, real.getActions(), real.getGeneratingActionIndex(), real.getProviders());
   }
 
   public static ConfiguredAspect forNonapplicableTarget(AspectDescriptor descriptor) {
-    return new ConfiguredAspect(descriptor, new TransitiveInfoProviderMapBuilder().add().build());
+    return new ConfiguredAspect(
+        descriptor,
+        ImmutableList.of(),
+        ImmutableMap.of(),
+        new TransitiveInfoProviderMapBuilder().add().build());
   }
 
   public static Builder builder(
@@ -230,8 +259,7 @@ public final class ConfiguredAspect {
       return this;
     }
 
-
-    public ConfiguredAspect build() {
+    public ConfiguredAspect build() throws ActionConflictException {
       if (!outputGroupBuilders.isEmpty()) {
         ImmutableMap.Builder<String, NestedSet<Artifact>> outputGroups = ImmutableMap.builder();
         for (Map.Entry<String, NestedSetBuilder<Artifact>> entry : outputGroupBuilders.entrySet()) {
@@ -250,7 +278,17 @@ public final class ConfiguredAspect {
               /* actionsWithoutExtraAction= */ ImmutableSet.<ActionAnalysisMetadata>of(),
               ruleContext));
 
-      return new ConfiguredAspect(descriptor, providers.build());
+      AnalysisEnvironment analysisEnvironment = ruleContext.getAnalysisEnvironment();
+      GeneratingActions generatingActions =
+          Actions.filterSharedActionsAndThrowActionConflict(
+              analysisEnvironment.getActionKeyContext(),
+              analysisEnvironment.getRegisteredActions());
+
+      return new ConfiguredAspect(
+          descriptor,
+          generatingActions.getActions(),
+          generatingActions.getGeneratingActionIndex(),
+          providers.build());
     }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspectFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspectFactory.java
index 18462fb74d..a613ec620f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspectFactory.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis;
 
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
 
@@ -30,5 +31,5 @@ public interface ConfiguredAspectFactory {
    */
   ConfiguredAspect create(
       ConfiguredTargetAndData ctadBase, RuleContext context, AspectParameters parameters)
-      throws InterruptedException;
+      throws ActionConflictException, InterruptedException;
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index 456c6a10b9..c04365a903 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -64,6 +64,7 @@ import com.google.devtools.build.lib.packages.RuleVisibility;
 import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.profiler.memory.CurrentRuleTracker;
+import com.google.devtools.build.lib.skyframe.AspectFunction.AspectFunctionException;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import com.google.devtools.build.lib.util.OrderedSetMultimap;
@@ -389,7 +390,7 @@ public final class ConfiguredTargetFactory {
       @Nullable ToolchainContext toolchainContext,
       BuildConfiguration aspectConfiguration,
       BuildConfiguration hostConfiguration)
-      throws InterruptedException {
+      throws AspectFunctionException, InterruptedException {
 
     // Load the requested toolchains into the ToolchainContext.
     if (toolchainContext != null) {
@@ -424,8 +425,13 @@ public final class ConfiguredTargetFactory {
       return null;
     }
 
-    ConfiguredAspect configuredAspect =
-        aspectFactory.create(associatedTarget, ruleContext, aspect.getParameters());
+    ConfiguredAspect configuredAspect;
+    try {
+      configuredAspect =
+          aspectFactory.create(associatedTarget, ruleContext, aspect.getParameters());
+    } catch (ActionConflictException e) {
+      throw new AspectFunctionException(e);
+    }
     if (configuredAspect != null) {
       validateAdvertisedProviders(
           configuredAspect, aspect.getDefinition().getAdvertisedProviders(),
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java
index bb1e90851f..1e60f643fb 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.analysis.configuredtargets;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -39,6 +40,7 @@ import java.util.function.Consumer;
  */
 public final class MergedConfiguredTarget extends AbstractConfiguredTarget {
   private final ConfiguredTarget base;
+  private final ImmutableList<ConfiguredAspect> aspects;
   private final TransitiveInfoProviderMap providers;
 
   /**
@@ -52,9 +54,13 @@ public final class MergedConfiguredTarget extends AbstractConfiguredTarget {
     }
   }
 
-  private MergedConfiguredTarget(ConfiguredTarget base, TransitiveInfoProviderMap providers) {
+  private MergedConfiguredTarget(
+      ConfiguredTarget base,
+      Iterable<ConfiguredAspect> aspects,
+      TransitiveInfoProviderMap providers) {
     super(base.getLabel(), base.getConfigurationKey());
     this.base = base;
+    this.aspects = ImmutableList.copyOf(aspects);
     this.providers = providers;
   }
 
@@ -81,6 +87,7 @@ public final class MergedConfiguredTarget extends AbstractConfiguredTarget {
         result.accept((String) classAt);
       }
     }
+    result.accept(RuleConfiguredTarget.ACTIONS_FIELD_NAME);
   }
 
   @Override
@@ -94,6 +101,16 @@ public final class MergedConfiguredTarget extends AbstractConfiguredTarget {
 
   @Override
   protected Object rawGetSkylarkProvider(String providerKey) {
+    if (providerKey.equals(RuleConfiguredTarget.ACTIONS_FIELD_NAME)) {
+      ImmutableList.Builder<ActionAnalysisMetadata> actions = ImmutableList.builder();
+      for (ConfiguredAspect aspect : aspects) {
+        actions.addAll(aspect.getActions());
+      }
+      if (base instanceof RuleConfiguredTarget) {
+        actions.addAll(((RuleConfiguredTarget) base).getActions());
+      }
+      return actions.build();
+    }
     Object provider = providers.getProvider(providerKey);
     if (provider == null) {
       provider = base.get(providerKey);
@@ -159,7 +176,7 @@ public final class MergedConfiguredTarget extends AbstractConfiguredTarget {
         }
       }
     }
-    return new MergedConfiguredTarget(base, aspectProviders.build());
+    return new MergedConfiguredTarget(base, aspects, aspectProviders.build());
   }
 
   private static ImmutableList<OutputGroupInfo> getAllOutputGroupProviders(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java
index bea36d8e19..2f668f6f06 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java
@@ -60,7 +60,13 @@ import javax.annotation.Nullable;
  */
 @AutoCodec(checkClassExplicitlyAllowed = true)
 public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
-  private static final String ACTIONS_FIELD_NAME = "actions";
+  /**
+   * The name of the key for the 'actions' synthesized provider.
+   *
+   * <p>If you respond to this key you are expected to return a list of actions belonging to this
+   * configured target.
+   */
+  public static final String ACTIONS_FIELD_NAME = "actions";
 
   /**
    * The configuration transition for an attribute through which a prerequisite
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java
index 953f413af5..44d6228ea5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.rules.android;
 
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
 import com.google.devtools.build.lib.analysis.RuleContext;
@@ -46,7 +47,8 @@ public class AndroidNeverlinkAspect extends NativeAspectClass implements Configu
 
   @Override
   public ConfiguredAspect create(
-      ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters) {
+      ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
+      throws ActionConflictException {
     if (!JavaCommon.getConstraints(ruleContext).contains("android")
         && !ruleContext.getRule().getRuleClass().startsWith("android_")) {
       return new ConfiguredAspect.Builder(this, parameters, ruleContext).build();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
index 15460c70e5..d63f5936c8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
@@ -33,6 +33,7 @@ import com.google.common.collect.Sets;
 import com.google.common.collect.Streams;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ExecutionRequirements;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.actions.ParamFileInfo;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
@@ -178,7 +179,7 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu
   @Override
   public ConfiguredAspect create(
       ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters params)
-      throws InterruptedException {
+      throws InterruptedException, ActionConflictException {
     ConfiguredAspect.Builder result = new ConfiguredAspect.Builder(this, params, ruleContext);
     Function<Artifact, Artifact> desugaredJars =
         desugarJarsIfRequested(ctadBase.getConfiguredTarget(), ruleContext, result);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
index f4d3bb1570..17f5ac8842 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
@@ -23,6 +23,7 @@ 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.Artifact;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
 import com.google.devtools.build.lib.analysis.OutputGroupInfo;
@@ -90,7 +91,7 @@ public abstract class CcProtoAspect extends NativeAspectClass implements Configu
   @Override
   public ConfiguredAspect create(
       ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
-      throws InterruptedException {
+      throws InterruptedException, ActionConflictException {
     // Get SupportData, which is provided by the proto_library rule we attach to.
     SupportData supportData =
         checkNotNull(ctadBase.getConfiguredTarget().getProvider(ProtoSupportDataProvider.class))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java
index 635933476d..9e5b521b16 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java
@@ -24,6 +24,7 @@ import static com.google.devtools.build.lib.rules.java.proto.StrictDepsUtils.cre
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
 import com.google.devtools.build.lib.analysis.RuleContext;
@@ -91,7 +92,7 @@ public class JavaLiteProtoAspect extends NativeAspectClass implements Configured
   @Override
   public ConfiguredAspect create(
       ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
-      throws InterruptedException {
+      throws InterruptedException, ActionConflictException {
     ConfiguredAspect.Builder aspect = new ConfiguredAspect.Builder(this, parameters, ruleContext);
 
     // Get SupportData, which is provided by the proto_library rule we attach to.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
index 60128d3182..3c64f91a7d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
@@ -25,6 +25,7 @@ import static com.google.devtools.build.lib.rules.java.proto.StrictDepsUtils.cre
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
 import com.google.devtools.build.lib.analysis.RuleContext;
@@ -96,7 +97,7 @@ public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspe
   @Override
   public ConfiguredAspect create(
       ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
-      throws InterruptedException {
+      throws InterruptedException, ActionConflictException {
     ConfiguredAspect.Builder aspect = new ConfiguredAspect.Builder(this, parameters, ruleContext);
 
     if (!rpcSupport.checkAttributes(ruleContext, parameters)) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
index e28151d34d..3a5db10056 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
@@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.actions.ParamFileInfo;
 import com.google.devtools.build.lib.actions.ParameterFile;
 import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
@@ -220,7 +221,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
   @Override
   public ConfiguredAspect create(
       ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
-      throws InterruptedException {
+      throws InterruptedException, ActionConflictException {
     ConfiguredTarget base = ctadBase.getConfiguredTarget();
     if (isProtoRule(base)) {
       return proto(base, ruleContext, parameters);
@@ -237,7 +238,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
       J2ObjcMappingFileProvider directJ2ObjcMappingFileProvider,
       Iterable<Attribute> depAttributes,
       List<TransitiveInfoCollection> otherDeps)
-      throws InterruptedException {
+      throws InterruptedException, ActionConflictException {
     ConfiguredAspect.Builder builder = new ConfiguredAspect.Builder(this, parameters, ruleContext);
     ObjcCommon common;
 
@@ -300,7 +301,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
 
   private ConfiguredAspect java(
       ConfiguredTarget base, RuleContext ruleContext, AspectParameters parameters)
-      throws InterruptedException {
+      throws InterruptedException, ActionConflictException {
     JavaCompilationArgsProvider compilationArgsProvider =
         JavaInfo.getProvider(JavaCompilationArgsProvider.class, base);
     JavaSourceInfoProvider sourceInfoProvider = base.getProvider(JavaSourceInfoProvider.class);
@@ -349,7 +350,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
 
   private ConfiguredAspect proto(
       ConfiguredTarget base, RuleContext ruleContext, AspectParameters parameters)
-      throws InterruptedException {
+      throws InterruptedException, ActionConflictException {
     ProtoSourcesProvider protoSourcesProvider = base.getProvider(ProtoSourcesProvider.class);
     ImmutableList<Artifact> protoSources = protoSourcesProvider.getDirectProtoSources();
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java
index 279f19ec73..013fdb71e6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.objc;
 
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
 import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts;
@@ -48,7 +49,7 @@ public class ObjcProtoAspect extends SkylarkNativeAspect implements ConfiguredAs
   @Override
   public ConfiguredAspect create(
       ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
-      throws InterruptedException {
+      throws InterruptedException, ActionConflictException {
     ConfiguredAspect.Builder aspectBuilder = new ConfiguredAspect.Builder(
         this, parameters, ruleContext);
 
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 de5db333d1..e1339795f5 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
@@ -18,8 +18,6 @@ import com.google.common.base.Preconditions;
 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.Actions;
-import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
 import com.google.devtools.build.lib.actions.InconsistentFilesystemException;
 import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.analysis.AliasProvider;
@@ -573,7 +571,6 @@ public final class AspectFunction implements SkyFunction {
         originalTarget.getLabel(),
         originalTarget.getLocation(),
         ConfiguredAspect.forAlias(real.getConfiguredAspect()),
-        GeneratingActions.EMPTY,
         transitivePackagesForPackageRootResolution);
   }
 
@@ -644,23 +641,12 @@ 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,
-        generatingActions,
         transitivePackagesForPackageRootResolution == null
             ? null
             : transitivePackagesForPackageRootResolution.build());
@@ -709,10 +695,8 @@ public final class AspectFunction implements SkyFunction {
     }
   }
 
-  /**
-   * Used to indicate errors during the computation of an {@link AspectValue}.
-   */
-  private static final class AspectFunctionException extends SkyFunctionException {
+  /** Used to indicate errors during the computation of an {@link AspectValue}. */
+  public static final class AspectFunctionException extends SkyFunctionException {
     public AspectFunctionException(NoSuchThingException e) {
       super(e, Transience.PERSISTENT);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index 7096da3a5b..01a0df969b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -18,11 +18,7 @@ import com.google.common.base.MoreObjects;
 import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Interner;
-import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
-import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
-import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.BasicActionLookupValue;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -441,10 +437,8 @@ public final class AspectValue extends BasicActionLookupValue {
       Aspect aspect,
       Label label,
       Location location,
-      ConfiguredAspect configuredAspect,
-      ImmutableList<ActionAnalysisMetadata> actions,
-      ImmutableMap<Artifact, Integer> generatingActionIndex) {
-    super(actions, generatingActionIndex);
+      ConfiguredAspect configuredAspect) {
+    super(configuredAspect.getActions(), configuredAspect.getGeneratingActionIndex());
     this.label = Preconditions.checkNotNull(label, actions);
     this.aspect = Preconditions.checkNotNull(aspect, label);
     this.location = Preconditions.checkNotNull(location, label);
@@ -459,9 +453,8 @@ public final class AspectValue extends BasicActionLookupValue {
       Label label,
       Location location,
       ConfiguredAspect configuredAspect,
-      GeneratingActions actions,
       NestedSet<Package> transitivePackagesForPackageRootResolution) {
-    super(actions);
+    super(configuredAspect.getActions(), configuredAspect.getGeneratingActionIndex());
     this.label = Preconditions.checkNotNull(label, actions);
     this.aspect = Preconditions.checkNotNull(aspect, label);
     this.location = Preconditions.checkNotNull(location, label);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
index 1bb02261b2..2c95721c0b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
@@ -49,7 +50,7 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory {
   @Override
   public ConfiguredAspect create(
       ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
-      throws InterruptedException {
+      throws InterruptedException, ActionConflictException {
     SkylarkRuleContext skylarkRuleContext = null;
     try (Mutability mutability = Mutability.create("aspect")) {
       AspectDescriptor aspectDescriptor =
@@ -106,7 +107,7 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory {
 
   private ConfiguredAspect createAspect(
       Object aspectSkylarkObject, AspectDescriptor aspectDescriptor, RuleContext ruleContext)
-      throws EvalException {
+      throws EvalException, ActionConflictException {
 
     ConfiguredAspect.Builder builder = new ConfiguredAspect.Builder(aspectDescriptor, ruleContext);
 
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index fe64dbd10e..7ecbba6163 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil.NullAction;
 import com.google.devtools.build.lib.analysis.config.HostTransition;
 import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
@@ -437,7 +438,7 @@ public class AspectTest extends AnalysisTestCase {
       @Override
       public ConfiguredAspect create(
           ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
-          throws InterruptedException {
+          throws InterruptedException, ActionConflictException {
         Object lateBoundPrereq = ruleContext.getPrerequisite(":late", TARGET);
         return new ConfiguredAspect.Builder(this, parameters, ruleContext)
             .addProvider(
@@ -507,7 +508,7 @@ public class AspectTest extends AnalysisTestCase {
       @Override
       public ConfiguredAspect create(
           ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
-          throws InterruptedException {
+          throws InterruptedException, ActionConflictException {
         ruleContext.registerAction(new NullAction(ruleContext.createOutputArtifact()));
         return new ConfiguredAspect.Builder(this, parameters, ruleContext).build();
       }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
index cebf5db59b..22220e54e9 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
@@ -228,7 +228,8 @@ public class TestAspects {
     implements ConfiguredAspectFactory {
     @Override
     public ConfiguredAspect create(
-        ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters) {
+        ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
+        throws ActionConflictException {
       String information = parameters.isEmpty()
           ? ""
           : " data " + Iterables.getFirst(parameters.getAttribute("baz"), null);
@@ -274,7 +275,8 @@ public class TestAspects {
 
     @Override
     public ConfiguredAspect create(
-        ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters) {
+        ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
+        throws ActionConflictException {
       return new ConfiguredAspect.Builder(this, parameters, ruleContext)
           .addProvider(new FooProvider())
           .build();
@@ -293,7 +295,8 @@ public class TestAspects {
 
     @Override
     public ConfiguredAspect create(
-        ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters) {
+        ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
+        throws ActionConflictException {
       return new ConfiguredAspect.Builder(this, parameters, ruleContext)
           .addProvider(new BarProvider())
           .build();
@@ -428,7 +431,8 @@ public class TestAspects {
 
     @Override
     public ConfiguredAspect create(
-        ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters) {
+        ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
+        throws ActionConflictException {
       StringBuilder information = new StringBuilder("aspect " + ruleContext.getLabel());
       if (!parameters.isEmpty()) {
         information.append(" data " + Iterables.getFirst(parameters.getAttribute("baz"), null));
@@ -475,7 +479,8 @@ public class TestAspects {
 
     @Override
     public ConfiguredAspect create(
-        ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters) {
+        ConfiguredTargetAndData ctadBase, RuleContext ruleContext, AspectParameters parameters)
+        throws ActionConflictException {
       ruleContext.ruleWarning("Aspect warning on " + ctadBase.getTarget().getLabel());
       return new ConfiguredAspect.Builder(this, parameters, ruleContext).build();
     }
@@ -531,7 +536,7 @@ public class TestAspects {
     @Override
     public ConfiguredAspect create(
         ConfiguredTargetAndData ctadBase, RuleContext context, AspectParameters parameters)
-        throws InterruptedException {
+        throws InterruptedException, ActionConflictException {
       return new ConfiguredAspect.Builder(this, parameters, context).build();
     }
   }
@@ -777,7 +782,7 @@ public class TestAspects {
     @Override
     public ConfiguredAspect create(
         ConfiguredTargetAndData ctadBase, RuleContext context, AspectParameters parameters)
-        throws InterruptedException {
+        throws InterruptedException, ActionConflictException {
       return ConfiguredAspect.builder(this, parameters, context)
           .addProvider(Provider.class, new Provider(ctadBase.getConfiguredTarget().getLabel()))
           .build();
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
index 57170f82f2..b81b4936a9 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
@@ -17,10 +17,12 @@ import static com.google.common.collect.Iterables.transform;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static com.google.devtools.build.lib.analysis.OutputGroupInfo.INTERNAL_SUFFIX;
+import static java.util.stream.Collectors.toList;
 import static org.junit.Assert.fail;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.AnalysisResult;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
@@ -2589,6 +2591,58 @@ public class SkylarkDefinedAspectsTest extends AnalysisTestCase {
     assertThat(objcProtoProvider).isNotNull();
   }
 
+  @Test
+  public void testAspectActionProvider() throws Exception {
+    scratch.file(
+        "test/aspect.bzl",
+        "a1p = provider()",
+        "def _a1_impl(target,ctx):",
+        "  ctx.actions.run_shell(",
+        "    outputs = [ctx.actions.declare_file('a1')],",
+        "    command = 'touch $@'",
+        "  )",
+        "  return struct(a1p=a1p())",
+        "a1 = aspect(_a1_impl, attr_aspects = ['dep'], provides = ['a1p'])",
+        "a2p = provider()",
+        "def _a2_impl(target, ctx):",
+        "  value = []",
+        "  if hasattr(ctx.rule.attr, 'dep') and hasattr(ctx.rule.attr.dep, 'a2p'):",
+        "     value += ctx.rule.attr.dep.a2p.value",
+        "  value += target.actions",
+        "  return struct(a2p = a2p(value = value))",
+        "a2 = aspect(_a2_impl, attr_aspects = ['dep'], required_aspect_providers = ['a1p'])",
+        "def _r0_impl(ctx):",
+        "  ctx.actions.run_shell(",
+        "    outputs = [ctx.actions.declare_file('r0')],",
+        "    command = 'touch $@'",
+        "  )",
+        "def _r1_impl(ctx):",
+        "  pass",
+        "def _r2_impl(ctx):",
+        "  return struct(result = ctx.attr.dep.a2p.value)",
+        "r0 = rule(_r0_impl)",
+        "r1 = rule(_r1_impl, attrs = { 'dep' : attr.label(aspects = [a1])})",
+        "r2 = rule(_r2_impl, attrs = { 'dep' : attr.label(aspects = [a2])})");
+    scratch.file(
+        "test/BUILD",
+        "load(':aspect.bzl', 'r0', 'r1', 'r2')",
+        "r0(name = 'r0')",
+        "r1(name = 'r1', dep = ':r0')",
+        "r2(name = 'r2', dep = ':r1')");
+    AnalysisResult analysisResult = update("//test:r2");
+    ConfiguredTarget target = Iterables.getOnlyElement(analysisResult.getTargetsToBuild());
+    SkylarkList<?> result = (SkylarkList<?>) target.get("result");
+
+    // We should see both the action from the 'r0' rule, and the action from the 'a1' aspect
+    assertThat(result).hasSize(2);
+    assertThat(
+            result
+                .stream()
+                .map(a -> ((Action) a).getPrimaryOutput().getExecPath().getBaseName())
+                .collect(toList()))
+        .containsExactly("r0", "a1");
+  }
+
   /** SkylarkAspectTest with "keep going" flag */
   @RunWith(JUnit4.class)
   public static final class WithKeepGoing extends SkylarkDefinedAspectsTest {