Names of extra-action protos now take into account all aspect names.

If an Aspect registered an action that an extra-action is
shadowing, its name is used when creating the extra-action's ID and
name.
Since recently, an aspect can see other aspects applied to the same
target. This CL record the names of other aspects applied to the target
as well, disambiguating the action owners.

--
PiperOrigin-RevId: 142264153
MOS_MIGRATED_REVID=142264153

src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java

@@ -27,6 +27,7 @@
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
@@ -44,6 +45,7 @@
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Map;
+import java.util.Map.Entry;
 import javax.annotation.Nullable;
 
 /**
@@ -470,12 +472,29 @@ public ExtraActionInfo.Builder getExtraActionInfo() {
             .setOwner(owner.getLabel().toString())
             .setId(getKey())
             .setMnemonic(getMnemonic());
-    if (owner.getAspectName() != null) {
-      result.setAspectName(owner.getAspectName());
+    Iterable<AspectDescriptor> aspectDescriptors = owner.getAspectDescriptors();
+    AspectDescriptor lastAspect = null;
+
+    for (AspectDescriptor aspectDescriptor : aspectDescriptors) {
+      ExtraActionInfo.AspectDescriptor.Builder builder =
+          ExtraActionInfo.AspectDescriptor.newBuilder()
+            .setAspectName(aspectDescriptor.getAspectClass().getName());
+      for (Entry<String, Collection<String>> entry :
+          aspectDescriptor.getParameters().getAttributes().asMap().entrySet()) {
+          builder.putAspectParameters(
+            entry.getKey(),
+            ExtraActionInfo.AspectDescriptor.StringList.newBuilder()
+                .addAllValue(entry.getValue())
+                .build()
+          );
+      }
+      lastAspect = aspectDescriptor;
     }
-    if (owner.getAspectParameters() != null) {
+    if (lastAspect != null) {
+      result.setAspectName(lastAspect.getAspectClass().getName());
+
       for (Map.Entry<String, Collection<String>> entry :
-          owner.getAspectParameters().getAttributes().asMap().entrySet()) {
+          lastAspect.getParameters().getAttributes().asMap().entrySet()) {
         result.putAspectParameters(
             entry.getKey(),
             ExtraActionInfo.StringList.newBuilder().addAllValue(entry.getValue()).build());

src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java

@@ -14,10 +14,11 @@
 package com.google.devtools.build.lib.actions;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.packages.AspectParameters;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.util.Preconditions;
 import javax.annotation.Nullable;
 
@@ -33,12 +34,12 @@
 public abstract class ActionOwner {
   /** An action owner for special cases. Usage is strongly discouraged. */
   public static final ActionOwner SYSTEM_ACTION_OWNER =
