Implement aspect(...) Skylark function.

--
MOS_MIGRATED_REVID=105844221

17 files changed, 514 insertions, 94 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Aspect.java b/src/main/java/com/google/devtools/build/lib/analysis/Aspect.java
index fe97bfc3b4..bb6c38d9a4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Aspect.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Aspect.java
@@ -22,6 +22,7 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.events.Location;
 
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -88,6 +89,8 @@ public final class Aspect implements Iterable<TransitiveInfoProvider> {
     private final Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider>
         providers = new LinkedHashMap<>();
     private final Map<String, NestedSetBuilder<Artifact>> outputGroupBuilders = new TreeMap<>();
+    private final ImmutableMap.Builder<String, Object> skylarkProviderBuilder =
+        ImmutableMap.builder();
     private final String name;
 
     public Builder(String name) {
@@ -103,6 +106,8 @@ public final class Aspect implements Iterable<TransitiveInfoProvider> {
       Preconditions.checkNotNull(value);
       AnalysisUtils.checkProvider(key);
       Preconditions.checkState(!providers.containsKey(key));
+      Preconditions.checkArgument(!SkylarkProviders.class.equals(key),
+          "Do not provide SkylarkProviders directly");
       providers.put(key, value);
       return this;
     }
@@ -127,6 +132,12 @@ public final class Aspect implements Iterable<TransitiveInfoProvider> {
       return this;
     }
 
+    public Builder addSkylarkTransitiveInfo(String name, Object value, Location loc) {
+      // TODO(dslomov): add {@link RuleConfiguredTargetBuilder#checkSkylarkObjectSafe}
+      skylarkProviderBuilder.put(name, value);
+      return this;
+    }
+
     public Aspect build() {
       if (!outputGroupBuilders.isEmpty()) {
         ImmutableMap.Builder<String, NestedSet<Artifact>> outputGroups = ImmutableMap.builder();
@@ -141,6 +152,11 @@ public final class Aspect implements Iterable<TransitiveInfoProvider> {
         addProvider(OutputGroupProvider.class, new OutputGroupProvider(outputGroups.build()));
       }
 
+      ImmutableMap<String, Object> skylarkProvidersMap = skylarkProviderBuilder.build();
+      if (!skylarkProvidersMap.isEmpty()) {
+        providers.put(SkylarkProviders.class, new SkylarkProviders(skylarkProvidersMap));
+      }
+
       return new Aspect(name, ImmutableMap.copyOf(providers));
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 14302ac179..e05dd596d9 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -42,6 +42,7 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -75,6 +76,7 @@ import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.util.RegexFilter;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.WalkableGraph;
 import com.google.devtools.common.options.Option;
@@ -450,18 +452,43 @@ public class BuildView {
 
     List<AspectKey> aspectKeys = new ArrayList<>();
     for (String aspect : aspects) {
-      @SuppressWarnings("unchecked")
-      final Class<? extends ConfiguredAspectFactory> aspectFactoryClass =
-          (Class<? extends ConfiguredAspectFactory>)
-              ruleClassProvider.getAspectFactoryMap().get(aspect);
-      if (aspectFactoryClass != null) {
+
+      // Syntax: label%aspect
+      int delimiterPosition = aspect.indexOf('%');
+      if (delimiterPosition >= 0) {
+        PackageIdentifier bzlFile;
+        try {
+          bzlFile =
+              PackageIdentifier.create(
+                  PackageIdentifier.DEFAULT_REPOSITORY,
+                  new PathFragment(aspect.substring(0, delimiterPosition)));
+        } catch (LabelSyntaxException e) {
+          throw new ViewCreationFailedException("Error", e);
+        }
+
+        String skylarkFunctionName = aspect.substring(delimiterPosition + 1);
         for (ConfiguredTargetKey targetSpec : targetSpecs) {
           aspectKeys.add(
-              AspectValue.createAspectKey(
-                  targetSpec.getLabel(), targetSpec.getConfiguration(), aspectFactoryClass));
+              AspectValue.createSkylarkAspectKey(
+                  targetSpec.getLabel(),
+                  targetSpec.getConfiguration(),
+                  bzlFile,
+                  skylarkFunctionName));
         }
       } else {
-        throw new ViewCreationFailedException("Aspect '" + aspect + "' is unknown");
+        @SuppressWarnings("unchecked")
+        final Class<? extends ConfiguredAspectFactory> aspectFactoryClass =
+            (Class<? extends ConfiguredAspectFactory>)
+                ruleClassProvider.getAspectFactoryMap().get(aspect);
+        if (aspectFactoryClass != null) {
+          for (ConfiguredTargetKey targetSpec : targetSpecs) {
+            aspectKeys.add(
+                AspectValue.createAspectKey(
+                    targetSpec.getLabel(), targetSpec.getConfiguration(), aspectFactoryClass));
+          }
+        } else {
+          throw new ViewCreationFailedException("Aspect '" + aspect + "' is unknown");
+        }
       }
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index 6d97e86c42..a8cc9e4932 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -286,12 +286,14 @@ public final class ConfiguredTargetFactory {
    * {@code aspectFactory} should call one of the error reporting methods of {@link RuleContext}.
    */
   public Aspect createAspect(
-      AnalysisEnvironment env, RuleConfiguredTarget associatedTarget,
+      AnalysisEnvironment env,
+      RuleConfiguredTarget associatedTarget,
       ConfiguredAspectFactory aspectFactory,
       AspectParameters aspectParameters,
       ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
       Set<ConfigMatchingProvider> configConditions,
-      BuildConfiguration hostConfiguration) {
+      BuildConfiguration hostConfiguration)
+      throws InterruptedException {
     RuleContext.Builder builder = new RuleContext.Builder(env,
         associatedTarget.getTarget(),
         associatedTarget.getConfiguration(),
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
index 9bb9365d80..d8614597a4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
@@ -22,7 +22,6 @@ import com.google.common.collect.UnmodifiableIterator;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
 import com.google.devtools.build.lib.analysis.config.RunUnder;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.OutputFile;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.SkylarkApiProvider;
@@ -131,20 +130,14 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
     providers.addAll(base.providers.keySet());
 
     // Merge output group providers.
-    OutputGroupProvider baseOutputGroupProvider = base.getProvider(OutputGroupProvider.class);
-    List<OutputGroupProvider> outputGroupProviders = new ArrayList<>();
-    if (baseOutputGroupProvider != null) {
-      outputGroupProviders.add(baseOutputGroupProvider);
-    }
+    List<OutputGroupProvider> outputGroupProviders =
+        getAllProviders(base, aspects, OutputGroupProvider.class);
+    OutputGroupProvider mergedOutputGroupProvider = OutputGroupProvider.merge(outputGroupProviders);
 
-    for (Aspect aspect : aspects) {
-      final OutputGroupProvider aspectProvider = aspect.getProvider(OutputGroupProvider.class);
-      if (aspectProvider == null) {
-        continue;
-      }
-      outputGroupProviders.add(aspectProvider);
-    }
-    OutputGroupProvider outputGroupProvider = OutputGroupProvider.merge(outputGroupProviders);
+    // Merge Skylark providers.
+    List<SkylarkProviders> skylarkProviders =
+        getAllProviders(base, aspects, SkylarkProviders.class);
+    SkylarkProviders mergedSkylarkProviders = SkylarkProviders.merge(skylarkProviders);
 
     // Validate that all other providers are only provided once.
     for (Aspect aspect : aspects) {
@@ -153,13 +146,17 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
         if (OutputGroupProvider.class.equals(aClass)) {
           continue;
         }
+        if (SkylarkProviders.class.equals(aClass)) {
+          continue;
+        }
         if (!providers.add(aClass)) {
           throw new IllegalStateException("Provider " + aClass + " provided twice");
         }
       }
     }
 
-    if (baseOutputGroupProvider == outputGroupProvider) {
+    if (base.getProvider(OutputGroupProvider.class) == mergedOutputGroupProvider
+        && base.getProvider(SkylarkProviders.class) == mergedSkylarkProviders) {
       this.providers = base.providers;
     } else {
       ImmutableMap.Builder<Class<? extends TransitiveInfoProvider>, Object> builder =
@@ -168,9 +165,17 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
         if (OutputGroupProvider.class.equals(aClass)) {
           continue;
         }
+        if (SkylarkProviders.class.equals(aClass)) {
+          continue;
+        }
         builder.put(aClass, base.providers.get(aClass));
       }
-      builder.put(OutputGroupProvider.class, outputGroupProvider);
+      if (mergedOutputGroupProvider != null) {
+        builder.put(OutputGroupProvider.class, mergedOutputGroupProvider);
+      }
+      if (mergedSkylarkProviders != null) {
+        builder.put(SkylarkProviders.class, skylarkProviders);
+      }
       this.providers = builder.build();
     }
     this.mandatoryStampFiles = base.mandatoryStampFiles;
@@ -178,6 +183,26 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
     this.aspects = ImmutableList.copyOf(aspects);
   }
 
+  private static <T extends TransitiveInfoProvider> List<T> getAllProviders(
+      RuleConfiguredTarget base,
+      Iterable<Aspect> aspects,
+      Class<T> providerClass) {
+    T baseProvider = base.getProvider(providerClass);
+    List<T> providers = new ArrayList<>();
+    if (baseProvider != null) {
+      providers.add(baseProvider);
+    }
+
+    for (Aspect aspect : aspects) {
+      final T aspectProvider = aspect.getProvider(providerClass);
+      if (aspectProvider == null) {
+        continue;
+      }
+      providers.add(aspectProvider);
+    }
+    return providers;
+  }
+
   /**
    * The configuration conditions that trigger this rule's configurable attributes.
    */
@@ -208,7 +233,7 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
    */
   @Override
   public Object get(String providerKey) {
-    return getProvider(SkylarkProviders.class).skylarkProviders.get(providerKey);
+    return getProvider(SkylarkProviders.class).getValue(providerKey);
   }
 
   public ImmutableList<Artifact> getMandatoryStampFiles() {
@@ -220,33 +245,6 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
     return (Rule) super.getTarget();
   }
 
-  /**
-   * A helper class for transitive infos provided by Skylark rule implementations.
-   */
-  @Immutable
-  public static final class SkylarkProviders implements TransitiveInfoProvider {
-    private final ImmutableMap<String, Object> skylarkProviders;
-
-    private SkylarkProviders(ImmutableMap<String, Object> skylarkProviders) {
-      Preconditions.checkNotNull(skylarkProviders);
-      this.skylarkProviders = skylarkProviders;
-    }
-
-    /**
-     * Returns the keys for the Skylark providers.
-     */
-    public ImmutableCollection<String> getKeys() {
-      return skylarkProviders.keySet();
-    }
-
-    /**
-     * Returns a Skylark provider; "key" must be one from {@link #getKeys()}.
-     */
-    public Object getValue(String key) {
-      return skylarkProviders.get(key);
-    }
-  }
-
   @Override
   public UnmodifiableIterator<TransitiveInfoProvider> iterator() {
     Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> allProviders =
@@ -273,6 +271,6 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
   @Override
   public ImmutableCollection<String> getKeys() {
     return ImmutableList.<String>builder().addAll(super.getKeys())
-        .addAll(getProvider(SkylarkProviders.class).skylarkProviders.keySet()).build();
+        .addAll(getProvider(SkylarkProviders.class).getKeys()).build();
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
new file mode 100644
index 0000000000..d4746c4659
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
@@ -0,0 +1,79 @@
+// Copyright 2014 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.analysis;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableCollection;
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * A helper class for transitive infos provided by Skylark rule implementations.
+ */
+@Immutable
+public final class SkylarkProviders implements TransitiveInfoProvider {
+  private final ImmutableMap<String, Object> skylarkProviders;
+
+  SkylarkProviders(ImmutableMap<String, Object> skylarkProviders) {
+    Preconditions.checkNotNull(skylarkProviders);
+    this.skylarkProviders = skylarkProviders;
+  }
+
+  /**
+   * Returns the keys for the Skylark providers.
+   */
+  public ImmutableCollection<String> getKeys() {
+    return skylarkProviders.keySet();
+  }
+
+  /**
+   * Returns a Skylark provider; "key" must be one from {@link #getKeys()}.
+   */
+  public Object getValue(String key) {
+    return skylarkProviders.get(key);
+  }
+
+  /**
+   * Merges skylark providers. The set of providers must be disjoint.
+   *
+   * @param providers providers to merge {@code this} with.
+   */
+
+  public static SkylarkProviders merge(List<SkylarkProviders> providers) {
+    if (providers.size() == 0) {
+      return null;
+    }
+    if (providers.size() == 1) {
+      return providers.get(0);
+    }
+
+    ImmutableMap.Builder<String, Object> resultBuilder = new ImmutableMap.Builder<>();
+    Set<String> seenKeys = new HashSet<>();
+    for (SkylarkProviders provider : providers) {
+      for (String key : provider.skylarkProviders.keySet()) {
+        if (!seenKeys.add(key)) {
+          // TODO(dslomov): add better diagnostics.
+          throw new IllegalStateException("Skylark provider " + key + " provided twice");
+        }
+
+        resultBuilder.put(key, provider.getValue(key));
+      }
+    }
+    return new SkylarkProviders(resultBuilder.build());
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java b/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java
index eea286d607..cd7ba2c51f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java
@@ -29,7 +29,8 @@ public interface AspectFactory<TConfiguredTarget, TRuleContext, TAspect> {
    * @param parameters information from attributes of the rule that have requested this
    *     aspect
    */
-  TAspect create(TConfiguredTarget base, TRuleContext context, AspectParameters parameters);
+  TAspect create(TConfiguredTarget base, TRuleContext context, AspectParameters parameters)
+      throws InterruptedException;
 
   /**
    * Returns the definition of the aspect.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index 87b570318c..90d351859f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -69,6 +69,7 @@ import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.EvalUtils;
 import com.google.devtools.build.lib.syntax.FuncallExpression;
 import com.google.devtools.build.lib.syntax.FunctionSignature;
+import com.google.devtools.build.lib.syntax.Printer;
 import com.google.devtools.build.lib.syntax.Runtime;
 import com.google.devtools.build.lib.syntax.SkylarkCallbackFunction;
 import com.google.devtools.build.lib.syntax.SkylarkList;
@@ -76,6 +77,7 @@ import com.google.devtools.build.lib.syntax.SkylarkModuleNameResolver;
 import com.google.devtools.build.lib.syntax.SkylarkSignature;
 import com.google.devtools.build.lib.syntax.SkylarkSignature.Param;
 import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
+import com.google.devtools.build.lib.syntax.SkylarkValue;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.syntax.Type.ConversionException;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -329,6 +331,21 @@ public class SkylarkRuleClassFunctions {
     }
   };
 
+  @SkylarkSignature(
+    name = "aspect",
+    returnType = SkylarkAspect.class,
+    documented = false, // TODO(dslomov): Experimental, document later.
+    mandatoryPositionals = {@Param(name = "implementation", type = BaseFunction.class)},
+    useEnvironment = true
+  )
+  private static final BuiltinFunction aspect =
+      new BuiltinFunction("aspect") {
+        public SkylarkAspect invoke(BaseFunction implementation, Environment funcallEnv) {
+          return new SkylarkAspect(implementation, funcallEnv);
+        }
+      };
+
+
   // This class is needed for testing
   public static final class RuleFunction extends BaseFunction {
     // Note that this means that we can reuse the same builder.
@@ -516,4 +533,36 @@ public class SkylarkRuleClassFunctions {
   static {
     SkylarkSignatureProcessor.configureSkylarkFunctions(SkylarkRuleClassFunctions.class);
   }
+
+  /**
+   * A Skylark value that is a result of 'aspect(..)' function call.
+   */
+  public static class SkylarkAspect implements SkylarkValue {
+    private final BaseFunction implementation;
+    private final Environment funcallEnv;
+
+    public SkylarkAspect(BaseFunction implementation, Environment funcallEnv) {
+      this.implementation = implementation;
+      this.funcallEnv = funcallEnv;
+    }
+
+    public BaseFunction getImplementation() {
+      return implementation;
+    }
+
+    public Environment getFuncallEnv() {
+      return funcallEnv;
+    }
+
+    @Override
+    public boolean isImmutable() {
+      return implementation.isImmutable();
+    }
+
+    @Override
+    public void write(Appendable buffer, char quotationMark) {
+      Printer.append(buffer, "Aspect:");
+      implementation.write(buffer, quotationMark);
+    }
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
index 71619eb4ea..9e7ba336e5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
@@ -62,7 +62,8 @@ public final class SkylarkRuleConfiguredTargetBuilder {
           .setGlobals(
               ruleContext.getRule().getRuleClassObject().getRuleDefinitionEnvironment().getGlobals())
           .setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler())
-          .build(); // NB: we do *not* setLoadingPhase()
+          .build(); // NB: loading phase functions are not available: this is analysis already,
+                    // so we do *not* setLoadingPhase().
       Object target = ruleImplementation.call(
           ImmutableList.<Object>of(skylarkRuleContext),
           ImmutableMap.<String, Object>of(),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/workspace/Bind.java b/src/main/java/com/google/devtools/build/lib/rules/workspace/Bind.java
index 3ffbedeac7..a0dc80b2da 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/workspace/Bind.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/workspace/Bind.java
@@ -20,9 +20,9 @@ import com.google.common.collect.UnmodifiableIterator;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.FileProvider;
-import com.google.devtools.build.lib.analysis.RuleConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.SkylarkProviders;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -111,7 +111,7 @@ public class Bind implements RuleConfiguredTargetFactory {
       ImmutableList.Builder<String> result = ImmutableList.<String>builder().add("label", "files");
       if (configuredTarget != null) {
           result.addAll(
-              configuredTarget.getProvider(RuleConfiguredTarget.SkylarkProviders.class).getKeys());
+              configuredTarget.getProvider(SkylarkProviders.class).getKeys());
       }
       return result.build();
     }
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 df627ce9e7..57dc717f00 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
@@ -35,9 +35,14 @@ 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.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;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -50,13 +55,30 @@ import javax.annotation.Nullable;
 /**
  * The Skyframe function that generates aspects.
  */
-public final class AspectFunction implements SkyFunction {
+public final class AspectFunction<T extends AspectKey> implements SkyFunction {
   private final BuildViewProvider buildViewProvider;
   private final RuleClassProvider ruleClassProvider;
+  private final AspectFactoryCreator<T> aspectFactoryCreator;
 
-  public AspectFunction(BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
+  public static AspectFunction<NativeAspectKey> createNativeAspectFunction(
+      BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
+    return new AspectFunction<>(
+        buildViewProvider, ruleClassProvider, new NativeAspectFactoryCreator());
+  }
+
+  public static AspectFunction<SkylarkAspectKey> createSkylarkAspectFunction(
+      BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
+    return new AspectFunction<>(
+        buildViewProvider, ruleClassProvider, new SkylarkAspectFactoryCreator());
+  }
+
+  private AspectFunction(
+      BuildViewProvider buildViewProvider,
+      RuleClassProvider ruleClassProvider,
+      AspectFactoryCreator<T> aspectFactoryCreator) {
     this.buildViewProvider = buildViewProvider;
     this.ruleClassProvider = ruleClassProvider;
+    this.aspectFactoryCreator = aspectFactoryCreator;
   }
 
   @Nullable
@@ -66,8 +88,7 @@ public final class AspectFunction implements SkyFunction {
     SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
     NestedSetBuilder<Package> transitivePackages = NestedSetBuilder.stableOrder();
     AspectKey key = (AspectKey) skyKey.argument();
-    ConfiguredAspectFactory aspectFactory =
-        (ConfiguredAspectFactory) AspectFactory.Util.create(key.getAspect());
+    ConfiguredAspectFactory aspectFactory = aspectFactoryCreator.createAspectFactory(skyKey, env);
 
     PackageValue packageValue =
         (PackageValue) env.getValue(PackageValue.key(key.getLabel().getPackageIdentifier()));
@@ -132,7 +153,13 @@ public final class AspectFunction implements SkyFunction {
               ruleClassProvider, view.getHostConfiguration(ctgValue.getConfiguration()),
               transitivePackages);
 
-      return createAspect(env, key, associatedTarget, configConditions, depValueMap,
+      return createAspect(
+          env,
+          key,
+          aspectFactory,
+          associatedTarget,
+          configConditions,
+          depValueMap,
           transitivePackages);
     } catch (DependencyEvaluationException e) {
       throw new AspectFunctionException(e.getRootCauseSkyKey(), e.getCause());
@@ -142,11 +169,15 @@ public final class AspectFunction implements SkyFunction {
   }
 
   @Nullable
-  private AspectValue createAspect(Environment env, AspectKey key,
-      RuleConfiguredTarget associatedTarget, Set<ConfigMatchingProvider> configConditions,
+  private AspectValue createAspect(
+      Environment env,
+      AspectKey key,
+      ConfiguredAspectFactory aspectFactory,
+      RuleConfiguredTarget associatedTarget,
+      Set<ConfigMatchingProvider> configConditions,
       ListMultimap<Attribute, ConfiguredTarget> directDeps,
       NestedSetBuilder<Package> transitivePackages)
-      throws AspectFunctionException {
+      throws AspectFunctionException, InterruptedException {
 
     SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
     BuildConfiguration configuration = associatedTarget.getConfiguration();
@@ -158,8 +189,6 @@ public final class AspectFunction implements SkyFunction {
       return null;
     }
 
-    ConfiguredAspectFactory aspectFactory =
-        (ConfiguredAspectFactory) AspectFactory.Util.create(key.getAspect());
     Aspect aspect = view.createAspect(
         analysisEnvironment, associatedTarget, aspectFactory, directDeps, configConditions,
         key.getParameters());
@@ -217,4 +246,58 @@ public final class AspectFunction implements SkyFunction {
       super(transitiveError, childKey);
     }
   }
+
+  /**
+   * 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 16277c7a47..5a4052fb8c 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
@@ -22,6 +22,7 @@ import com.google.devtools.build.lib.analysis.AspectWithParameters;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.AspectParameters;
@@ -35,21 +36,17 @@ import javax.annotation.Nullable;
  * An aspect in the context of the Skyframe graph.
  */
 public final class AspectValue extends ActionLookupValue {
+
   /**
-   * The key of an action that is generated by an aspect.
+   * A base class for a key representing an aspect applied to a particular target.
    */
-  public static final class AspectKey extends ActionLookupKey {
-    private final Label label;
-    private final BuildConfiguration configuration;
-    private final AspectWithParameters aspect;
+  public abstract static class AspectKey extends ActionLookupKey {
+    protected final Label label;
+    protected final BuildConfiguration configuration;
 
-    private AspectKey(Label label, BuildConfiguration configuration,
-        Class<? extends ConfiguredAspectFactory> aspectFactory,
-        AspectParameters parameters) {
-      Preconditions.checkNotNull(parameters);
+    protected AspectKey(Label label, BuildConfiguration configuration) {
       this.label = label;
       this.configuration = configuration;
-      this.aspect = new AspectWithParameters(aspectFactory, parameters);
     }
 
     @Override
@@ -57,14 +54,34 @@ public final class AspectValue extends ActionLookupValue {
       return label;
     }
 
+    public abstract AspectParameters getParameters();
+
     public BuildConfiguration getConfiguration() {
       return configuration;
     }
+  }
+
+  /**
+   * The key of an action that is generated by a native aspect.
+   */
+  public static final class NativeAspectKey extends AspectKey {
+    private final AspectWithParameters aspect;
+
+    private NativeAspectKey(
+        Label label,
+        BuildConfiguration configuration,
+        Class<? extends ConfiguredAspectFactory> aspectFactory,
+        AspectParameters parameters) {
+      super(label, configuration);
+      Preconditions.checkNotNull(parameters);
+      this.aspect = new AspectWithParameters(aspectFactory, parameters);
+    }
 
     public Class<? extends ConfiguredAspectFactory> getAspect() {
       return aspect.getAspectFactory();
     }
 
+    @Override
     @Nullable
     public AspectParameters getParameters() {
       return aspect.getParameters();
@@ -72,7 +89,7 @@ public final class AspectValue extends ActionLookupValue {
 
     @Override
     SkyFunctionName getType() {
-      return SkyFunctions.ASPECT;
+      return SkyFunctions.NATIVE_ASPECT;
     }
 
     @Override
@@ -86,11 +103,11 @@ public final class AspectValue extends ActionLookupValue {
         return true;
       }
 
-      if (!(other instanceof AspectKey)) {
+      if (!(other instanceof NativeAspectKey)) {
         return false;
       }
 
-      AspectKey that = (AspectKey) other;
+      NativeAspectKey that = (NativeAspectKey) other;
       return Objects.equal(label, that.label)
           && Objects.equal(configuration, that.configuration)
           && Objects.equal(aspect, that.aspect);
@@ -104,6 +121,43 @@ public final class AspectValue extends ActionLookupValue {
     }
   }
 
+  /**
+   * The key of an action that is generated by a skylark aspect.
+   */
+  public static class SkylarkAspectKey extends AspectKey {
+    private final PackageIdentifier extensionFile;
+    private final String skylarkFunctionName;
+
+    private SkylarkAspectKey(
+        Label targetLabel,
+        BuildConfiguration targetConfiguration,
+        PackageIdentifier extensionFile,
+        String skylarkFunctionName) {
+      super(targetLabel, targetConfiguration);
+      this.extensionFile = extensionFile;
+      this.skylarkFunctionName = skylarkFunctionName;
+    }
+
+    public PackageIdentifier getExtensionFile() {
+      return extensionFile;
+    }
+
+    public String getSkylarkValueName() {
+      return skylarkFunctionName;
+    }
+
+    @Override
+    public AspectParameters getParameters() {
+      return AspectParameters.EMPTY;
+    }
+
+    @Override
+    SkyFunctionName getType() {
+      return SkyFunctions.SKYLARK_ASPECT;
+    }
+  }
+
+
   private final Label label;
   private final Location location;
   private final AspectKey key;
@@ -144,18 +198,27 @@ public final class AspectValue extends ActionLookupValue {
   public static SkyKey key(Label label, BuildConfiguration configuration,
       Class<? extends ConfiguredAspectFactory> aspectFactory,
       AspectParameters additionalConfiguration) {
-    return new SkyKey(SkyFunctions.ASPECT,
-        new AspectKey(label, configuration, aspectFactory, additionalConfiguration));
+    return new SkyKey(
+        SkyFunctions.NATIVE_ASPECT,
+        new NativeAspectKey(label, configuration, aspectFactory, additionalConfiguration));
   }
 
   public static SkyKey key(AspectKey aspectKey) {
-    return new SkyKey(SkyFunctions.ASPECT, aspectKey);
+    return new SkyKey(aspectKey.getType(), aspectKey);
   }
 
-  public static AspectKey createAspectKey(
+  public static NativeAspectKey createAspectKey(
       Label label,
       BuildConfiguration configuration,
       Class<? extends ConfiguredAspectFactory> aspectFactory) {
-    return new AspectKey(label, configuration, aspectFactory, AspectParameters.EMPTY);
+    return new NativeAspectKey(label, configuration, aspectFactory, AspectParameters.EMPTY);
+  }
+
+  public static SkylarkAspectKey createSkylarkAspectKey(
+      Label targetLabel,
+      BuildConfiguration targetConfiguration,
+      PackageIdentifier bzlFile,
+      String skylarkFunctionName) {
+    return new SkylarkAspectKey(targetLabel, targetConfiguration, bzlFile, skylarkFunctionName);
   }
 }
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 c98da4e978..addf514e77 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
@@ -52,12 +52,10 @@ import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
 import com.google.devtools.build.lib.packages.BuildType;
-import com.google.devtools.build.lib.packages.InputFile;
 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.Package;
-import com.google.devtools.build.lib.packages.PackageGroup;
 import com.google.devtools.build.lib.packages.RawAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
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 6c17858983..ee3d395841 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
@@ -63,9 +63,10 @@ public final class SkyFunctions {
       SkyFunctionName.create("TRANSITIVE_TRAVERSAL");
   public static final SkyFunctionName CONFIGURED_TARGET =
       SkyFunctionName.create("CONFIGURED_TARGET");
-  public static final SkyFunctionName ASPECT = SkyFunctionName.create("ASPECT");
   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 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 22e272e9fe..49766ffc44 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
@@ -507,11 +507,13 @@ public final class SkyframeBuildView {
 
   @Nullable
   public Aspect createAspect(
-      AnalysisEnvironment env, RuleConfiguredTarget associatedTarget,
+      AnalysisEnvironment env,
+      RuleConfiguredTarget associatedTarget,
       ConfiguredAspectFactory aspectFactory,
       ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
       Set<ConfigMatchingProvider> configConditions,
-      AspectParameters aspectParameters) {
+      AspectParameters aspectParameters)
+      throws InterruptedException {
     return factory.createAspect(env, associatedTarget, aspectFactory, aspectParameters,
         prerequisiteMap, configConditions,
         getHostConfiguration(associatedTarget.getConfiguration()));
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 0b5c5cb707..cc809502d8 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
@@ -351,7 +351,12 @@ 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.ASPECT, new AspectFunction(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.POST_CONFIGURED_TARGET,
         new PostConfiguredTargetFunction(new BuildViewProvider(), ruleClassProvider));
     map.put(SkyFunctions.BUILD_CONFIGURATION,
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
new file mode 100644
index 0000000000..d7bbf40ef9
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
@@ -0,0 +1,95 @@
+// Copyright 2014 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.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.analysis.Aspect;
+import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.RuleContext;
+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;
+import com.google.devtools.build.lib.syntax.ClassObject.SkylarkClassObject;
+import com.google.devtools.build.lib.syntax.Environment;
+import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.Mutability;
+
+/**
+ * A factory for aspects that are defined in Skylark.
+ */
+public class SkylarkAspectFactory implements ConfiguredAspectFactory {
+
+  private final String name;
+  private final SkylarkAspect aspectFunction;
+
+  public SkylarkAspectFactory(String name, SkylarkAspect aspectFunction) {
+    this.name = name;
+    this.aspectFunction = aspectFunction;
+  }
+
+  @Override
+  public Aspect create(ConfiguredTarget base, RuleContext ruleContext, AspectParameters parameters)
+      throws InterruptedException {
+    try (Mutability mutability = Mutability.create("aspect")) {
+      SkylarkRuleContext skylarkRuleContext;
+      try {
+        skylarkRuleContext = new SkylarkRuleContext(ruleContext);
+      } catch (EvalException e) {
+        ruleContext.ruleError(e.getMessage());
+        return null;
+      }
+      Environment env =
+          Environment.builder(mutability)
+              .setSkylark()
+              .setGlobals(aspectFunction.getFuncallEnv().getGlobals())
+              .setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler())
+              .build(); // NB: loading phase functions are not available: this is analysis already,
+                        // so we do *not* setLoadingPhase().
+      Object aspect;
+      try {
+        aspect =
+            aspectFunction
+                .getImplementation()
+                .call(
+                    ImmutableList.<Object>of(base, skylarkRuleContext),
+                    ImmutableMap.<String, Object>of(),
+                    /*ast=*/ null,
+                    env);
+      } catch (EvalException e) {
+        ruleContext.ruleError(e.getMessage());
+        return null;
+      }
+      // TODO(dslomov): unify this code with
+      // {@link com.google.devtools.build.lib.rules.SkylarkRuleConfiguredTargetBuilder}
+      Aspect.Builder builder = new Aspect.Builder(name);
+      if (aspect instanceof SkylarkClassObject) {
+        SkylarkClassObject struct = (SkylarkClassObject) aspect;
+        Location loc = struct.getCreationLoc();
+        for (String key : struct.getKeys()) {
+          builder.addSkylarkTransitiveInfo(key, struct.getValue(key), loc);
+        }
+      }
+      return builder.build();
+    }
+  }
+
+  @Override
+  public AspectDefinition getDefinition() {
+    return new AspectDefinition.Builder(name).build();
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
index dc997a1959..85308841cd 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
@@ -67,7 +67,7 @@ public class SkylarkImportLookupValue implements SkyValue {
   }
 
   @VisibleForTesting
-  static SkyKey key(PackageIdentifier pkgIdentifier) throws ASTLookupInputException {
+  public static SkyKey key(PackageIdentifier pkgIdentifier) throws ASTLookupInputException {
     return key(pkgIdentifier.getRepository(), pkgIdentifier.getPackageFragment());
   }