TransitiveTraversalFunction now implements "conservative" (a la ConservativeAspectResolver) aspect resolution.

--
MOS_MIGRATED_REVID=100526575

3 files changed, 69 insertions, 31 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
index 36484551e6..fab884281a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
@@ -15,11 +15,8 @@ package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
-import com.google.common.collect.Multimap;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.packages.AspectDefinition;
-import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.InputFile;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
@@ -37,7 +34,6 @@ import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import com.google.devtools.build.skyframe.ValueOrException2;
 
-import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map.Entry;
@@ -143,32 +139,12 @@ abstract class TransitiveBaseTraversalFunction<TProcessedTargets>
     return Label.print(((Label) skyKey.argument()));
   }
 
-  private Iterable<SkyKey> getLabelAspectKeys(Target target, Environment env) {
-    List<SkyKey> depKeys = Lists.newArrayList();
-    if (target instanceof Rule) {
-      Multimap<Attribute, Label> transitions =
-          ((Rule) target).getTransitions(Rule.NO_NODEP_ATTRIBUTES);
-      for (Entry<Attribute, Label> entry : transitions.entries()) {
-        SkyKey packageKey = PackageValue.key(entry.getValue().getPackageIdentifier());
-        try {
-          PackageValue pkgValue = (PackageValue) env.getValueOrThrow(packageKey,
-              NoSuchThingException.class);
-          if (pkgValue == null) {
-            continue;
-          }
-          Collection<Label> labels = AspectDefinition.visitAspectsIfRequired(target, entry.getKey(),
-              pkgValue.getPackage().getTarget(entry.getValue().getName())).values();
-          for (Label label : labels) {
-            depKeys.add(getKey(label));
-          }
-        } catch (NoSuchThingException e) {
-          // Do nothing. This error was handled when we computed the corresponding
-          // TransitiveTargetValue.
-        }
-      }
-    }
-    return depKeys;
-  }
+  /**
+   * Return an Iterable of SkyKeys corresponding to the Aspect-related dependencies of target.
+   *
+   *  <p>This method may return a conservative over-approximation of the exact set.
+   */
+  protected abstract Iterable<SkyKey> getLabelAspectKeys(Target target, Environment env);
 
   private Iterable<SkyKey> getLabelDepKeys(Target target) {
     List<SkyKey> depKeys = Lists.newArrayList();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
index 8df7e1cfbd..7ff71175bd 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
@@ -14,6 +14,8 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Multimap;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
@@ -21,8 +23,11 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactor
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.packages.AspectDefinition;
+import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
+import com.google.devtools.build.lib.packages.NoSuchThingException;
 import com.google.devtools.build.lib.packages.PackageIdentifier;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
@@ -35,6 +40,7 @@ import com.google.devtools.build.skyframe.ValueOrException2;
 
 import java.util.Collection;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Map.Entry;
 import java.util.Set;
 
@@ -147,6 +153,34 @@ public class TransitiveTargetFunction
     return builder.build(errorLoadingTarget);
   }
 
+  @Override
+  protected Iterable<SkyKey> getLabelAspectKeys(Target target, Environment env) {
+    List<SkyKey> depKeys = Lists.newArrayList();
+    if (target instanceof Rule) {
+      Multimap<Attribute, Label> transitions =
+          ((Rule) target).getTransitions(Rule.NO_NODEP_ATTRIBUTES);
+      for (Entry<Attribute, Label> entry : transitions.entries()) {
+        SkyKey packageKey = PackageValue.key(entry.getValue().getPackageIdentifier());
+        try {
+          PackageValue pkgValue = (PackageValue) env.getValueOrThrow(packageKey,
+              NoSuchThingException.class);
+          if (pkgValue == null) {
+            continue;
+          }
+          Collection<Label> labels = AspectDefinition.visitAspectsIfRequired(target, entry.getKey(),
+              pkgValue.getPackage().getTarget(entry.getValue().getName())).values();
+          for (Label label : labels) {
+            depKeys.add(getKey(label));
+          }
+        } catch (NoSuchThingException e) {
+          // Do nothing. This error was handled when we computed the corresponding
+          // TransitiveTargetValue.
+        }
+      }
+    }
+    return depKeys;
+  }
+
   /**
    * Returns every configuration fragment known to the system.
    */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
index 910c8c31b3..1be5863bca 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
@@ -13,10 +13,17 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.LinkedHashMultimap;
+import com.google.common.collect.Multimap;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.packages.AspectDefinition;
+import com.google.devtools.build.lib.packages.AspectFactory;
+import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
+import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.skyframe.TransitiveTraversalFunction.DummyAccumulator;
 import com.google.devtools.build.lib.syntax.Label;
@@ -79,7 +86,28 @@ public class TransitiveTraversalFunction extends TransitiveBaseTraversalFunction
         : TransitiveTraversalValue.unsuccessfulTransitiveTraversal(errorLoadingTarget);
   }
 
-  /**
+ @Override
+ protected Iterable<SkyKey> getLabelAspectKeys(Target target, Environment env) {
+  if (!(target instanceof Rule)) {
+   return ImmutableSet.of();
+  }
+  Rule rule = (Rule) target;
+  Multimap<Attribute, Label> attibuteMap = LinkedHashMultimap.create();
+   for (Attribute attribute : rule.getTransitions(Rule.NO_NODEP_ATTRIBUTES).keys()) {
+    for (Class<? extends AspectFactory<?, ?, ?>> aspectFactory : attribute.getAspects()) {
+      AspectDefinition.addAllAttributesOfAspect(rule, attibuteMap,
+          AspectFactory.Util.create(aspectFactory).getDefinition(), Rule.ALL_DEPS);
+    }
+  }
+
+  ImmutableSet.Builder<SkyKey> depKeys = new ImmutableSet.Builder<>();
+  for (Label label : attibuteMap.values()) {
+    depKeys.add(getKey(label));
+  }
+  return depKeys.build();
+ }
+
+ /**
    * Because {@link TransitiveTraversalFunction} is invoked only when its side-effects are desired,
    * this value accumulator has nothing to keep track of.
    */