Aspects-on-aspect see and propagate over aspect attributes.

If an aspect is applied to a rule+aspect node, all attributes are merged
into ctx.rule.attr collection, and the first one with the same name wins
(in particular, rule attribute will always win over aspect attribute).
This is backwards-compatible, and unlikely to cause problems in
practice.

RELNOTES: Aspects-on-aspect see and propagate over aspect attributes.
PiperOrigin-RevId: 179675660

1 files changed, 34 insertions, 30 deletions

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 6a027834fb..8fbdf4bf32 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
@@ -18,7 +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.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.AspectCollection.AspectCycleOnPathException;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -576,24 +575,31 @@ public abstract class DependencyResolver {
   }
 
   /**
-   * Collects into {@code filteredAspectPath}
-   * aspects from {@code aspectPath} that propagate along {@code attribute}
-   * and apply to a given {@code target}.
+   * Collects into {@code filteredAspectPath} aspects from {@code aspectPath} that propagate along
+   * {@code attributeAndOwner} and apply to a given {@code target}.
    *
-   * The last aspect in {@code aspectPath} is (potentially) visible and recorded
-   * in {@code visibleAspects}.
+   * <p>The last aspect in {@code aspectPath} is (potentially) visible and recorded in {@code
+   * visibleAspects}.
    */
-  private static void collectPropagatingAspects(Iterable<Aspect> aspectPath,
-      Attribute attribute, Rule target,
+  private static void collectPropagatingAspects(
+      Iterable<Aspect> aspectPath,
+      AttributeAndOwner attributeAndOwner,
+      Rule target,
       ImmutableList.Builder<Aspect> filteredAspectPath,
       ImmutableSet.Builder<AspectDescriptor> visibleAspects) {
 
     Aspect lastAspect = null;
     for (Aspect aspect : aspectPath) {
+      if (aspect.getAspectClass().equals(attributeAndOwner.ownerAspect)) {
+        // Do not propagate over the aspect's own attributes.
+        continue;
+      }
       lastAspect = aspect;
-      if (aspect.getDefinition().propagateAlong(attribute)
-          && aspect.getDefinition().getRequiredProviders()
-                  .isSatisfiedBy(target.getRuleClassObject().getAdvertisedProviders())) {
+      if (aspect.getDefinition().propagateAlong(attributeAndOwner.attribute)
+          && aspect
+              .getDefinition()
+              .getRequiredProviders()
+              .isSatisfiedBy(target.getRuleClassObject().getAdvertisedProviders())) {
         filteredAspectPath.add(aspect);
       } else {
         lastAspect = null;
@@ -682,22 +688,24 @@ public abstract class DependencyResolver {
       this.rootCauses = rootCauses;
       this.outgoingEdges = outgoingEdges;
 
-      this.attributes = getAttributes(rule,
-          // These are attributes that the application of `aspects` "path"
-          // to the rule will see. Application of path is really the
-          // application of the last aspect in the path, so we only let it see
-          // it's own attributes.
-          Iterables.getLast(aspects, null));
+      this.attributes =
+          getAttributes(
+              rule,
+              // These are attributes that the application of `aspects` "path"
+              // to the rule will see. Application of path is really the
+              // application of the last aspect in the path, so we only let it see
+              // it's own attributes.
+              aspects);
     }
 
     /** Returns the attributes that should be visited for this rule/aspect combination. */
-    private List<AttributeAndOwner> getAttributes(Rule rule, @Nullable Aspect aspect) {
+    private List<AttributeAndOwner> getAttributes(Rule rule, Iterable<Aspect> aspects) {
       ImmutableList.Builder<AttributeAndOwner> result = ImmutableList.builder();
       List<Attribute> ruleDefs = rule.getRuleClassObject().getAttributes();
       for (Attribute attribute : ruleDefs) {
         result.add(new AttributeAndOwner(attribute));
       }
-      if (aspect != null) {
+      for (Aspect aspect : aspects) {
         for (Attribute attribute : aspect.getDefinition().getAttributes().values()) {
           result.add(new AttributeAndOwner(attribute, aspect.getAspectClass()));
         }
@@ -754,25 +762,21 @@ public abstract class DependencyResolver {
         return AspectCollection.EMPTY;
       }
 
-      if (attributeAndOwner.ownerAspect != null) {
-        // Do not propagate aspects along aspect attributes.
-        return AspectCollection.EMPTY;
-      }
 
       ImmutableList.Builder<Aspect> filteredAspectPath = ImmutableList.builder();
       ImmutableSet.Builder<AspectDescriptor> visibleAspects = ImmutableSet.builder();
 
-      Attribute attribute = attributeAndOwner.attribute;
-      collectOriginatingAspects(rule, attribute, (Rule) target,
-          filteredAspectPath, visibleAspects);
+      if (attributeAndOwner.ownerAspect == null) {
+        collectOriginatingAspects(
+            rule, attributeAndOwner.attribute, (Rule) target, filteredAspectPath, visibleAspects);
+      }
 
-      collectPropagatingAspects(aspects,
-          attribute,
-          (Rule) target, filteredAspectPath, visibleAspects);
+      collectPropagatingAspects(
+          aspects, attributeAndOwner, (Rule) target, filteredAspectPath, visibleAspects);
       try {
         return AspectCollection.create(filteredAspectPath.build(), visibleAspects.build());
       } catch (AspectCycleOnPathException e) {
-        throw new InconsistentAspectOrderException(rule, attribute, target, e);
+        throw new InconsistentAspectOrderException(rule, attributeAndOwner.attribute, target, e);
       }
     }
   }