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

3 files changed, 119 insertions, 12 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
index 8e0be0cfe1..817bc3c69c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
@@ -30,20 +30,17 @@ public final class Aspect implements DependencyFilter.AttributeInfoProvider {
   /** */
   public static final String INJECTING_RULE_KIND_PARAMETER_KEY = "$injecting_rule_kind";
 
-  // TODO(bazel-team): class objects are not really hashable or comparable for equality other than
-  // by reference. We should identify the aspect here in a way that does not rely on comparison
-  // by reference so that keys can be serialized and deserialized properly.
-  private final AspectClass aspectClass;
-  private final AspectParameters parameters;
+  private final AspectDescriptor aspectDescriptor;
   private final AspectDefinition aspectDefinition;
 
   private Aspect(
       AspectClass aspectClass,
       AspectDefinition aspectDefinition,
       AspectParameters parameters) {
-    this.aspectClass = Preconditions.checkNotNull(aspectClass);
+    this.aspectDescriptor = new AspectDescriptor(
+        Preconditions.checkNotNull(aspectClass),
+        Preconditions.checkNotNull(parameters));
     this.aspectDefinition = Preconditions.checkNotNull(aspectDefinition);
-    this.parameters = Preconditions.checkNotNull(parameters);
   }
 
   public static Aspect forNative(
@@ -66,19 +63,23 @@ public final class Aspect implements DependencyFilter.AttributeInfoProvider {
    * Returns the aspectClass required for building the aspect.
    */
   public AspectClass getAspectClass() {
-    return aspectClass;
+    return aspectDescriptor.getAspectClass();
   }
 
   /**
    * Returns parameters for evaluation of the aspect.
    */
   public AspectParameters getParameters() {
-    return parameters;
+    return aspectDescriptor.getParameters();
+  }
+
+  public AspectDescriptor getDescriptor() {
+    return aspectDescriptor;
   }
 
   @Override
   public String toString() {
-    return String.format("Aspect %s(%s)", aspectClass, parameters);
+    return String.format("Aspect %s", aspectDescriptor.toString());
   }
 
   public AspectDefinition getDefinition() {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java b/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java
index 2d615e1f6f..e8c8cbe2d5 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java
@@ -72,7 +72,7 @@ package com.google.devtools.build.lib.packages;
  *    </li>
  *    <li>{@link AspectParameters} is a (key,value) pair list that can be used to
  *    parameterize aspect classes</li>
- *    <li>{@link com.google.devtools.build.lib.analysis.AspectDescriptor} is a pair
+ *    <li>{@link AspectDescriptor} is a pair
  *    of {@code AspectClass} and {@link AspectParameters}. It uniquely identifies
  *    the aspect and can be used in SkyKeys.
  *    </li>
@@ -89,7 +89,7 @@ package com.google.devtools.build.lib.packages;
  *    </li>
  *  </ul>
  *
- *  {@link com.google.devtools.build.lib.analysis.AspectDescriptor}, or in general, a tuple
+ *  {@link AspectDescriptor}, or in general, a tuple
  *  of ({@link AspectClass}, {@link AspectParameters}) is an identifier that should be
  *  used in SkyKeys or in other contexts that need equality for aspects.
  *  See also {@link com.google.devtools.build.lib.skyframe.AspectFunction} for details
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDescriptor.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDescriptor.java
new file mode 100644
index 0000000000..0d6e7435bd
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDescriptor.java
@@ -0,0 +1,106 @@
+// 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.packages;
+
+import com.google.common.collect.ImmutableMultimap;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+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);
+  }
+
+  @Override
+  public String toString() {
+    return getDescription();
+  }
+
+  /**
+   * 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();
+  }
+}