Rollback of commit ac7195025b073948785db8c6975a53fc305c5087.

*** Reason for rollback ***

Uses tests that don't run on Bazel

*** Original change description ***

Implement aspect(...) Skylark function.

--
MOS_MIGRATED_REVID=105808850

18 files changed, 94 insertions, 549 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 bb6c38d9a4..fe97bfc3b4 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,7 +22,6 @@ 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;
@@ -89,8 +88,6 @@ 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) {
@@ -106,8 +103,6 @@ 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;
     }
@@ -132,12 +127,6 @@ 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();
@@ -152,11 +141,6 @@ 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 e05dd596d9..14302ac179 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,7 +42,6 @@ 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;
@@ -76,7 +75,6 @@ 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;
@@ -452,43 +450,18 @@ public class BuildView {
 
     List<AspectKey> aspectKeys = new ArrayList<>();
     for (String aspect : aspects) {
-
-      // 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);
+      @SuppressWarnings("unchecked")
+      final Class<? extends ConfiguredAspectFactory> aspectFactoryClass =
+          (Class<? extends ConfiguredAspectFactory>)
+              ruleClassProvider.getAspectFactoryMap().get(aspect);
+      if (aspectFactoryClass != null) {
         for (ConfiguredTargetKey targetSpec : targetSpecs) {
           aspectKeys.add(
-              AspectValue.createSkylarkAspectKey(
-                  targetSpec.getLabel(),
-                  targetSpec.getConfiguration(),
-                  bzlFile,
-                  skylarkFunctionName));
+              AspectValue.createAspectKey(
+                  targetSpec.getLabel(), targetSpec.getConfiguration(), aspectFactoryClass));
         }
       } else {
-        @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");
-        }
+        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 a8cc9e4932..6d97e86c42 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,14 +286,12 @@ 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)
-      throws InterruptedException {
+      BuildConfiguration hostConfiguration) {
     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 d8614597a4..9bb9365d80 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,6 +22,7 @@ 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;
@@ -130,14 +131,20 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
     providers.addAll(base.providers.keySet());
 
     // Merge output group providers.
-    List<OutputGroupProvider> outputGroupProviders =
-        getAllProviders(base, aspects, OutputGroupProvider.class);
-    OutputGroupProvider mergedOutputGroupProvider = OutputGroupProvider.merge(outputGroupProviders);
+    OutputGroupProvider baseOutputGroupProvider = base.getProvider(OutputGroupProvider.class);
+    List<OutputGroupProvider> outputGroupProviders = new ArrayList<>();
+    if (baseOutputGroupProvider != null) {
+      outputGroupProviders.add(baseOutputGroupProvider);
+    }
 
-    // Merge Skylark providers.
-    List<SkylarkProviders> skylarkProviders =
-        getAllProviders(base, aspects, SkylarkProviders.class);
-    SkylarkProviders mergedSkylarkProviders = SkylarkProviders.merge(skylarkProviders);
+    for (Aspect aspect : aspects) {
+      final OutputGroupProvider aspectProvider = aspect.getProvider(OutputGroupProvider.class);
+      if (aspectProvider == null) {
+        continue;
+      }
+      outputGroupProviders.add(aspectProvider);
+    }
+    OutputGroupProvider outputGroupProvider = OutputGroupProvider.merge(outputGroupProviders);
 
     // Validate that all other providers are only provided once.
     for (Aspect aspect : aspects) {
@@ -146,17 +153,13 @@ 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 (base.getProvider(OutputGroupProvider.class) == mergedOutputGroupProvider
-        && base.getProvider(SkylarkProviders.class) == mergedSkylarkProviders) {
+    if (baseOutputGroupProvider == outputGroupProvider) {
       this.providers = base.providers;
     } else {
       ImmutableMap.Builder<Class<? extends TransitiveInfoProvider>, Object> builder =
@@ -165,17 +168,9 @@ 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));
       }
-      if (mergedOutputGroupProvider != null) {
-        builder.put(OutputGroupProvider.class, mergedOutputGroupProvider);
-      }
-      if (mergedSkylarkProviders != null) {
-        builder.put(SkylarkProviders.class, skylarkProviders);
-      }
+      builder.put(OutputGroupProvider.class, outputGroupProvider);
       this.providers = builder.build();
     }
     this.mandatoryStampFiles = base.mandatoryStampFiles;
@@ -183,26 +178,6 @@ 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.
    */