-      ActionOwner.create(null, null, null, null, "system", "empty target kind", "system", null);
+      ActionOwner.create(null, ImmutableList.<AspectDescriptor>of(),
+          null, "system", "empty target kind", "system", null);
 
   public static ActionOwner create(
       @Nullable Label label,
-      @Nullable String aspectName,
-      @Nullable AspectParameters aspectParameters,
+      ImmutableList<AspectDescriptor> aspectDescriptors,
       @Nullable Location location,
       @Nullable String mnemonic,
       @Nullable String targetKind,
@@ -47,8 +48,7 @@ public static ActionOwner create(
     return new AutoValue_ActionOwner(
         location,
         label,
-        aspectName,
-        aspectParameters,
+        aspectDescriptors,
         mnemonic,
         Preconditions.checkNotNull(configurationChecksum),
         targetKind,
@@ -63,11 +63,7 @@ public static ActionOwner create(
   @Nullable
   public abstract Label getLabel();
 
-  @Nullable
-  public abstract String getAspectName();
-
-  @Nullable
-  public abstract AspectParameters getAspectParameters();
+  public abstract ImmutableList<AspectDescriptor> getAspectDescriptors();
 
   /** Returns the configuration's mnemonic. */
   @Nullable

src/main/java/com/google/devtools/build/lib/analysis/BuildView.java

@@ -51,6 +51,7 @@
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.AspectClass;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.BuildType;
@@ -1105,8 +1106,7 @@ public RuleContext getRuleContextForTesting(EventHandler eventHandler, Configure
     return new RuleContext.Builder(
             env,
             (Rule) target.getTarget(),
-            null,
-            null,
+            ImmutableList.<AspectDescriptor>of(),
             targetConfig,
             configurations.getHostConfiguration(),
             ruleClassProvider.getPrerequisiteValidator(),

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java

@@ -27,6 +27,7 @@
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.AspectClass;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor.Key;

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java

@@ -14,7 +14,11 @@
 
 package com.google.devtools.build.lib.analysis;
 
+import static com.google.common.collect.Iterables.transform;
+
+import com.google.common.base.Function;
 import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactFactory;
@@ -32,6 +36,7 @@
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.Aspect;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy;
 import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
@@ -227,8 +232,7 @@ private ConfiguredTarget createRule(
         new RuleContext.Builder(
                 env,
                 rule,
-                null,
-                null,
+                ImmutableList.<AspectDescriptor>of(),
                 configuration,
                 hostConfiguration,
                 ruleClassProvider.getPrerequisiteValidator(),
@@ -304,13 +308,21 @@ private String missingFragmentError(
     return result.toString();
   }
 
+  private static final Function<Aspect, AspectDescriptor> ASPECT_TO_DESCRIPTOR =
+      new Function<Aspect, AspectDescriptor>() {
+        @Override
+        public AspectDescriptor apply(Aspect aspect) {
+          return aspect.getDescriptor();
+        }
+      };
   /**
    * Constructs an {@link ConfiguredAspect}. Returns null if an error occurs; in that case,
    * {@code aspectFactory} should call one of the error reporting methods of {@link RuleContext}.
    */
   public ConfiguredAspect createAspect(
       AnalysisEnvironment env,
       ConfiguredTarget associatedTarget,
+      ImmutableList<Aspect> aspectPath,
       ConfiguredAspectFactory aspectFactory,
       Aspect aspect,
       OrderedSetMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
@@ -321,8 +333,7 @@ public ConfiguredAspect createAspect(
     RuleContext.Builder builder = new RuleContext.Builder(
         env,
         associatedTarget.getTarget().getAssociatedRule(),
-        aspect.getAspectClass().getName(),
-        aspect.getParameters(),
+        ImmutableList.copyOf(transform(aspectPath, ASPECT_TO_DESCRIPTOR)),
         aspectConfiguration,
         hostConfiguration,
         ruleClassProvider.getPrerequisiteValidator(),

src/main/java/com/google/devtools/build/lib/analysis/Dependency.java

@@ -17,6 +17,7 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.util.Preconditions;
 import java.util.Map;

src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java

@@ -28,6 +28,7 @@
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.packages.Aspect;
 import com.google.devtools.build.lib.packages.AspectClass;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault;
 import com.google.devtools.build.lib.packages.AttributeMap;

src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java

@@ -16,6 +16,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor.Key;
 import java.util.ArrayList;

src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java

@@ -53,7 +53,7 @@
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.packages.AspectParameters;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
 import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
@@ -151,8 +151,7 @@ public ImmutableList<TransitiveInfoCollection> getFiles() {
   private static final String HOST_CONFIGURATION_PROGRESS_TAG = "for host";
 
   private final Rule rule;
-  @Nullable private final String aspectName;
-  @Nullable private final AspectParameters aspectParameters;
+  private final ImmutableList<AspectDescriptor> aspectDescriptors;
   private final ListMultimap<String, ConfiguredTarget> targetMap;
   private final ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap;
   private final ImmutableMap<Label, ConfigMatchingProvider> configConditions;
@@ -183,8 +182,7 @@ private RuleContext(
     super(builder.env, builder.rule, builder.configuration, builder.prerequisiteMap.get(null),
         builder.visibility);
     this.rule = builder.rule;
-    this.aspectName = builder.getAspectName();
-    this.aspectParameters = builder.getAspectParameters();
+    this.aspectDescriptors = builder.aspectDescriptors;
     this.configurationFragmentPolicy = builder.configurationFragmentPolicy;
     this.universalFragment = universalFragment;
     this.targetMap = targetMap;
@@ -334,7 +332,7 @@ public ListMultimap<String, ConfiguredFilesetEntry> getFilesetEntryMap() {
   @Override
   public ActionOwner getActionOwner() {
     if (actionOwner == null) {
-      actionOwner = createActionOwner(rule, aspectName, aspectParameters, getConfiguration());
+      actionOwner = createActionOwner(rule, aspectDescriptors, getConfiguration());
     }
     return actionOwner;
   }
@@ -412,13 +410,11 @@ public ImmutableList<Artifact> getBuildInfo(BuildInfoKey key) throws Interrupted
   @VisibleForTesting
   public static ActionOwner createActionOwner(
       Rule rule,
-      @Nullable String aspectName,
-      @Nullable AspectParameters aspectParameters,
+      ImmutableList<AspectDescriptor> aspectDescriptors,
       BuildConfiguration configuration) {
     return ActionOwner.create(
         rule.getLabel(),
-        aspectName,
-        aspectParameters,
+        aspectDescriptors,
         rule.getLocation(),
         configuration.getMnemonic(),
         rule.getTargetKind(),
@@ -1434,28 +1430,25 @@ public static final class Builder implements RuleErrorConsumer  {
     private final BuildConfiguration configuration;
     private final BuildConfiguration hostConfiguration;
     private final PrerequisiteValidator prerequisiteValidator;
-    @Nullable private final String aspectName;
-    @Nullable private final AspectParameters aspectParameters;
     private final ErrorReporter reporter;
     private OrderedSetMultimap<Attribute, ConfiguredTarget> prerequisiteMap;
     private ImmutableMap<Label, ConfigMatchingProvider> configConditions;
     private NestedSet<PackageSpecification> visibility;
     private ImmutableMap<String, Attribute> aspectAttributes;
     private ImmutableBiMap<String, Class<? extends TransitiveInfoProvider>> skylarkProviderRegistry;
+    private ImmutableList<AspectDescriptor> aspectDescriptors;
 
     Builder(
         AnalysisEnvironment env,
         Rule rule,
-        @Nullable String aspectName,
-        @Nullable AspectParameters aspectParameters,
+        ImmutableList<AspectDescriptor> aspectDescriptors,
         BuildConfiguration configuration,
         BuildConfiguration hostConfiguration,
         PrerequisiteValidator prerequisiteValidator,
         ConfigurationFragmentPolicy configurationFragmentPolicy) {
       this.env = Preconditions.checkNotNull(env);
       this.rule = Preconditions.checkNotNull(rule);
-      this.aspectName = aspectName;
-      this.aspectParameters = aspectParameters;
+      this.aspectDescriptors = aspectDescriptors;
       this.configurationFragmentPolicy = Preconditions.checkNotNull(configurationFragmentPolicy);
       this.configuration = Preconditions.checkNotNull(configuration);
       this.hostConfiguration = Preconditions.checkNotNull(hostConfiguration);
@@ -1736,7 +1729,7 @@ private void validateDirectPrerequisiteType(ConfiguredTarget prerequisite,
 
     /** Returns whether the context being constructed is for the evaluation of an aspect. */
     public boolean forAspect() {
-      return aspectName != null;
+      return !aspectDescriptors.isEmpty();
     }
 
     public Rule getRule() {
@@ -1747,9 +1740,11 @@ public Rule getRule() {
      * Returns a rule class name suitable for log messages, including an aspect name if applicable.
      */
     public String getRuleClassNameForLogging() {
-      return aspectName != null
-          ? aspectName + " aspect on " + rule.getRuleClass()
-          : rule.getRuleClass();
+      if (aspectDescriptors.isEmpty()) {
+        return rule.getRuleClass();
+      }
+
+      return Joiner.on(",").join(aspectDescriptors) + " aspect on " + rule.getRuleClass();
     }
 
     public BuildConfiguration getConfiguration() {
@@ -1959,16 +1954,6 @@ private void validateDirectPrerequisite(Attribute attribute, ConfiguredTarget pr
         prerequisiteValidator.validate(this, prerequisite, attribute);
       }
     }
-
-    @Nullable
-    public AspectParameters getAspectParameters() {
-      return aspectParameters;
-    }
-
-    @Nullable
-    public String getAspectName() {
-      return aspectName;
-    }
   }
 
   /**

src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java

@@ -33,7 +33,6 @@
 import com.google.common.collect.Multimap;
 import com.google.common.collect.MutableClassToInstanceMap;
 import com.google.devtools.build.lib.actions.Root;
-import com.google.devtools.build.lib.analysis.AspectDescriptor;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.Dependency;
@@ -44,6 +43,7 @@
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Attribute.Configurator;
 import com.google.devtools.build.lib.packages.Attribute.SplitTransition;