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

9 files changed, 37 insertions, 140 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectDescriptor.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectDescriptor.java
deleted file mode 100644
index 2f56abbc15..0000000000
--- a/src/main/java/com/google/devtools/build/lib/analysis/AspectDescriptor.java
+++ /dev/null
@@ -1,103 +0,0 @@
-// Copyright 2016 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.analysis;
-
-import com.google.common.collect.ImmutableMultimap;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.packages.AspectClass;
-import com.google.devtools.build.lib.packages.AspectParameters;
-import com.google.protobuf.TextFormat;
-import java.util.Map.Entry;
-import java.util.Objects;
-
-/**
- * A pair of {@link AspectClass} and {@link AspectParameters}.
- *
- * Used for dependency resolution.
- */
-@Immutable
-public final class AspectDescriptor {
-  private final AspectClass aspectClass;
-  private final AspectParameters aspectParameters;
-
-  public AspectDescriptor(AspectClass aspectClass,
-      AspectParameters aspectParameters) {
-    this.aspectClass = aspectClass;
-    this.aspectParameters = aspectParameters;
-  }
-
-  public AspectDescriptor(AspectClass aspectClass) {
-    this(aspectClass, AspectParameters.EMPTY);
-  }
-
-  public AspectClass getAspectClass() {
-    return aspectClass;
-  }
-
-  public AspectParameters getParameters() {
-    return aspectParameters;
-  }
-
-  @Override
-  public int hashCode() {
-    return Objects.hash(aspectClass, aspectParameters);
-  }
-
-  @Override
-  public boolean equals(Object obj) {
-    if (obj == this) {
-      return true;
-    }
-
-    if (!(obj instanceof AspectDescriptor)) {
-      return false;
-    }
-
-    AspectDescriptor that = (AspectDescriptor) obj;
-    return Objects.equals(aspectClass, that.aspectClass)
-        && Objects.equals(aspectParameters, that.aspectParameters);
-  }
-
-  /**
-   * Creates a presentable description of this aspect, avaliable
-   * to Skylark via "Target.aspects".
-   *
-   * The description is designed to be unique for each aspect descriptor,
-   * but not to be parseable.
-   */
-  public String getDescription() {
-    if (aspectParameters.isEmpty()) {
-      return aspectClass.getName();
-    }
-
-    StringBuilder builder = new StringBuilder(aspectClass.getName());
-    builder.append('[');
-    ImmutableMultimap<String, String> attributes = aspectParameters.getAttributes();
-    boolean first = true;
-    for (Entry<String, String> attribute : attributes.entries()) {
-      if (!first) {
-        builder.append(',');
-      } else {
-        first = false;
-      }
-      builder.append(attribute.getKey());
-      builder.append("=\"");
-      builder.append(TextFormat.escapeDoubleQuotesAndBackslashes(attribute.getValue()));
-      builder.append("\"");
-    }
-    builder.append(']');
-    return builder.toString();
-  }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 8f065efa4d..19e972c4fe 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -51,6 +51,7 @@ import com.google.devtools.build.lib.events.Event;
 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 class BuildView {
     return new RuleContext.Builder(
             env,
             (Rule) target.getTarget(),
-            null,
-            null,
+            ImmutableList.<AspectDescriptor>of(),
             targetConfig,
             configurations.getHostConfiguration(),
             ruleClassProvider.getPrerequisiteValidator(),
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 6196bce78c..ff90a87a64 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
@@ -27,6 +27,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 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;
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 fdc0c74684..11d03fa117 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
@@ -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.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.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 @@ public final class ConfiguredTargetFactory {
         new RuleContext.Builder(
                 env,
                 rule,
-                null,
-                null,
+                ImmutableList.<AspectDescriptor>of(),
                 configuration,
                 hostConfiguration,
                 ruleClassProvider.getPrerequisiteValidator(),
@@ -304,6 +308,13 @@ public final class ConfiguredTargetFactory {
     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}.
@@ -311,6 +322,7 @@ public final class ConfiguredTargetFactory {
   public ConfiguredAspect createAspect(
       AnalysisEnvironment env,
       ConfiguredTarget associatedTarget,
+      ImmutableList<Aspect> aspectPath,
       ConfiguredAspectFactory aspectFactory,
       Aspect aspect,
       OrderedSetMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
@@ -321,8 +333,7 @@ public final class ConfiguredTargetFactory {
     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(),
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java b/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
index 755b991d86..d72bced27a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
@@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableMap;
 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;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
index 9fadc28fc3..60d274787a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.cmdline.Label;
 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;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
index e067612835..d5a483767c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.analysis;
 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;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 202be7b633..937cf74c45 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -53,7 +53,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 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 final class RuleContext extends TargetContext
   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 @@ public final class RuleContext extends TargetContext
     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 final class RuleContext extends TargetContext
   @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 final class RuleContext extends TargetContext
   @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 final class RuleContext extends TargetContext
     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 @@ public final class RuleContext extends TargetContext
 
     /** 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 final class RuleContext extends TargetContext
      * 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 @@ public final class RuleContext extends TargetContext
         prerequisiteValidator.validate(this, prerequisite, attribute);
       }
     }
-
-    @Nullable
-    public AspectParameters getAspectParameters() {
-      return aspectParameters;
-    }
-
-    @Nullable
-    public String getAspectName() {
-      return aspectName;
-    }
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index c4689c90f2..05da5c83cb 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -33,7 +33,6 @@ import com.google.common.collect.Maps;
 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.LabelSyntaxException;
 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;