@@ -233,7 +208,7 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
    */
   @Override
   public Object get(String providerKey) {
-    return getProvider(SkylarkProviders.class).getValue(providerKey);
+    return getProvider(SkylarkProviders.class).skylarkProviders.get(providerKey);
   }
 
   public ImmutableList<Artifact> getMandatoryStampFiles() {
@@ -245,6 +220,33 @@ 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 =
@@ -271,6 +273,6 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
   @Override
   public ImmutableCollection<String> getKeys() {
     return ImmutableList.<String>builder().addAll(super.getKeys())
-        .addAll(getProvider(SkylarkProviders.class).getKeys()).build();
+        .addAll(getProvider(SkylarkProviders.class).skylarkProviders.keySet()).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
deleted file mode 100644
index d4746c4659..0000000000
--- a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
+++ /dev/null
@@ -1,79 +0,0 @@
-// 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 cd7ba2c51f..eea286d607 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,8 +29,7 @@ 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)
-      throws InterruptedException;
+  TAspect create(TConfiguredTarget base, TRuleContext context, AspectParameters parameters);
 
   /**
    * 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 05d6100ab2..87b570318c 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,7 +69,6 @@ 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;
@@ -77,7 +76,6 @@ 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;
@@ -331,21 +329,6 @@ public class SkylarkRuleClassFunctions {
     }
   };
 
-  @SkylarkSignature(
-    name = "aspect",
-    returnType = SkylarkAspect.class,
-    documented = false, // todo(dslomov)
-    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.
@@ -533,36 +516,4 @@ 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 9e7ba336e5..71619eb4ea 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,8 +62,7 @@ public final class SkylarkRuleConfiguredTargetBuilder {
           .setGlobals(
               ruleContext.getRule().getRuleClassObject().getRuleDefinitionEnvironment().getGlobals())
           .setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler())
-          .build(); // NB: loading phase functions are not available: this is analysis already,
-                    // so we do *not* setLoadingPhase().
+          .build(); // NB: 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 a0dc80b2da..3ffbedeac7 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(SkylarkProviders.class).getKeys());
+              configuredTarget.getProvider(RuleConfiguredTarget.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 57dc717f00..df627ce9e7 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,14 +35,9 @@ 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;
@@ -55,30 +50,13 @@ 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 static AspectFunction<SkylarkAspectKey> createSkylarkAspectFunction(
-      BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
-    return new AspectFunction<>(
-        buildViewProvider, ruleClassProvider, new SkylarkAspectFactoryCreator());
-  }
-
-  private AspectFunction(
-      BuildViewProvider buildViewProvider,
-      RuleClassProvider ruleClassProvider,
-      AspectFactoryCreator<T> aspectFactoryCreator) {
+  public AspectFunction(BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
     this.buildViewProvider = buildViewProvider;
     this.ruleClassProvider = ruleClassProvider;
-    this.aspectFactoryCreator = aspectFactoryCreator;
   }
 
   @Nullable
@@ -88,7 +66,8 @@ 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 =
+        (ConfiguredAspectFactory) AspectFactory.Util.create(key.getAspect());
 
     PackageValue packageValue =
         (PackageValue) env.getValue(PackageValue.key(key.getLabel().getPackageIdentifier()));
@@ -153,13 +132,7 @@ public final class AspectFunction<T extends AspectKey> implements SkyFunction {
               ruleClassProvider, view.getHostConfiguration(ctgValue.getConfiguration()),
               transitivePackages);
 
-      return createAspect(
-          env,
-          key,
-          aspectFactory,
-          associatedTarget,
-          configConditions,
-          depValueMap,
+      return createAspect(env, key, associatedTarget, configConditions, depValueMap,
           transitivePackages);
     } catch (DependencyEvaluationException e) {
       throw new AspectFunctionException(e.getRootCauseSkyKey(), e.getCause());
@@ -169,15 +142,11 @@ public final class AspectFunction<T extends AspectKey> implements SkyFunction {
   }
 
   @Nullable
-  private AspectValue createAspect(
-      Environment env,
-      AspectKey key,
-      ConfiguredAspectFactory aspectFactory,
-      RuleConfiguredTarget associatedTarget,
-      Set<ConfigMatchingProvider> configConditions,
+  private AspectValue createAspect(Environment env, AspectKey key,
+      RuleConfiguredTarget associatedTarget, Set<ConfigMatchingProvider> configConditions,
       ListMultimap<Attribute, ConfiguredTarget> directDeps,
       NestedSetBuilder<Package> transitivePackages)
-      throws AspectFunctionException, InterruptedException {
+      throws AspectFunctionException {
 
     SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
     BuildConfiguration configuration = associatedTarget.getConfiguration();
@@ -189,6 +158,8 @@ public final class AspectFunction<T extends AspectKey> implements SkyFunction {
       return null;
     }
 
+    ConfiguredAspectFactory aspectFactory =
+        (ConfiguredAspectFactory) AspectFactory.Util.create(key.getAspect());
     Aspect aspect = view.createAspect(
         analysisEnvironment, associatedTarget, aspectFactory, directDeps, configConditions,
         key.getParameters());
@@ -246,58 +217,4 @@ public final class AspectFunction<T extends AspectKey> 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 5a4052fb8c..16277c7a47 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,7 +22,6 @@ 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;
@@ -36,17 +35,21 @@ import javax.annotation.Nullable;
  * An aspect in the context of the Skyframe graph.
  */
 public final class AspectValue extends ActionLookupValue {
-
   /**
-   * A base class for a key representing an aspect applied to a particular target.
+   * The key of an action that is generated by an aspect.
    */
-  public abstract static class AspectKey extends ActionLookupKey {
-    protected final Label label;
-    protected final BuildConfiguration configuration;
+  public static final class AspectKey extends ActionLookupKey {
+    private final Label label;
+    private final BuildConfiguration configuration;
+    private final AspectWithParameters aspect;
 
-    protected AspectKey(Label label, BuildConfiguration configuration) {
+    private AspectKey(Label label, BuildConfiguration configuration,
+        Class<? extends ConfiguredAspectFactory> aspectFactory,
+        AspectParameters parameters) {
+      Preconditions.checkNotNull(parameters);
       this.label = label;
       this.configuration = configuration;
+      this.aspect = new AspectWithParameters(aspectFactory, parameters);
     }
 
     @Override
@@ -54,34 +57,14 @@ 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();
@@ -89,7 +72,7 @@ public final class AspectValue extends ActionLookupValue {
 
     @Override
     SkyFunctionName getType() {
-      return SkyFunctions.NATIVE_ASPECT;
+      return SkyFunctions.ASPECT;
     }
 
     @Override
@@ -103,11 +86,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);
@@ -121,43 +104,6 @@ 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;
@@ -198,27 +144,18 @@ public final class AspectValue extends ActionLookupValue {
   public static SkyKey key(Label label, BuildConfiguration configuration,
       Class<? extends ConfiguredAspectFactory> aspectFactory,
       AspectParameters additionalConfiguration) {
-    return new SkyKey(
-        SkyFunctions.NATIVE_ASPECT,
-        new NativeAspectKey(label, configuration, aspectFactory, additionalConfiguration));
+    return new SkyKey(SkyFunctions.ASPECT,
+        new AspectKey(label, configuration, aspectFactory, additionalConfiguration));
   }
 
   public static SkyKey key(AspectKey aspectKey) {
-    return new SkyKey(aspectKey.getType(), aspectKey);
+    return new SkyKey(SkyFunctions.ASPECT, aspectKey);
   }
 
-  public static NativeAspectKey createAspectKey(
+  public static AspectKey createAspectKey(
       Label label,
       BuildConfiguration configuration,
       Class<? extends ConfiguredAspectFactory> aspectFactory) {
-    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);
+    return new AspectKey(label, configuration, aspectFactory, AspectParameters.EMPTY);
   }
 }
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 addf514e77..c98da4e978 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,10 +52,12 @@ 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 ee3d395841..6c17858983 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,10 +63,9 @@ 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 49766ffc44..22e272e9fe 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,13 +507,11 @@ 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)
-      throws InterruptedException {
+      AspectParameters aspectParameters) {
     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 cc809502d8..0b5c5cb707 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,12 +351,7 @@ 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.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
deleted file mode 100644
index f757847fa2..0000000000
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
+++ /dev/null
@@ -1,95 +0,0 @@
-// 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 85308841cd..dc997a1959 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
-  public static SkyKey key(PackageIdentifier pkgIdentifier) throws ASTLookupInputException {
+  static SkyKey key(PackageIdentifier pkgIdentifier) throws ASTLookupInputException {
     return key(pkgIdentifier.getRepository(), pkgIdentifier.getPackageFragment());
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
index db45c510d0..db6532486d 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -14,20 +14,16 @@
 package com.google.devtools.build.lib.skylark;
 
 import static com.google.common.collect.Iterables.getOnlyElement;
-import static com.google.common.collect.Iterables.transform;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.baseArtifactNames;
 
-import com.google.common.base.Function;
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
-import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
-import com.google.devtools.build.lib.analysis.BuildView.AnalysisResult;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.FileConfiguredTarget;
 import com.google.devtools.build.lib.analysis.FileProvider;
@@ -59,8 +55,6 @@ import junit.framework.AssertionFailedError;
 import java.util.LinkedList;
 import java.util.List;
 
-import javax.annotation.Nullable;
-
 /**
  * Integration tests for Skylark.
  */
@@ -2650,35 +2644,6 @@ public class SkylarkIntegrationTest extends BuildViewTestCase {
     }
   }
 
-  public void testAspect() throws Exception {
-    scratch.file(
-        "test/aspect.bzl",
-        "def _impl(target, ctx):",
-        "   print('This aspect does nothing')",
-        "MyAspect = aspect(implementation=_impl)");
-    scratch.file("test/BUILD", "java_library(name = 'xxx',)");
-
-    AnalysisResult analysisResult =
-        update(
-            ImmutableList.of("//test:xxx"),
-            ImmutableList.<String>of("test/aspect.bzl%MyAspect"),
-            false,
-            LOADING_PHASE_THREADS,
-            true,
-            new EventBus());
-    assertThat(
-        transform(
-            analysisResult.getTargetsToBuild(),
-            new Function<ConfiguredTarget, String>() {
-              @Nullable
-              @Override
-              public String apply(ConfiguredTarget configuredTarget) {
-                return configuredTarget.getLabel().toString();
-              }
-            }))
-        .containsExactly("//test:xxx");
-  }
-
   /**
    * Skylark integration test that forces inlining.
    */