commit 2eb8bdd40f760d2f97d6597163148a4c2c7d1f41
author Dmitry Lomov <dslomov@google.com> Wed Apr 06 08:47:30 2016 +0000
committer Lukacs Berki <lberki@google.com> Thu Apr 07 11:43:50 2016 +0000
tree 76e96f9cdbc2cfbe2dc25e9c662a2cb50bf36208
parent b94a6d1593efb1052f9d7b70d21b9658797cb005

Use the correct Aspect in AspectFunction for Skylark aspects.

Previously AspectFunction was using an Aspect from the SkyKey, which
might have been stale.

This CL fixes the bug as uncovered in the test (see SkylarkAspectsTest),
but further refactoring is needed since SkylarkAspectClass equals() is
incorrect, and in fact obtaining the Skylark aspect definition should
always introduce Skyframe dependency.

--
MOS_MIGRATED_REVID=119137636

src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index 400aba7..c390a81499 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -31,6 +31,7 @@
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.StoredEventHandler;
+import com.google.devtools.build.lib.packages.Aspect;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
 import com.google.devtools.build.lib.packages.NativeAspectClass;
@@ -110,9 +111,12 @@
     NestedSetBuilder<Label> transitiveRootCauses = NestedSetBuilder.stableOrder();
     AspectKey key = (AspectKey) skyKey.argument();
     ConfiguredAspectFactory aspectFactory;
+    Aspect aspect;
     if (key.getAspectClass() instanceof NativeAspectClass<?>) {
+      NativeAspectClass<?> nativeAspectClass = (NativeAspectClass<?>) key.getAspectClass();
       aspectFactory =
-          (ConfiguredAspectFactory) ((NativeAspectClass<?>) key.getAspectClass()).newInstance();
+          (ConfiguredAspectFactory) nativeAspectClass.newInstance();
+      aspect = new Aspect(nativeAspectClass, key.getParameters());
     } else if (key.getAspectClass() instanceof SkylarkAspectClass) {
       SkylarkAspectClass skylarkAspectClass = (SkylarkAspectClass) key.getAspectClass();
       SkylarkAspect skylarkAspect;
@@ -128,6 +132,7 @@
       }
 
       aspectFactory = new SkylarkAspectFactory(skylarkAspect.getName(), skylarkAspect);
+      aspect = new Aspect(skylarkAspect.getAspectClass(), key.getParameters());
     } else {
       throw new IllegalStateException();
     }
@@ -202,7 +207,7 @@
               env,
               resolver,
               originalTargetAndAspectConfiguration,
-              key.getAspect(),
+              aspect,
               configConditions,
               ruleClassProvider,
               view.getHostConfiguration(originalTargetAndAspectConfiguration.getConfiguration()),
@@ -219,6 +224,7 @@
       return createAspect(
           env,
           key,
+          aspect,
           aspectFactory,
           associatedTarget,
           key.getAspectConfiguration(),
@@ -245,6 +251,7 @@
   private AspectValue createAspect(
       Environment env,
       AspectKey key,
+      Aspect aspect,
       ConfiguredAspectFactory aspectFactory,
       RuleConfiguredTarget associatedTarget,
       BuildConfiguration aspectConfiguration,
@@ -267,7 +274,7 @@
             analysisEnvironment,
             associatedTarget,
             aspectFactory,
-            key.getAspect(),
+            aspect,
             directDeps,
             configConditions,
             aspectConfiguration,
@@ -291,6 +298,7 @@
 
     return new AspectValue(
         key,
+        aspect,
         associatedTarget.getLabel(),
         associatedTarget.getTarget().getLocation(),
         configuredAspect,

src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index 43bc287..83a8a71 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -85,10 +85,6 @@
       return aspect.getParameters();
     }
 
-    public Aspect getAspect() {
-      return aspect;
-    }
-
     @Override
     public String getDescription() {
       return String.format("%s of %s", aspect.getAspectClass().getName(), getLabel());
@@ -231,6 +227,7 @@
 
 
   private final Label label;
+  private final Aspect aspect;
   private final Location location;
   private final AspectKey key;
   private final ConfiguredAspect configuredAspect;
@@ -238,12 +235,14 @@
 
   public AspectValue(
       AspectKey key,
+      Aspect aspect,
       Label label,
       Location location,
       ConfiguredAspect configuredAspect,
       Iterable<Action> actions,
       NestedSet<Package> transitivePackages) {
     super(actions);
+    this.aspect = aspect;
     this.location = location;
     this.label = label;
     this.key = key;
@@ -267,6 +266,10 @@
     return key;
   }
 
+  public Aspect getAspect() {
+    return aspect;
+  }
+
   public NestedSet<Package> getTransitivePackages() {
     return transitivePackages;
   }