Implement propagation along dependencies for Skylark aspects.

--
MOS_MIGRATED_REVID=106694515

9 files changed, 245 insertions, 182 deletions

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 57dc717f00..13764f62dc 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
@@ -25,21 +25,21 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget;
 import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.events.StoredEventHandler;
-import com.google.devtools.build.lib.packages.AspectFactory;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
+import com.google.devtools.build.lib.packages.NativeAspectClass;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspect;
+import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspectClass;
 import com.google.devtools.build.lib.skyframe.ASTFileLookupValue.ASTLookupInputException;
 import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
-import com.google.devtools.build.lib.skyframe.AspectValue.NativeAspectKey;
-import com.google.devtools.build.lib.skyframe.AspectValue.SkylarkAspectKey;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.DependencyEvaluationException;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor.BuildViewProvider;
 import com.google.devtools.build.lib.syntax.Type.ConversionException;
@@ -55,30 +55,38 @@ import javax.annotation.Nullable;
 /**
  * The Skyframe function that generates aspects.
  */
-public final class AspectFunction<T extends AspectKey> implements SkyFunction {
+public final class AspectFunction implements SkyFunction {
   private final BuildViewProvider buildViewProvider;
   private final RuleClassProvider ruleClassProvider;
-  private final AspectFactoryCreator<T> aspectFactoryCreator;
 
-  public static AspectFunction<NativeAspectKey> createNativeAspectFunction(
-      BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
-    return new AspectFunction<>(
-        buildViewProvider, ruleClassProvider, new NativeAspectFactoryCreator());
+  public AspectFunction(BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
+    this.buildViewProvider = buildViewProvider;
+    this.ruleClassProvider = ruleClassProvider;
   }
 
-  public static AspectFunction<SkylarkAspectKey> createSkylarkAspectFunction(
-      BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
-    return new AspectFunction<>(
-        buildViewProvider, ruleClassProvider, new SkylarkAspectFactoryCreator());
-  }
+  /**
+   * Load Skylark aspect from an extension file. Is to be called from a SkyFunction.
+   *
+   * @return {@code null} if dependencies cannot be satisfied.
+   */
+  @Nullable
+  public static SkylarkAspect loadSkylarkAspect(
+      Environment env, PackageIdentifier extensionFile, String skylarkValueName)
+      throws ASTLookupInputException, ConversionException {
+    SkyKey importFileKey;
+    importFileKey = SkylarkImportLookupValue.key(extensionFile);
+    SkylarkImportLookupValue skylarkImportLookupValue =
+        (SkylarkImportLookupValue) env.getValue(importFileKey);
+    if (skylarkImportLookupValue == null) {
+      return null;
+    }
 
-  private AspectFunction(
-      BuildViewProvider buildViewProvider,
-      RuleClassProvider ruleClassProvider,
-      AspectFactoryCreator<T> aspectFactoryCreator) {
-    this.buildViewProvider = buildViewProvider;
-    this.ruleClassProvider = ruleClassProvider;
-    this.aspectFactoryCreator = aspectFactoryCreator;
+    Object skylarkValue = skylarkImportLookupValue.getEnvironmentExtension().get(skylarkValueName);
+    if (!(skylarkValue instanceof SkylarkAspect)) {
+      throw new ConversionException(
+          skylarkValueName + " from " + extensionFile.toString() + " is not an aspect");
+    }
+    return (SkylarkAspect) skylarkValue;
   }
 
   @Nullable
@@ -88,7 +96,28 @@ public final class AspectFunction<T extends AspectKey> implements SkyFunction {
     SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
     NestedSetBuilder<Package> transitivePackages = NestedSetBuilder.stableOrder();
     AspectKey key = (AspectKey) skyKey.argument();
-    ConfiguredAspectFactory aspectFactory = aspectFactoryCreator.createAspectFactory(skyKey, env);
+    ConfiguredAspectFactory aspectFactory;
+    if (key.getAspect() instanceof NativeAspectClass<?>) {
+      aspectFactory =
+          (ConfiguredAspectFactory) ((NativeAspectClass<?>) key.getAspect()).newInstance();
+    } else if (key.getAspect() instanceof SkylarkAspectClass) {
+      SkylarkAspectClass skylarkAspectClass = (SkylarkAspectClass) key.getAspect();
+      SkylarkAspect skylarkAspect;
+      try {
+        skylarkAspect =
+            loadSkylarkAspect(
+                env, skylarkAspectClass.getExtensionFile(), skylarkAspectClass.getExportedName());
+      } catch (ASTLookupInputException | ConversionException e) {
+        throw new AspectFunctionException(skyKey, e);
+      }
+      if (skylarkAspect == null) {
+        return null;
+      }
+
+      aspectFactory = new SkylarkAspectFactory(skylarkAspect.getName(), skylarkAspect);
+    } else {
+      throw new IllegalStateException();
+    }
 
     PackageValue packageValue =
         (PackageValue) env.getValue(PackageValue.key(key.getLabel().getPackageIdentifier()));
@@ -148,9 +177,15 @@ public final class AspectFunction<T extends AspectKey> implements SkyFunction {
       }
 
       ListMultimap<Attribute, ConfiguredTarget> depValueMap =
-          ConfiguredTargetFunction.computeDependencies(env, resolver, ctgValue,
-              aspectFactory.getDefinition(), key.getParameters(), configConditions,
-              ruleClassProvider, view.getHostConfiguration(ctgValue.getConfiguration()),
+          ConfiguredTargetFunction.computeDependencies(
+              env,
+              resolver,
+              ctgValue,
+              key.getAspect().getDefinition(),
+              key.getParameters(),
+              configConditions,
+              ruleClassProvider,
+              view.getHostConfiguration(ctgValue.getConfiguration()),
               transitivePackages);
 
       return createAspect(
@@ -247,57 +282,4 @@ public final class AspectFunction<T extends AspectKey> implements SkyFunction {
     }
   }
 
-  /**
-   * Factory for {@link ConfiguredAspectFactory} given a particular kind of {@link AspectKey}.
-   */
-  private interface AspectFactoryCreator<T extends AspectKey> {
-    ConfiguredAspectFactory createAspectFactory(SkyKey skyKey, Environment env)
-        throws AspectFunctionException;
-  }
-
-  /**
-   * Factory for native aspects.
-   */
-  private static class NativeAspectFactoryCreator implements AspectFactoryCreator<NativeAspectKey> {
-
-    @Override
-    public ConfiguredAspectFactory createAspectFactory(SkyKey skyKey, Environment env) {
-      NativeAspectKey key = (NativeAspectKey) skyKey.argument();
-      return (ConfiguredAspectFactory) AspectFactory.Util.create(key.getAspect());
-    }
-  }
-
-  /**
-   * Factory for Skylark aspects.
-   */
-  private static class SkylarkAspectFactoryCreator
-      implements AspectFactoryCreator<SkylarkAspectKey> {
-
-    @Override
-    public ConfiguredAspectFactory createAspectFactory(SkyKey skyKey, Environment env)
-        throws AspectFunctionException {
-      SkylarkAspectKey skylarkAspectKey = (SkylarkAspectKey) skyKey.argument();
-      SkyKey importFileKey;
-      try {
-        importFileKey = SkylarkImportLookupValue.key(skylarkAspectKey.getExtensionFile());
-      } catch (ASTLookupInputException e) {
-        throw new AspectFunctionException(skyKey, e);
-      }
-      SkylarkImportLookupValue skylarkImportLookupValue =
-          (SkylarkImportLookupValue) env.getValue(importFileKey);
-      if (skylarkImportLookupValue == null) {
-        return null;
-      }
-      Object skylarkValue = skylarkImportLookupValue
-          .getEnvironmentExtension()
-          .get(skylarkAspectKey.getSkylarkValueName());
-      if (!(skylarkValue instanceof SkylarkAspect)) {
-        throw new AspectFunctionException(
-            new ConversionException(skylarkAspectKey.getSkylarkValueName() + " from "
-                + skylarkAspectKey.getExtensionFile().toString() + " is not an aspect"));
-      }
-      SkylarkAspect skylarkAspect = (SkylarkAspect) skylarkValue;
-      return new SkylarkAspectFactory(skylarkAspectKey.getSkylarkValueName(), skylarkAspect);
-    }
-  }
 }
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 336056fad1..318b6683be 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
@@ -15,7 +15,6 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.base.Objects;
-import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.analysis.Aspect;
 import com.google.devtools.build.lib.analysis.AspectWithParameters;
@@ -38,65 +37,59 @@ import javax.annotation.Nullable;
 public final class AspectValue extends ActionLookupValue {
 
   /**
-   * A base class for a key representing an aspect applied to a particular target.
+   * A base class for keys that have AspectValue as a Sky value.
    */
-  public abstract static class AspectKey extends ActionLookupKey {
-    protected final Label label;
-    protected final BuildConfiguration configuration;
-
-    protected AspectKey(Label label, BuildConfiguration configuration) {
-      this.label = label;
-      this.configuration = configuration;
-    }
-
-    @Override
-    public Label getLabel() {
-      return label;
-    }
-
-    public abstract AspectParameters getParameters();
+  public abstract static class AspectValueKey extends ActionLookupKey {
 
     public abstract String getDescription();
-
-    public BuildConfiguration getConfiguration() {
-      return configuration;
-    }
   }
 
   /**
-   * The key of an action that is generated by a native aspect.
+   * A base class for a key representing an aspect applied to a particular target.
    */
-  public static final class NativeAspectKey extends AspectKey {
+  public static final class AspectKey extends AspectValueKey {
+    private final Label label;
+    private final BuildConfiguration configuration;
     private final AspectWithParameters aspect;
+    private final String aspectName;
 
-    private NativeAspectKey(
+    protected AspectKey(
         Label label,
         BuildConfiguration configuration,
-        AspectClass aspectClass ,
+        AspectClass aspectClass,
         AspectParameters parameters) {
-      super(label, configuration);
-      Preconditions.checkNotNull(parameters);
+      this.label = label;
+      this.configuration = configuration;
+      this.aspectName = aspectClass.getName();
       this.aspect = new AspectWithParameters(aspectClass, parameters);
     }
 
+    @Override
+    SkyFunctionName getType() {
+      return SkyFunctions.ASPECT;
+    }
+
+
+    @Override
+    public Label getLabel() {
+      return label;
+    }
+
     public AspectClass getAspect() {
       return aspect.getAspectClass();
     }
 
-    @Override
     @Nullable
     public AspectParameters getParameters() {
       return aspect.getParameters();
     }
 
-    @Override
     public String getDescription() {
       return String.format("%s of %s", aspect.getAspectClass().getName(), getLabel());
     }
 
-    @Override
-    SkyFunctionName getType() {
-      return SkyFunctions.NATIVE_ASPECT;
+    public BuildConfiguration getConfiguration() {
+      return configuration;
     }
 
     @Override
@@ -110,11 +103,11 @@ public final class AspectValue extends ActionLookupValue {
         return true;
       }
 
-      if (!(other instanceof NativeAspectKey)) {
+      if (!(other instanceof AspectKey)) {
         return false;
       }
 
-      NativeAspectKey that = (NativeAspectKey) other;
+      AspectKey that = (AspectKey) other;
       return Objects.equal(label, that.label)
           && Objects.equal(configuration, that.configuration)
           && Objects.equal(aspect, that.aspect);
@@ -129,20 +122,30 @@ public final class AspectValue extends ActionLookupValue {
   }
 
   /**
-   * The key of an action that is generated by a skylark aspect.
+   * The key for a skylark aspect.
    */
-  public static class SkylarkAspectKey extends AspectKey {
+  public static class SkylarkAspectLoadingKey extends AspectValueKey {
+
+    private final Label targetLabel;
+    private final BuildConfiguration targetConfiguration;
     private final PackageIdentifier extensionFile;
-    private final String skylarkFunctionName;
+    private final String skylarkValueName;
 
-    private SkylarkAspectKey(
+    private SkylarkAspectLoadingKey(
         Label targetLabel,
         BuildConfiguration targetConfiguration,
         PackageIdentifier extensionFile,
         String skylarkFunctionName) {
-      super(targetLabel, targetConfiguration);
+      this.targetLabel = targetLabel;
+      this.targetConfiguration = targetConfiguration;
+
       this.extensionFile = extensionFile;
-      this.skylarkFunctionName = skylarkFunctionName;
+      this.skylarkValueName = skylarkFunctionName;
+    }
+
+    @Override
+    SkyFunctionName getType() {
+      return SkyFunctions.LOAD_SKYLARK_ASPECT;
     }
 
     public PackageIdentifier getExtensionFile() {
@@ -150,24 +153,20 @@ public final class AspectValue extends ActionLookupValue {
     }
 
     public String getSkylarkValueName() {
-      return skylarkFunctionName;
+      return skylarkValueName;
     }
 
-    @Override
-    public AspectParameters getParameters() {
-      return AspectParameters.EMPTY;
+    public Label getTargetLabel() {
+      return targetLabel;
     }
 
-    @Override
-    public String getDescription() {
-      // Skylark aspects are referred to on command line with <file>%<value name>
-      return String.format(
-          "%s%%%s of %s", extensionFile.toString(), skylarkFunctionName, getLabel());
+    public BuildConfiguration getTargetConfiguration() {
+      return targetConfiguration;
     }
 
-    @Override
-    SkyFunctionName getType() {
-      return SkyFunctions.SKYLARK_ASPECT;
+    public String getDescription() {
+      // Skylark aspects are referred to on command line with <file>%<value ame>
+      return String.format("%s%%%s of %s", extensionFile.toString(), skylarkValueName, targetLabel);
     }
   }
 
@@ -215,24 +214,25 @@ public final class AspectValue extends ActionLookupValue {
       AspectClass aspectFactory,
       AspectParameters additionalConfiguration) {
     return new SkyKey(
-        SkyFunctions.NATIVE_ASPECT,
-        new NativeAspectKey(label, configuration, aspectFactory, additionalConfiguration));
+        SkyFunctions.ASPECT,
+        new AspectKey(label, configuration, aspectFactory, additionalConfiguration));
   }
 
-  public static SkyKey key(AspectKey aspectKey) {
+  public static SkyKey key(AspectValueKey aspectKey) {
     return new SkyKey(aspectKey.getType(), aspectKey);
   }
 
-  public static NativeAspectKey createAspectKey(
+  public static AspectKey createAspectKey(
       Label label, BuildConfiguration configuration, AspectClass aspectFactory) {
-    return new NativeAspectKey(label, configuration, aspectFactory, AspectParameters.EMPTY);
+    return new AspectKey(label, configuration, aspectFactory, AspectParameters.EMPTY);
   }
 
-  public static SkylarkAspectKey createSkylarkAspectKey(
+  public static SkylarkAspectLoadingKey createSkylarkAspectKey(
       Label targetLabel,
       BuildConfiguration targetConfiguration,
-      PackageIdentifier bzlFile,
-      String skylarkFunctionName) {
-    return new SkylarkAspectKey(targetLabel, targetConfiguration, bzlFile, skylarkFunctionName);
+      PackageIdentifier skylarkFile,
+      String skylarkExportName) {
+    return new SkylarkAspectLoadingKey(
+        targetLabel, targetConfiguration, skylarkFile, skylarkExportName);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index 237feea9d7..4493229290 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -47,7 +47,6 @@ import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.AspectClass;
 import com.google.devtools.build.lib.packages.AspectDefinition;
-import com.google.devtools.build.lib.packages.AspectFactory;
 import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
@@ -510,7 +509,8 @@ final class ConfiguredTargetFunction implements SkyFunction {
       }
       ConfiguredTarget depConfiguredTarget = configuredTargetMap.get(depKey);
       for (AspectWithParameters depAspect : dep.getAspects()) {
-        if (!aspectMatchesConfiguredTarget(depConfiguredTarget, depAspect.getAspectClass())) {
+        AspectClass depAspectClass = depAspect.getAspectClass();
+        if (!aspectMatchesConfiguredTarget(depConfiguredTarget, depAspectClass)) {
           continue;
         }
 
@@ -522,11 +522,12 @@ final class ConfiguredTargetFunction implements SkyFunction {
           // The configured target should have been created in resolveConfiguredTargetDependencies()
           throw new IllegalStateException(e);
         } catch (NoSuchThingException e) {
-          AspectFactory<?, ?, ?> depAspectFactory =
-              AspectFactory.Util.create(depAspect.getAspectClass());
           throw new AspectCreationException(
-              String.format("Evaluation of aspect %s on %s failed: %s",
-                  depAspectFactory.getDefinition().getName(), dep.getLabel(), e.toString()));
+              String.format(
+                  "Evaluation of aspect %s on %s failed: %s",
+                  depAspectClass.getDefinition().getName(),
+                  dep.getLabel(),
+                  e.toString()));
         }
 
         if (aspectValue == null) {
@@ -550,7 +551,7 @@ final class ConfiguredTargetFunction implements SkyFunction {
 
   private static boolean aspectMatchesConfiguredTarget(
       ConfiguredTarget dep, AspectClass aspectClass) {
-    AspectDefinition aspectDefinition = AspectFactory.Util.create(aspectClass).getDefinition();
+    AspectDefinition aspectDefinition = aspectClass.getDefinition();
     for (Class<?> provider : aspectDefinition.getRequiredProviders()) {
       if (dep.getProvider(provider.asSubclass(TransitiveInfoProvider.class)) == null) {
         return false;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
index ee3d395841..b04121163e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
@@ -65,8 +65,9 @@ public final class SkyFunctions {
       SkyFunctionName.create("CONFIGURED_TARGET");
   public static final SkyFunctionName POST_CONFIGURED_TARGET =
       SkyFunctionName.create("POST_CONFIGURED_TARGET");
-  public static final SkyFunctionName NATIVE_ASPECT = SkyFunctionName.create("NATIVE_ASPECT");
-  public static final SkyFunctionName SKYLARK_ASPECT = SkyFunctionName.create("SKYLARK_ASPECT");
+  public static final SkyFunctionName ASPECT = SkyFunctionName.create("ASPECT");
+  public static final SkyFunctionName LOAD_SKYLARK_ASPECT =
+      SkyFunctionName.create("LOAD_SKYLARK_ASPECT");
   public static final SkyFunctionName TARGET_COMPLETION =
       SkyFunctionName.create("TARGET_COMPLETION");
   public static final SkyFunctionName ASPECT_COMPLETION =
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index f7a85e6902..e1dc3a025a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -63,7 +63,7 @@ import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.pkgcache.LoadingPhaseRunner;
 import com.google.devtools.build.lib.skyframe.ActionLookupValue.ActionLookupKey;
 import com.google.devtools.build.lib.skyframe.AspectFunction.AspectCreationException;
-import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
+import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey;
 import com.google.devtools.build.lib.skyframe.BuildInfoCollectionValue.BuildInfoKeyAndConfig;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredValueCreationException;
 import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ConflictException;
@@ -238,7 +238,7 @@ public final class SkyframeBuildView {
   public SkyframeAnalysisResult configureTargets(
       EventHandler eventHandler,
       List<ConfiguredTargetKey> values,
-      List<AspectKey> aspectKeys,
+      List<AspectValueKey> aspectKeys,
       EventBus eventBus,
       boolean keepGoing)
       throws InterruptedException, ViewCreationFailedException {
@@ -253,7 +253,7 @@ public final class SkyframeBuildView {
 
     Collection<AspectValue> goodAspects = Lists.newArrayListWithCapacity(values.size());
     NestedSetBuilder<Package> packages = NestedSetBuilder.stableOrder();
-    for (AspectKey aspectKey : aspectKeys) {
+    for (AspectValueKey aspectKey : aspectKeys) {
       AspectValue value = (AspectValue) result.get(AspectValue.key(aspectKey));
       if (value == null) {
         // Skip aspects that couldn't be applied to targets.
@@ -320,8 +320,8 @@ public final class SkyframeBuildView {
             "Analysis of target '"
                 + ConfiguredTargetValue.extractLabel(topLevel)
                 + "' failed; build aborted";
-      } else if (topLevel.argument() instanceof AspectKey) {
-        AspectKey aspectKey = (AspectKey) topLevel.argument();
+      } else if (topLevel.argument() instanceof AspectValueKey) {
+        AspectValueKey aspectKey = (AspectValueKey) topLevel.argument();
         errorMsg = "Analysis of aspect '" + aspectKey.getDescription() + "' failed; build aborted";
       } else {
         assert false;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 1b5dfd8468..f13505b3d4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -92,7 +92,7 @@ import com.google.devtools.build.lib.pkgcache.PackageManager;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
 import com.google.devtools.build.lib.pkgcache.TransitivePackageLoader;
 import com.google.devtools.build.lib.profiler.AutoProfiler;
-import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
+import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey;
 import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.FileDirtinessChecker;
 import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ActionCompletedReceiver;
 import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ProgressSupplier;
@@ -351,12 +351,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     map.put(SkyFunctions.TRANSITIVE_TRAVERSAL, new TransitiveTraversalFunction());
     map.put(SkyFunctions.CONFIGURED_TARGET,
         new ConfiguredTargetFunction(new BuildViewProvider(), ruleClassProvider));
-    map.put(
-        SkyFunctions.NATIVE_ASPECT,
-        AspectFunction.createNativeAspectFunction(new BuildViewProvider(), ruleClassProvider));
-    map.put(
-        SkyFunctions.SKYLARK_ASPECT,
-        AspectFunction.createSkylarkAspectFunction(new BuildViewProvider(), ruleClassProvider));
+    map.put(SkyFunctions.ASPECT, new AspectFunction(new BuildViewProvider(), ruleClassProvider));
+    map.put(SkyFunctions.LOAD_SKYLARK_ASPECT, new ToplevelSkylarkAspectFunction());
     map.put(SkyFunctions.POST_CONFIGURED_TARGET,
         new PostConfiguredTargetFunction(new BuildViewProvider(), ruleClassProvider));
     map.put(SkyFunctions.BUILD_CONFIGURATION,
@@ -1320,13 +1316,16 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   }
 
   /** Configures a given set of configured targets. */
-  public EvaluationResult<ActionLookupValue> configureTargets(EventHandler eventHandler,
-      List<ConfiguredTargetKey> values, List<AspectKey> aspectKeys, boolean keepGoing)
-          throws InterruptedException {
+  public EvaluationResult<ActionLookupValue> configureTargets(
+      EventHandler eventHandler,
+      List<ConfiguredTargetKey> values,
+      List<AspectValueKey> aspectKeys,
+      boolean keepGoing)
+      throws InterruptedException {
     checkActive();
 
     List<SkyKey> keys = new ArrayList<>(ConfiguredTargetValue.keys(values));
-    for (AspectKey aspectKey : aspectKeys) {
+    for (AspectValueKey aspectKey : aspectKeys) {
       keys.add(AspectValue.key(aspectKey));
     }
     // Make sure to not run too many analysis threads. This can cause memory thrashing.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
index 916803a795..c8775c97c3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
@@ -21,7 +21,6 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.SkylarkProviderValidationUtil;
 import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.packages.AspectDefinition;
 import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspect;
 import com.google.devtools.build.lib.rules.SkylarkRuleContext;
@@ -37,11 +36,11 @@ import com.google.devtools.build.lib.syntax.Mutability;
 public class SkylarkAspectFactory implements ConfiguredAspectFactory {
 
   private final String name;
-  private final SkylarkAspect aspectFunction;
+  private final SkylarkAspect skylarkAspect;
 
-  public SkylarkAspectFactory(String name, SkylarkAspect aspectFunction) {
+  public SkylarkAspectFactory(String name, SkylarkAspect skylarkAspect) {
     this.name = name;
-    this.aspectFunction = aspectFunction;
+    this.skylarkAspect = skylarkAspect;
   }
 
   @Override
@@ -58,14 +57,14 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory {
       Environment env =
           Environment.builder(mutability)
               .setSkylark()
-              .setGlobals(aspectFunction.getFuncallEnv().getGlobals())
+              .setGlobals(skylarkAspect.getFuncallEnv().getGlobals())
               .setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler())
               .build(); // NB: loading phase functions are not available: this is analysis already,
                         // so we do *not* setLoadingPhase().
       Object aspectSkylarkObject;
       try {
         aspectSkylarkObject =
-            aspectFunction
+            skylarkAspect
                 .getImplementation()
                 .call(
                     ImmutableList.<Object>of(base, skylarkRuleContext),
@@ -105,12 +104,7 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory {
           .registerPhantomFuncall(
               String.format("%s(...)", name),
               base.getTarget().getAssociatedRule().getLocation(),
-              aspectFunction.getImplementation());
+              skylarkAspect.getImplementation());
     }
   }
-
-  @Override
-  public AspectDefinition getDefinition() {
-    return new AspectDefinition.Builder(name).build();
-  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
index c0c72f05e2..bba850b316 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
@@ -177,7 +177,7 @@ public class SkylarkImportLookupFunction implements SkyFunction {
 
     // Skylark UserDefinedFunction-s in that file will share this function definition Environment,
     // which will be frozen by the time it is returned by createExtension.
-    Extension extension = createExtension(ast, file, importMap, env);
+    Extension extension = createExtension(ast, pkgId, importMap, env);
 
     return new SkylarkImportLookupValue(
         extension, new SkylarkFileDependency(label, fileDependencies.build()));
@@ -229,26 +229,27 @@ public class SkylarkImportLookupFunction implements SkyFunction {
    */
   private Extension createExtension(
       BuildFileAST ast,
-      PathFragment file,
+      PackageIdentifier packageIdentifier,
       Map<PathFragment, Extension> importMap,
       Environment env)
-          throws InterruptedException, SkylarkImportLookupFunctionException {
+      throws InterruptedException, SkylarkImportLookupFunctionException {
     StoredEventHandler eventHandler = new StoredEventHandler();
     // TODO(bazel-team): this method overestimates the changes which can affect the
     // Skylark RuleClass. For example changes to comments or unused functions can modify the hash.
     // A more accurate - however much more complicated - way would be to calculate a hash based on
     // the transitive closure of the accessible AST nodes.
-    try (Mutability mutability = Mutability.create("importing %s", file)) {
+    try (Mutability mutability = Mutability.create("importing %s", packageIdentifier)) {
       com.google.devtools.build.lib.syntax.Environment extensionEnv =
           ruleClassProvider.createSkylarkRuleClassEnvironment(
               mutability, eventHandler, ast.getContentHashCode(), importMap)
           .setupOverride("native", packageFactory.getNativeModule());
       ast.exec(extensionEnv, eventHandler);
-      SkylarkRuleClassFunctions.exportRuleFunctions(extensionEnv, file);
+      SkylarkRuleClassFunctions.exportRuleFunctionsAndAspects(extensionEnv, packageIdentifier);
 
       Event.replayEventsOn(env.getListener(), eventHandler.getEvents());
       if (eventHandler.hasErrors()) {
-        throw new SkylarkImportLookupFunctionException(SkylarkImportFailedException.errors(file));
+        throw new SkylarkImportLookupFunctionException(
+            SkylarkImportFailedException.errors(packageIdentifier.getPackageFragment()));
       }
       return new Extension(extensionEnv);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
new file mode 100644
index 0000000000..fabb1bb058
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
@@ -0,0 +1,85 @@
+// Copyright 2015 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.skyframe;
+
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.packages.AspectParameters;
+import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspect;
+import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspectClass;
+import com.google.devtools.build.lib.skyframe.ASTFileLookupValue.ASTLookupInputException;
+import com.google.devtools.build.lib.skyframe.AspectValue.SkylarkAspectLoadingKey;
+import com.google.devtools.build.lib.syntax.Type.ConversionException;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionException;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+
+import javax.annotation.Nullable;
+
+/**
+ * SkyFunction to load aspects from Skylark extensions and calculate their values.
+ *
+ * Used for loading top-level aspects. At top level, in
+ * {@link com.google.devtools.build.lib.analysis.BuildView}, we cannot invoke two SkyFunctions
+ * one aftre another, so BuildView call this function to do the work.
+ */
+public class ToplevelSkylarkAspectFunction implements SkyFunction {
+
+  @Nullable
+  @Override
+  public SkyValue compute(SkyKey skyKey, Environment env)
+      throws LoadSkylarkAspectFunctionException, InterruptedException {
+    SkylarkAspectLoadingKey aspectLoadingKey = (SkylarkAspectLoadingKey) skyKey.argument();
+    String skylarkValueName = aspectLoadingKey.getSkylarkValueName();
+    PackageIdentifier extensionFile = aspectLoadingKey.getExtensionFile();
+    SkylarkAspect skylarkAspect = null;
+    try {
+      skylarkAspect = AspectFunction.loadSkylarkAspect(env, extensionFile, skylarkValueName);
+    } catch (ASTLookupInputException | ConversionException e) {
+      throw new LoadSkylarkAspectFunctionException(e, skyKey);
+    }
+    if (skylarkAspect == null) {
+      return null;
+    }
+    SkyKey aspectKey =
+        AspectValue.key(
+            aspectLoadingKey.getTargetLabel(),
+            aspectLoadingKey.getTargetConfiguration(),
+            new SkylarkAspectClass(skylarkAspect),
+            AspectParameters.EMPTY);
+
+    return env.getValue(aspectKey);
+  }
+
+  @Nullable
+  @Override
+  public String extractTag(SkyKey skyKey) {
+    return null;
+  }
+
+  /**
+   * Exceptions thrown from ToplevelSkylarkAspectFunction.
+   */
+  public class LoadSkylarkAspectFunctionException extends SkyFunctionException {
+
+    public LoadSkylarkAspectFunctionException(Exception cause, SkyKey childKey) {
+      super(cause, childKey);
+    }
+
+    public LoadSkylarkAspectFunctionException(Exception cause) {
+      super(cause, Transience.PERSISTENT);
+    }
+  }
+}