Restrict aspects visible to other aspects according to their advertised providers.

--
PiperOrigin-RevId: 147526961
MOS_MIGRATED_REVID=147526961

19 files changed, 338 insertions, 238 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java
index 0018525784..6427014a85 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java
@@ -213,6 +213,10 @@ public final class AspectCollection {
     }
   }
 
+  public static AspectCollection createForTests(AspectDescriptor... descriptors) {
+    return createForTests(ImmutableSet.copyOf(descriptors));
+  }
+
   public static AspectCollection createForTests(ImmutableSet<AspectDescriptor> descriptors) {
     ImmutableSet.Builder<AspectDeps> depsBuilder = ImmutableSet.builder();
     for (AspectDescriptor descriptor : descriptors) {
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 19e972c4fe..10a9216ae5 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
@@ -857,7 +857,7 @@ public class BuildView {
                 targetAndConfig.getLabel(),
                 Attribute.ConfigurationTransition.NONE,
                 // TODO(bazel-team): support top-level aspects
-                ImmutableSet.<AspectDescriptor>of()));
+                AspectCollection.EMPTY));
       }
     }
 
@@ -1003,7 +1003,7 @@ public class BuildView {
           Iterable<BuildOptions> buildOptions) {
         Preconditions.checkArgument(ct.getConfiguration().fragmentClasses().equals(fragments));
         Dependency asDep = Dependency.withTransitionAndAspects(ct.getLabel(),
-            Attribute.ConfigurationTransition.NONE, ImmutableSet.<AspectDescriptor>of());
+            Attribute.ConfigurationTransition.NONE, AspectCollection.EMPTY);
         ImmutableList.Builder<BuildConfiguration> builder = ImmutableList.builder();
         for (BuildOptions options : buildOptions) {
           builder.add(Iterables.getOnlyElement(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java b/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
index d72bced27a..9b67fe8583 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.lib.analysis;
 
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.AspectDescriptor;
@@ -62,7 +61,9 @@ public abstract class Dependency {
    */
   public static Dependency withConfiguration(Label label, BuildConfiguration configuration) {
     return new StaticConfigurationDependency(
-        label, configuration, ImmutableMap.<AspectDescriptor, BuildConfiguration>of());
+        label, configuration,
+        AspectCollection.EMPTY,
+        ImmutableMap.<AspectDescriptor, BuildConfiguration>of());
   }
 
   /**
@@ -72,13 +73,13 @@ public abstract class Dependency {
    * <p>The configuration and aspects must not be {@code null}.
    */
   public static Dependency withConfigurationAndAspects(
-      Label label, BuildConfiguration configuration, Iterable<AspectDescriptor> aspects) {
+      Label label, BuildConfiguration configuration, AspectCollection aspects) {
     ImmutableMap.Builder<AspectDescriptor, BuildConfiguration> aspectBuilder =
         new ImmutableMap.Builder<>();
-    for (AspectDescriptor aspect : aspects) {
+    for (AspectDescriptor aspect : aspects.getAllAspects()) {
       aspectBuilder.put(aspect, configuration);
     }
-    return new StaticConfigurationDependency(label, configuration, aspectBuilder.build());
+    return new StaticConfigurationDependency(label, configuration, aspects, aspectBuilder.build());
   }
 
   /**
@@ -90,9 +91,10 @@ public abstract class Dependency {
    */
   public static Dependency withConfiguredAspects(
       Label label, BuildConfiguration configuration,
+      AspectCollection aspects,
       Map<AspectDescriptor, BuildConfiguration> aspectConfigurations) {
     return new StaticConfigurationDependency(
-        label, configuration, ImmutableMap.copyOf(aspectConfigurations));
+        label, configuration, aspects, ImmutableMap.copyOf(aspectConfigurations));
   }
 
   /**
@@ -100,8 +102,8 @@ public abstract class Dependency {
    * configuration builds.
    */
   public static Dependency withTransitionAndAspects(
-      Label label, Attribute.Transition transition, Iterable<AspectDescriptor> aspects) {
-    return new DynamicConfigurationDependency(label, transition, ImmutableSet.copyOf(aspects));
+      Label label, Attribute.Transition transition, AspectCollection aspects) {
+    return new DynamicConfigurationDependency(label, transition, aspects);
   }
 
   protected final Label label;
@@ -144,18 +146,16 @@ public abstract class Dependency {
    * Returns the set of aspects which should be evaluated and combined with the configured target
    * pointed to by this dependency.
    *
-   * @see #getAspectConfigurations()
+   * @see #getAspectConfiguration(AspectDescriptor)
    */
-  public abstract ImmutableSet<AspectDescriptor> getAspects();
+  public abstract AspectCollection getAspects();
 
   /**
-   * Returns the mapping from aspects to the static configurations they should be evaluated with.
-   *
-   * <p>The {@link Map#keySet()} of this map is equal to that returned by {@link #getAspects()}.
-   *
+   * Returns the the static configuration an aspect should be evaluated with
+   **
    * @throws IllegalStateException if {@link #hasStaticConfiguration()} returns false.
    */
-  public abstract ImmutableMap<AspectDescriptor, BuildConfiguration> getAspectConfigurations();
+  public abstract BuildConfiguration getAspectConfiguration(AspectDescriptor aspect);
 
   /**
    * Implementation of a dependency with no configuration, suitable for static configuration
@@ -184,13 +184,13 @@ public abstract class Dependency {
     }
 
     @Override
-    public ImmutableSet<AspectDescriptor> getAspects() {
-      return ImmutableSet.of();
+    public AspectCollection getAspects() {
+      return AspectCollection.EMPTY;
     }
 
     @Override
-    public ImmutableMap<AspectDescriptor, BuildConfiguration> getAspectConfigurations() {
-      return ImmutableMap.of();
+    public BuildConfiguration getAspectConfiguration(AspectDescriptor aspect) {
+      return null;
     }
 
     @Override
@@ -219,14 +219,17 @@ public abstract class Dependency {
    */
   private static final class StaticConfigurationDependency extends Dependency {
     private final BuildConfiguration configuration;
+    private final AspectCollection aspects;
     private final ImmutableMap<AspectDescriptor, BuildConfiguration> aspectConfigurations;
 
     public StaticConfigurationDependency(
         Label label, BuildConfiguration configuration,
-        ImmutableMap<AspectDescriptor, BuildConfiguration> aspects) {
+        AspectCollection aspects,
+        ImmutableMap<AspectDescriptor, BuildConfiguration> aspectConfigurations) {
       super(label);
       this.configuration = Preconditions.checkNotNull(configuration);
-      this.aspectConfigurations = Preconditions.checkNotNull(aspects);
+      this.aspects = Preconditions.checkNotNull(aspects);
+      this.aspectConfigurations = Preconditions.checkNotNull(aspectConfigurations);
     }
 
     @Override
@@ -246,13 +249,13 @@ public abstract class Dependency {
     }
 
     @Override
-    public ImmutableSet<AspectDescriptor> getAspects() {
-      return aspectConfigurations.keySet();
+    public AspectCollection getAspects() {
+      return aspects;
     }
 
     @Override
-    public ImmutableMap<AspectDescriptor, BuildConfiguration> getAspectConfigurations() {
-      return aspectConfigurations;
+    public BuildConfiguration getAspectConfiguration(AspectDescriptor aspect) {
+      return aspectConfigurations.get(aspect);
     }
 
     @Override
@@ -268,6 +271,7 @@ public abstract class Dependency {
       StaticConfigurationDependency otherDep = (StaticConfigurationDependency) other;
       return label.equals(otherDep.label)
           && configuration.equals(otherDep.configuration)
+          && aspects.equals(otherDep.aspects)
           && aspectConfigurations.equals(otherDep.aspectConfigurations);
     }
 
@@ -285,10 +289,10 @@ public abstract class Dependency {
    */
   private static final class DynamicConfigurationDependency extends Dependency {
     private final Attribute.Transition transition;
-    private final ImmutableSet<AspectDescriptor> aspects;
+    private final AspectCollection aspects;
 
     public DynamicConfigurationDependency(
-        Label label, Attribute.Transition transition, ImmutableSet<AspectDescriptor> aspects) {
+        Label label, Attribute.Transition transition, AspectCollection aspects) {
       super(label);
       this.transition = Preconditions.checkNotNull(transition);
       this.aspects = Preconditions.checkNotNull(aspects);
@@ -311,15 +315,15 @@ public abstract class Dependency {
     }
 
     @Override
-    public ImmutableSet<AspectDescriptor> getAspects() {
+    public AspectCollection getAspects() {
       return aspects;
     }
 
     @Override
-    public ImmutableMap<AspectDescriptor, BuildConfiguration> getAspectConfigurations() {
+    public BuildConfiguration getAspectConfiguration(AspectDescriptor aspect) {
       throw new IllegalStateException(
           "A dependency with a dynamic configuration transition does not have aspect "
-          + "configurations.");
+              + "configurations.");
     }
 
     @Override
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
index dc1f29278c..06d561bf48 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
@@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.analysis.AspectCollection.AspectCycleOnPathException;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
@@ -520,36 +521,86 @@ public abstract class DependencyResolver {
   }
 
 
-  private static ImmutableSet<AspectDescriptor> requiredAspects(
-      Iterable<Aspect> aspects,
+  private static AspectCollection requiredAspects(
+      Iterable<Aspect> aspectPath,
       AttributeAndOwner attributeAndOwner,
       final Target target,
       Rule originalRule) {
     if (!(target instanceof Rule)) {
-      return ImmutableSet.of();
+      return AspectCollection.EMPTY;
     }
 
     if (attributeAndOwner.ownerAspect != null) {
       // Do not propagate aspects along aspect attributes.
-      return ImmutableSet.of();
+      return AspectCollection.EMPTY;
     }
 
-    Iterable<Aspect> aspectCandidates =
-        extractAspectCandidates(aspects, attributeAndOwner.attribute, originalRule);
-    RuleClass ruleClass = ((Rule) target).getRuleClassObject();
-    ImmutableSet.Builder<AspectDescriptor> result = ImmutableSet.builder();
+    ImmutableList.Builder<Aspect> filteredAspectPath = ImmutableList.builder();
+    ImmutableSet.Builder<AspectDescriptor> visibleAspects = ImmutableSet.builder();
+
+    collectOriginatingAspects(originalRule, attributeAndOwner.attribute, (Rule) target,
+        filteredAspectPath, visibleAspects);
+
+    collectPropagatingAspects(aspectPath,
+        attributeAndOwner.attribute,
+        (Rule) target, filteredAspectPath, visibleAspects);
+    try {
+      return AspectCollection.create(filteredAspectPath.build(), visibleAspects.build());
+    } catch (AspectCycleOnPathException e) {
+      // TODO(dslomov): propagate the error and report to user.
+      throw new AssertionError(e);
+    }
+  }
 
-    for (Aspect aspectCandidate : aspectCandidates) {
-      if (aspectCandidate.getDefinition()
-          .getRequiredProviders()
+  /**
+   * Collects into {@code filteredAspectPath}
+   * aspects from {@code aspectPath} that propagate along {@code attribute}
+   * and apply to a given {@code target}.
+   *
+   * The last aspect in {@code aspectPath} is (potentially) visible and recorded
+   * in {@code visibleAspects}.
+   */
+  private static void collectPropagatingAspects(Iterable<Aspect> aspectPath,
+      Attribute attribute, Rule target,
+      ImmutableList.Builder<Aspect> filteredAspectPath,
+      ImmutableSet.Builder<AspectDescriptor> visibleAspects) {
+
+    Aspect lastAspect = null;
+    for (Aspect aspect : aspectPath) {
+      lastAspect = aspect;
+      if (aspect.getDefinition().propagateAlong(attribute)
+          && aspect.getDefinition().getRequiredProviders()
+                  .isSatisfiedBy(target.getRuleClassObject().getAdvertisedProviders())) {
+        filteredAspectPath.add(aspect);
+      } else {
+        lastAspect = null;
+      }
+    }
+
+    if (lastAspect != null) {
+      visibleAspects.add(lastAspect.getDescriptor());
+    }
+  }
+
+  /**
+   * Collect all aspects that originate on {@code attribute} of {@code originalRule}
+   * and are applicable to a {@code target}
+   *
+   * They are appended to {@code filteredAspectPath} and registered in {@code visibleAspects} set.
+   */
+  private static void collectOriginatingAspects(
+      Rule originalRule, Attribute attribute, Rule target,
+      ImmutableList.Builder<Aspect> filteredAspectPath,
+      ImmutableSet.Builder<AspectDescriptor> visibleAspects) {
+    ImmutableList<Aspect> baseAspects = attribute.getAspects(originalRule);
+    RuleClass ruleClass = target.getRuleClassObject();
+    for (Aspect baseAspect : baseAspects) {
+      if (baseAspect.getDefinition().getRequiredProviders()
           .isSatisfiedBy(ruleClass.getAdvertisedProviders())) {
-        result.add(
-            new AspectDescriptor(
-                aspectCandidate.getAspectClass(),
-                aspectCandidate.getParameters()));
+        filteredAspectPath.add(baseAspect);
+        visibleAspects.add(baseAspect.getDescriptor());
       }
     }
-    return result.build();
   }
 
   private static Iterable<Aspect> extractAspectCandidates(
@@ -686,7 +737,7 @@ public abstract class DependencyResolver {
         applyNullTransition = true;
       }
 
-      ImmutableSet<AspectDescriptor> aspects =
+      AspectCollection aspects =
           requiredAspects(this.aspects, attributeAndOwner, toTarget, rule);
       Dependency dep;
       if (config.useDynamicConfigurations() && !applyNullTransition) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 5823a52d81..b16a4a6bd4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -34,6 +34,7 @@ import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.MutableClassToInstanceMap;
 import com.google.devtools.build.lib.actions.Root;
+import com.google.devtools.build.lib.analysis.AspectCollection;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.Dependency;
@@ -45,7 +46,6 @@ import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
 import com.google.devtools.build.lib.packages.Attribute.Configurator;
@@ -1612,7 +1612,7 @@ public final class BuildConfiguration {
      * for each configuration represented by this instance.
      * TODO(bazel-team): this is a really ugly reverse dependency: factor this away.
      */
-    Iterable<Dependency> getDependencies(Label label, ImmutableSet<AspectDescriptor> aspects);
+    Iterable<Dependency> getDependencies(Label label, AspectCollection aspects);
   }
 
   /**
@@ -1706,7 +1706,7 @@ public final class BuildConfiguration {
 
     @Override
     public Iterable<Dependency> getDependencies(
-        Label label, ImmutableSet<AspectDescriptor> aspects) {
+        Label label, AspectCollection aspects) {
       ImmutableList.Builder<Dependency> deps = ImmutableList.builder();
       for (BuildConfiguration config : toConfigurations) {
         deps.add(config != null
@@ -1829,7 +1829,7 @@ public final class BuildConfiguration {
 
     @Override
     public Iterable<Dependency> getDependencies(
-        Label label, ImmutableSet<AspectDescriptor> aspects) {
+        Label label, AspectCollection aspects) {
       return ImmutableList.of(
           isNull()
               // We can trivially set the final value for null-configured targets now. This saves
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
index a305e515d2..8cc374f212 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
@@ -280,10 +280,8 @@ public final class AspectDefinition {
     }
 
     public Builder requireAspectsWithNativeProviders(
-        Iterable<ImmutableSet<SkylarkProviderIdentifier>> providerSets) {
-      for (ImmutableSet<SkylarkProviderIdentifier> providerSet : providerSets) {
-        requiredAspectProviders.addSkylarkSet(providerSet);
-      }
+        Class<?>... providers) {
+      requiredAspectProviders.addNativeSet(ImmutableSet.copyOf(providers));
       return this;
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
index 5518692436..ebd734bb45 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
@@ -136,7 +136,8 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu
             .allowedRuleClasses("android_sdk", "filegroup")
             .value(new AndroidRuleClasses.AndroidSdkLabel(
                 Label.parseAbsoluteUnchecked(toolsRepository + AndroidRuleClasses.DEFAULT_SDK))))
-        .requiresConfigurationFragments(AndroidConfiguration.class);
+        .requiresConfigurationFragments(AndroidConfiguration.class)
+        .requireAspectsWithNativeProviders(JavaCompilationArgsAspectProvider.class);
     if (TriState.valueOf(params.getOnlyValueOfAttribute("incremental_dexing")) != TriState.NO) {
       // Marginally improves "query2" precision for targets that disable incremental dexing
       result.add(attr(ASPECT_DEXBUILDER_PREREQ, LABEL).cfg(HOST).exec()
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiProvider.java
index 591057e4ac..5e89e5b14b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiProvider.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
 import com.google.devtools.build.lib.rules.SkylarkApiProvider;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
@@ -43,7 +44,8 @@ public final class JavaSkylarkApiProvider extends SkylarkApiProvider {
   /** The name of the field in Skylark used to access this class. */
   public static final String NAME = "java";
   /** The name of the field in Skylark proto aspects used to access this class. */
-  public static final String PROTO_NAME = "proto_java";
+  public static final SkylarkProviderIdentifier PROTO_NAME =
+      SkylarkProviderIdentifier.forLegacy("proto_java");
 
   private final JavaRuleOutputJarsProvider ruleOutputJarsProvider;
   @Nullable private final JavaSourceJarsProvider sourceJarsProvider;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java
index a151b3db54..9eef47a53c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java
@@ -118,6 +118,8 @@ public class JavaLiteProtoAspect extends NativeAspectClass implements Configured
             .propagateAlongAttribute("deps")
             .requiresConfigurationFragments(JavaConfiguration.class, ProtoConfiguration.class)
             .requireProviders(ProtoSourcesProvider.class)
+            .advertiseProvider(JavaCompilationArgsAspectProvider.class)
+            .advertiseProvider(ImmutableList.of(JavaSkylarkApiProvider.PROTO_NAME))
             .add(
                 attr(PROTO_TOOLCHAIN_ATTR, LABEL)
                     .mandatoryNativeProviders(
@@ -218,7 +220,8 @@ public class JavaLiteProtoAspect extends NativeAspectClass implements Configured
       skylarkApiProvider.setCompilationArgsProvider(generatedCompilationArgsProvider);
 
       aspect
-          .addSkylarkTransitiveInfo(JavaSkylarkApiProvider.PROTO_NAME, skylarkApiProvider.build())
+          .addSkylarkTransitiveInfo(
+              JavaSkylarkApiProvider.PROTO_NAME.getLegacyId(), skylarkApiProvider.build())
           .addProviders(
               new JavaProtoLibraryTransitiveFilesToBuildProvider(transitiveOutputJars.build()),
               new JavaCompilationArgsAspectProvider(generatedCompilationArgsProvider));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
index a248bbf7b6..e702f27fc9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
@@ -125,6 +125,8 @@ public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspe
             .propagateAlongAttribute("deps")
             .requiresConfigurationFragments(JavaConfiguration.class, ProtoConfiguration.class)
             .requireProviders(ProtoSourcesProvider.class)
+            .advertiseProvider(JavaCompilationArgsAspectProvider.class)
+            .advertiseProvider(ImmutableList.of(JavaSkylarkApiProvider.PROTO_NAME))
             .add(
                 attr(SPEED_PROTO_TOOLCHAIN_ATTR, LABEL)
                     // TODO(carmi): reinstate mandatoryNativeProviders(ProtoLangToolchainProvider)
@@ -230,7 +232,8 @@ public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspe
 
       skylarkApiProvider.setCompilationArgsProvider(generatedCompilationArgsProvider);
       aspect
-          .addSkylarkTransitiveInfo(JavaSkylarkApiProvider.PROTO_NAME, skylarkApiProvider.build())
+          .addSkylarkTransitiveInfo(
+              JavaSkylarkApiProvider.PROTO_NAME.getLegacyId(), skylarkApiProvider.build())
           .addProviders(
               new JavaProtoLibraryTransitiveFilesToBuildProvider(transitiveOutputJars.build()),
               new JavaCompilationArgsAspectProvider(generatedCompilationArgsProvider));
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 4f8835befb..3d5aa88e92 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
@@ -34,6 +34,7 @@ import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.Aspect;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
 import com.google.devtools.build.lib.packages.NativeAspectClass;
@@ -60,6 +61,7 @@ import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.Map;
 import javax.annotation.Nullable;
 
@@ -218,16 +220,18 @@ public final class AspectFunction implements SkyFunction {
 
     ImmutableList.Builder<Aspect> aspectPathBuilder = ImmutableList.builder();
 
-    if (key.getBaseKey() != null) {
-      ImmutableList<SkyKey> aspectKeys = getSkyKeysForAspects(key.getBaseKey());
+    if (!key.getBaseKeys().isEmpty()) {
+      // We transitively collect all required aspects to reduce the number of restarts.
+      // Semantically it is enough to just request key.getBaseKeys().
+      ImmutableMap<AspectDescriptor, SkyKey> aspectKeys = getSkyKeysForAspects(key.getBaseKeys());
 
-      Map<SkyKey, SkyValue> values = env.getValues(aspectKeys);
+      Map<SkyKey, SkyValue> values = env.getValues(aspectKeys.values());
       if (env.valuesMissing()) {
         return null;
       }
       try {
         associatedTarget = getBaseTargetAndCollectPath(
-            associatedTarget, aspectKeys, values, aspectPathBuilder);
+            associatedTarget, key.getBaseKeys(), values, aspectPathBuilder);
       } catch (DuplicateException e) {
         env.getListener().handle(
             Event.error(associatedTarget.getTarget().getLocation(), e.getMessage()));
@@ -321,11 +325,13 @@ public final class AspectFunction implements SkyFunction {
    * @throws DuplicateException if there is a duplicate provider provided by aspects.
    */
   private ConfiguredTarget getBaseTargetAndCollectPath(ConfiguredTarget target,
-      ImmutableList<SkyKey> aspectKeys, Map<SkyKey, SkyValue> values,
+      ImmutableList<AspectKey> aspectKeys,
+      Map<SkyKey, SkyValue> values,
       ImmutableList.Builder<Aspect> aspectPath)
       throws DuplicateException {
     ArrayList<ConfiguredAspect> aspectValues = new ArrayList<>();
-    for (SkyKey skyAspectKey : aspectKeys) {
+    for (AspectKey aspectKey : aspectKeys) {
+      SkyKey skyAspectKey = aspectKey.getSkyKey();
       AspectValue aspectValue = (AspectValue) values.get(skyAspectKey);
       ConfiguredAspect configuredAspect = aspectValue.getConfiguredAspect();
       aspectValues.add(configuredAspect);
@@ -335,19 +341,28 @@ public final class AspectFunction implements SkyFunction {
   }
 
   /**
-   *  Returns a list of SkyKeys for all aspects the given aspect key depends on.
-   *  The order corresponds to the order the aspects should be merged into a configured target.
+   *  Collect all SkyKeys that are needed for a given list of AspectKeys,
+   *  including transitive dependencies.
    */
-  private ImmutableList<SkyKey> getSkyKeysForAspects(AspectKey key) {
-    ImmutableList.Builder<SkyKey> aspectKeysBuilder = ImmutableList.builder();
-    AspectKey baseKey = key;
-    while (baseKey != null) {
-      aspectKeysBuilder.add(baseKey.getSkyKey());
-      baseKey = baseKey.getBaseKey();
+  private ImmutableMap<AspectDescriptor, SkyKey> getSkyKeysForAspects(
+      ImmutableList<AspectKey> keys) {
+    HashMap<AspectDescriptor, SkyKey> result = new HashMap<>();
+    for (AspectKey key : keys) {
+      buildSkyKeys(key, result);
     }
-    return aspectKeysBuilder.build().reverse();
+    return ImmutableMap.copyOf(result);
   }
 
+  private void buildSkyKeys(AspectKey key, HashMap<AspectDescriptor, SkyKey> result) {
+    if (result.containsKey(key.getAspectDescriptor())) {
+      return;
+    }
+    ImmutableList<AspectKey> baseKeys = key.getBaseKeys();
+    result.put(key.getAspectDescriptor(), key.getSkyKey());
+    for (AspectKey baseKey : baseKeys) {
+      buildSkyKeys(baseKey, result);
+    }
+  }
   private static SkyValue createAliasAspect(
       Environment env,
       Target originalTarget,
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 7accbc1cb0..df6048df17 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -47,36 +48,22 @@ public final class AspectValue extends ActionLookupValue {
    */
   public static final class AspectKey extends AspectValueKey {
     private final Label label;
-    private final AspectKey baseKey;
+    private final ImmutableList<AspectKey> baseKeys;
     private final BuildConfiguration aspectConfiguration;
     private final BuildConfiguration baseConfiguration;
-    private final AspectClass aspectClass;
-    private final AspectParameters parameters;
+    private final AspectDescriptor aspectDescriptor;
 
     private AspectKey(
         Label label,
-        BuildConfiguration aspectConfiguration,
         BuildConfiguration baseConfiguration,
-        AspectClass aspectClass,
-        AspectParameters parameters) {
-      this.baseKey = null;
+        ImmutableList<AspectKey> baseKeys,
+        AspectDescriptor aspectDescriptor,
+        BuildConfiguration aspectConfiguration) {
+      this.baseKeys = baseKeys;
       this.label = label;
-      this.aspectConfiguration = aspectConfiguration;
       this.baseConfiguration = baseConfiguration;
-      this.aspectClass = aspectClass;
-      this.parameters = parameters;
-    }
-
-    private AspectKey(
-        AspectKey baseKey,
-        AspectClass aspectClass, AspectParameters aspectParameters,
-        BuildConfiguration aspectConfiguration) {
-      this.baseKey = baseKey;
-      this.label = baseKey.label;
-      this.baseConfiguration = baseKey.getBaseConfiguration();
       this.aspectConfiguration = aspectConfiguration;
-      this.aspectClass = aspectClass;
-      this.parameters = aspectParameters;
+      this.aspectDescriptor = aspectDescriptor;
     }
 
     @Override
@@ -91,25 +78,31 @@ public final class AspectValue extends ActionLookupValue {
     }
 
     public AspectClass getAspectClass() {
-      return aspectClass;
+      return aspectDescriptor.getAspectClass();
     }
 
     @Nullable
     public AspectParameters getParameters() {
-      return parameters;
+      return aspectDescriptor.getParameters();
+    }
+
+    public AspectDescriptor getAspectDescriptor() {
+      return aspectDescriptor;
     }
 
     @Nullable
-    public AspectKey getBaseKey() {
-      return baseKey;
+    public ImmutableList<AspectKey> getBaseKeys() {
+      return baseKeys;
     }
 
     @Override
     public String getDescription() {
-      if (baseKey == null) {
-        return String.format("%s of %s", aspectClass.getName(), getLabel());
+      if (baseKeys.isEmpty()) {
+        return String.format("%s of %s",
+            aspectDescriptor.getAspectClass().getName(), getLabel());
       } else {
-        return String.format("%s on top of %s", aspectClass.getName(), baseKey.toString());
+        return String.format("%s on top of %s",
+            aspectDescriptor.getAspectClass().getName(), baseKeys.toString());
       }
     }
 
@@ -153,11 +146,10 @@ public final class AspectValue extends ActionLookupValue {
     public int hashCode() {
       return Objects.hashCode(
           label,
-          baseKey,
+          baseKeys,
           aspectConfiguration,
           baseConfiguration,
-          aspectClass,
-          parameters);
+          aspectDescriptor);
     }
 
     @Override
@@ -172,45 +164,52 @@ public final class AspectValue extends ActionLookupValue {
 
       AspectKey that = (AspectKey) other;
       return Objects.equal(label, that.label)
-          && Objects.equal(baseKey, that.baseKey)
+          && Objects.equal(baseKeys, that.baseKeys)
           && Objects.equal(aspectConfiguration, that.aspectConfiguration)
           && Objects.equal(baseConfiguration, that.baseConfiguration)
-          && Objects.equal(aspectClass, that.aspectClass)
-          && Objects.equal(parameters, that.parameters);
+          && Objects.equal(aspectDescriptor, that.aspectDescriptor);
     }
 
     public String prettyPrint() {
       if (label == null) {
         return "null";
       }
-      return String.format("%s with aspect %s%s",
-          baseKey == null ? label.toString() : baseKey.prettyPrint(),
-          aspectClass.getName(),
+
+      String baseKeysString =
+          baseKeys.isEmpty()
+          ? ""
+          : String.format(" (over %s)", baseKeys.toString());
+      return String.format("%s with aspect %s%s%s",
+          label.toString(),
+          aspectDescriptor.getAspectClass().getName(),
           (aspectConfiguration != null && aspectConfiguration.isHostConfiguration())
-              ? "(host) " : "");
+              ? "(host) " : "",
+          baseKeysString
+      );
     }
 
     @Override
     public String toString() {
-      return (baseKey == null ? label : baseKey.toString())
+      return (baseKeys == null ? label : baseKeys.toString())
           + "#"
-          + aspectClass.getName()
+          + aspectDescriptor.getAspectClass().getName()
           + " "
           + (aspectConfiguration == null ? "null" : aspectConfiguration.checksum())
           + " "
           + (baseConfiguration == null ? "null" : baseConfiguration.checksum())
           + " "
-          + parameters;
+          + aspectDescriptor.getParameters();
     }
 
     public AspectKey withLabel(Label label) {
-      if (baseKey == null) {
-        return new AspectKey(
-            label, aspectConfiguration, baseConfiguration, aspectClass, parameters);
-      } else {
-        return new AspectKey(
-            baseKey.withLabel(label), aspectClass, parameters, aspectConfiguration);
+      ImmutableList.Builder<AspectKey> newBaseKeys = ImmutableList.builder();
+      for (AspectKey baseKey : baseKeys) {
+        newBaseKeys.add(baseKey.withLabel(label));
       }
+
+      return new AspectKey(
+          label, baseConfiguration,
+          newBaseKeys.build(), aspectDescriptor, aspectConfiguration);
     }
   }
 
@@ -354,10 +353,14 @@ public final class AspectValue extends ActionLookupValue {
     return transitivePackages;
   }
 
-  public static AspectKey createAspectKey(AspectKey baseKey, AspectDescriptor aspectDescriptor,
+  public static AspectKey createAspectKey(
+      Label label, BuildConfiguration baseConfiguration,
+      ImmutableList<AspectKey> baseKeys, AspectDescriptor aspectDescriptor,
       BuildConfiguration aspectConfiguration) {
     return new AspectKey(
-        baseKey, aspectDescriptor.getAspectClass(), aspectDescriptor.getParameters(),
+        label, baseConfiguration,
+        baseKeys,
+        aspectDescriptor,
         aspectConfiguration
     );
   }
@@ -368,8 +371,8 @@ public final class AspectValue extends ActionLookupValue {
       BuildConfiguration baseConfiguration, AspectDescriptor aspectDescriptor,
       BuildConfiguration aspectConfiguration) {
     return new AspectKey(
-        label, aspectConfiguration, baseConfiguration,
-        aspectDescriptor.getAspectClass(), aspectDescriptor.getParameters());
+        label, baseConfiguration, ImmutableList.<AspectKey>of(),
+        aspectDescriptor, aspectConfiguration);
   }
 
 
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 e481dbd1d9..673a18fc40 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
@@ -31,6 +31,8 @@ import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.actions.Actions;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
+import com.google.devtools.build.lib.analysis.AspectCollection;
+import com.google.devtools.build.lib.analysis.AspectCollection.AspectDeps;
 import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -82,6 +84,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
@@ -849,18 +852,14 @@ final class ConfiguredTargetFunction implements SkyFunction {
       NestedSetBuilder<Package> transitivePackages)
       throws AspectCreationException, InterruptedException {
     OrderedSetMultimap<SkyKey, ConfiguredAspect> result = OrderedSetMultimap.create();
-    Set<SkyKey> aspectKeys = new HashSet<>();
+    Set<SkyKey> allAspectKeys = new HashSet<>();
     for (Dependency dep : deps) {
-      AspectKey key = null;
-      for (Entry<AspectDescriptor, BuildConfiguration> depAspect
-          : dep.getAspectConfigurations().entrySet()) {
-        key = getNextAspectKey(key, dep, depAspect);
-        aspectKeys.add(key.getSkyKey());
-      }
+      allAspectKeys.addAll(getAspectKeys(dep).values());
     }
 
     Map<SkyKey, ValueOrException2<AspectCreationException, NoSuchThingException>> depAspects =
-        env.getValuesOrThrow(aspectKeys, AspectCreationException.class, NoSuchThingException.class);
+        env.getValuesOrThrow(allAspectKeys,
+            AspectCreationException.class, NoSuchThingException.class);
 
     for (Dependency dep : deps) {
       SkyKey depKey = TO_KEYS.apply(dep);
@@ -869,12 +868,12 @@ final class ConfiguredTargetFunction implements SkyFunction {
       if (result.containsKey(depKey)) {
         continue;
       }
-      AspectKey key = null;
+      Map<AspectDescriptor, SkyKey> aspectToKeys = getAspectKeys(dep);
+
       ConfiguredTarget depConfiguredTarget = configuredTargetMap.get(depKey);
-      for (Entry<AspectDescriptor, BuildConfiguration> depAspect
-          : dep.getAspectConfigurations().entrySet()) {
-        key = getNextAspectKey(key, dep, depAspect);
-        SkyKey aspectKey = key.getSkyKey();
+      for (AspectDeps depAspect : dep.getAspects().getVisibleAspects()) {
+        SkyKey aspectKey = aspectToKeys.get(depAspect.getAspect());
+
         AspectValue aspectValue;
         try {
           // TODO(ulfjack): Catch all thrown AspectCreationException and NoSuchThingException
@@ -884,7 +883,7 @@ final class ConfiguredTargetFunction implements SkyFunction {
           throw new AspectCreationException(
               String.format(
                   "Evaluation of aspect %s on %s failed: %s",
-                  depAspect.getKey().getAspectClass().getName(),
+                  depAspect.getAspect().getAspectClass().getName(),
                   dep.getLabel(),
                   e.toString()));
         }
@@ -904,16 +903,32 @@ final class ConfiguredTargetFunction implements SkyFunction {
     return result;
   }
 
-  private static AspectKey getNextAspectKey(AspectKey key, Dependency dep,
-      Entry<AspectDescriptor, BuildConfiguration> depAspect) {
-    if (key == null) {
-      key = AspectValue.createAspectKey(dep.getLabel(),
-          dep.getConfiguration(), depAspect.getKey(), depAspect.getValue()
-      );
-    } else {
-      key = AspectValue.createAspectKey(key, depAspect.getKey(), depAspect.getValue());
+  private static Map<AspectDescriptor, SkyKey> getAspectKeys(Dependency dep) {
+    HashMap<AspectDescriptor, SkyKey> result = new HashMap<>();
+    AspectCollection aspects = dep.getAspects();
+    for (AspectDeps aspectDeps : aspects.getVisibleAspects()) {
+      buildAspectKey(aspectDeps, result, dep);
+    }
+    return result;
+  }
+
+  private static AspectKey buildAspectKey(AspectDeps aspectDeps,
+      HashMap<AspectDescriptor, SkyKey> result, Dependency dep) {
+    if (result.containsKey(aspectDeps.getAspect())) {
+      return (AspectKey) result.get(aspectDeps.getAspect()).argument();
+    }
+
+    ImmutableList.Builder<AspectKey> dependentAspects = ImmutableList.builder();
+    for (AspectDeps path : aspectDeps.getDependentAspects()) {
+      dependentAspects.add(buildAspectKey(path, result, dep));
     }
-    return key;
+    AspectKey aspectKey = AspectValue.createAspectKey(
+        dep.getLabel(), dep.getConfiguration(),
+        dependentAspects.build(),
+        aspectDeps.getAspect(),
+        dep.getAspectConfiguration(aspectDeps.getAspect()));
+    result.put(aspectKey.getAspectDescriptor(), aspectKey.getSkyKey());
+    return aspectKey;
   }
 
   private static boolean aspectMatchesConfiguredTarget(final ConfiguredTarget dep, Aspect aspect) {
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 10e565714b..46b2b9df47 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
@@ -50,6 +50,7 @@ import com.google.devtools.build.lib.actions.Executor;
 import com.google.devtools.build.lib.actions.PackageRootResolutionException;
 import com.google.devtools.build.lib.actions.ResourceManager;
 import com.google.devtools.build.lib.actions.Root;
+import com.google.devtools.build.lib.analysis.AspectCollection;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.BuildView.Options;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
@@ -1227,7 +1228,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       }
       for (BuildConfiguration depConfig : configs.get(key)) {
         skyKeys.add(ConfiguredTargetValue.key(key.getLabel(), depConfig));
-        for (AspectDescriptor aspectDescriptor : key.getAspects()) {
+        for (AspectDescriptor aspectDescriptor : key.getAspects().getAllAspects()) {
           skyKeys.add(ActionLookupValue.key(AspectValue.createAspectKey(key.getLabel(), depConfig,
               aspectDescriptor, depConfig)));
         }
@@ -1260,7 +1261,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
             ((ConfiguredTargetValue) result.get(configuredTargetKey)).getConfiguredTarget();
         List<ConfiguredAspect> configuredAspects = new ArrayList<>();
 
-        for (AspectDescriptor aspectDescriptor : key.getAspects()) {
+        for (AspectDescriptor aspectDescriptor : key.getAspects().getAllAspects()) {
           SkyKey aspectKey = ActionLookupValue.key(AspectValue.createAspectKey(key.getLabel(),
               depConfig, aspectDescriptor, depConfig));
           if (result.get(aspectKey) == null) {
@@ -1450,7 +1451,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       dep = Dependency.withNullConfiguration(label);
     } else if (configuration.useDynamicConfigurations()) {
       dep = Dependency.withTransitionAndAspects(label, Attribute.ConfigurationTransition.NONE,
-          ImmutableSet.<AspectDescriptor>of());
+          AspectCollection.EMPTY);
     } else {
       dep = Dependency.withConfiguration(label, configuration);
     }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectValueTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectValueTest.java
index 25477bd844..0714a0d253 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectValueTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectValueTest.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.testing.EqualsTester;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
@@ -234,7 +235,8 @@ public class AspectValueTest extends AnalysisTestCase {
     AspectKey baseKey = AspectValue.createAspectKey(label, baseConfiguration,
         new AspectDescriptor(aspectClass1, parameters1), aspectConfiguration1);
     return ActionLookupValue.key(AspectValue.createAspectKey(
-        baseKey, new AspectDescriptor(aspectClass2, parameters2),
+        label, baseConfiguration,
+        ImmutableList.of(baseKey), new AspectDescriptor(aspectClass2, parameters2),
         aspectConfiguration2
     ));
   }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index d7be1ffaf4..28ce907211 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -27,7 +27,6 @@ import static org.junit.Assert.fail;
 
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
@@ -44,7 +43,6 @@ import com.google.devtools.build.lib.analysis.util.AnalysisMock;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestBase;
 import com.google.devtools.build.lib.analysis.util.ExpectedDynamicConfigurationErrors;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.skyframe.SkyFunctions;
@@ -371,7 +369,7 @@ public class BuildViewTest extends BuildViewTestBase {
           Dependency.withTransitionAndAspects(
               Label.parseAbsolute("//package:inner"),
               Attribute.ConfigurationTransition.NONE,
-              ImmutableSet.<AspectDescriptor>of());
+              AspectCollection.EMPTY);
     } else {
       innerDependency =
           Dependency.withConfiguration(
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java
index eb6b55194c..09a2c17760 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java
@@ -135,7 +135,7 @@ public class DependencyResolverTest extends AnalysisTestCase {
     }
 
     assertNotNull("Dependency '" + dep + "' on attribute '" + attrName + "' not found", dependency);
-    assertThat(dependency.getAspects()).containsExactly((Object[]) aspects);
+    assertThat(dependency.getAspects().getAllAspects()).containsExactly((Object[]) aspects);
     return dependency;
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/DependencyTest.java b/src/test/java/com/google/devtools/build/lib/analysis/DependencyTest.java
index 82e14721b4..03369f7e8c 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/DependencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/DependencyTest.java
@@ -26,8 +26,6 @@ import com.google.devtools.build.lib.analysis.util.TestAspects;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
-import java.util.LinkedHashSet;
-import java.util.Set;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -46,8 +44,7 @@ public class DependencyTest extends AnalysisTestCase {
     assertThat(nullDep.getLabel()).isEqualTo(Label.parseAbsolute("//a"));
     assertThat(nullDep.hasStaticConfiguration()).isTrue();
     assertThat(nullDep.getConfiguration()).isNull();
-    assertThat(nullDep.getAspects()).isEmpty();
-    assertThat(nullDep.getAspectConfigurations()).isEmpty();
+    assertThat(nullDep.getAspects().getAllAspects()).isEmpty();
 
     try {
       nullDep.getTransition();
@@ -66,8 +63,7 @@ public class DependencyTest extends AnalysisTestCase {
     assertThat(targetDep.getLabel()).isEqualTo(Label.parseAbsolute("//a"));
     assertThat(targetDep.hasStaticConfiguration()).isTrue();
     assertThat(targetDep.getConfiguration()).isEqualTo(getTargetConfiguration());
-    assertThat(targetDep.getAspects()).isEmpty();
-    assertThat(targetDep.getAspectConfigurations()).isEmpty();
+    assertThat(targetDep.getAspects().getAllAspects()).isEmpty();
 
     try {
       targetDep.getTransition();
@@ -82,7 +78,8 @@ public class DependencyTest extends AnalysisTestCase {
     update();
     AspectDescriptor simpleAspect = new AspectDescriptor(TestAspects.SIMPLE_ASPECT);
     AspectDescriptor attributeAspect = new AspectDescriptor(TestAspects.ATTRIBUTE_ASPECT);
-    ImmutableSet<AspectDescriptor> twoAspects = ImmutableSet.of(simpleAspect, attributeAspect);
+    AspectCollection twoAspects = AspectCollection.createForTests(
+        ImmutableSet.of(simpleAspect, attributeAspect));
     Dependency targetDep =
         Dependency.withConfigurationAndAspects(
             Label.parseAbsolute("//a"), getTargetConfiguration(), twoAspects);
@@ -90,12 +87,10 @@ public class DependencyTest extends AnalysisTestCase {
     assertThat(targetDep.getLabel()).isEqualTo(Label.parseAbsolute("//a"));
     assertThat(targetDep.hasStaticConfiguration()).isTrue();
     assertThat(targetDep.getConfiguration()).isEqualTo(getTargetConfiguration());
-    assertThat(targetDep.getAspects()).containsExactlyElementsIn(twoAspects);
-    assertThat(targetDep.getAspectConfigurations())
-        .containsExactlyEntriesIn(
-            ImmutableMap.of(
-                simpleAspect, getTargetConfiguration(),
-                attributeAspect, getTargetConfiguration()));
+    assertThat(targetDep.getAspects()).isEqualTo(twoAspects);
+    assertThat(targetDep.getAspectConfiguration(simpleAspect)).isEqualTo(getTargetConfiguration());
+    assertThat(targetDep.getAspectConfiguration(attributeAspect))
+        .isEqualTo(getTargetConfiguration());
 
     try {
       targetDep.getTransition();
@@ -106,27 +101,12 @@ public class DependencyTest extends AnalysisTestCase {
   }
 
   @Test
-  public void withConfigurationAndAspects_RejectsNullAspectsWithNPE() throws Exception {
-    update();
-    Set<AspectDescriptor> nullSet = new LinkedHashSet<>();
-    nullSet.add(null);
-
-    try {
-      Dependency.withConfigurationAndAspects(
-          Label.parseAbsolute("//a"), getTargetConfiguration(), nullSet);
-      fail("should not be allowed to create a dependency with a null aspect");
-    } catch (NullPointerException expected) {
-      // good. just as planned.
-    }
-  }
-
-  @Test
   public void withConfigurationAndAspects_RejectsNullConfigWithNPE() throws Exception {
     // Although the NullPointerTester should check this, this test invokes a different code path,
     // because it includes aspects (which the NPT test will not).
     AspectDescriptor simpleAspect = new AspectDescriptor(TestAspects.SIMPLE_ASPECT);
     AspectDescriptor attributeAspect = new AspectDescriptor(TestAspects.ATTRIBUTE_ASPECT);
-    ImmutableSet<AspectDescriptor> twoAspects = ImmutableSet.of(simpleAspect, attributeAspect);
+    AspectCollection twoAspects = AspectCollection.createForTests(simpleAspect, attributeAspect);
 
     try {
       Dependency.withConfigurationAndAspects(Label.parseAbsolute("//a"), null, twoAspects);
@@ -143,10 +123,9 @@ public class DependencyTest extends AnalysisTestCase {
         Dependency.withConfigurationAndAspects(
             Label.parseAbsolute("//a"),
             getTargetConfiguration(),
-            ImmutableSet.<AspectDescriptor>of());
+            AspectCollection.EMPTY);
     // Here we're also checking that this doesn't throw an exception. No boom? OK. Good.
-    assertThat(dep.getAspects()).isEmpty();
-    assertThat(dep.getAspectConfigurations()).isEmpty();
+    assertThat(dep.getAspects().getAllAspects()).isEmpty();
   }
 
   @Test
@@ -154,18 +133,22 @@ public class DependencyTest extends AnalysisTestCase {
     update();
     AspectDescriptor simpleAspect = new AspectDescriptor(TestAspects.SIMPLE_ASPECT);
     AspectDescriptor attributeAspect = new AspectDescriptor(TestAspects.ATTRIBUTE_ASPECT);
+    AspectCollection aspects =
+        AspectCollection.createForTests(ImmutableSet.of(simpleAspect, attributeAspect));
     ImmutableMap<AspectDescriptor, BuildConfiguration> twoAspectMap = ImmutableMap.of(
         simpleAspect, getTargetConfiguration(), attributeAspect, getHostConfiguration());
     Dependency targetDep =
         Dependency.withConfiguredAspects(
-            Label.parseAbsolute("//a"), getTargetConfiguration(), twoAspectMap);
+            Label.parseAbsolute("//a"), getTargetConfiguration(), aspects, twoAspectMap);
 
     assertThat(targetDep.getLabel()).isEqualTo(Label.parseAbsolute("//a"));
     assertThat(targetDep.hasStaticConfiguration()).isTrue();
     assertThat(targetDep.getConfiguration()).isEqualTo(getTargetConfiguration());
-    assertThat(targetDep.getAspects())
+    assertThat(targetDep.getAspects().getAllAspects())
         .containsExactlyElementsIn(ImmutableSet.of(simpleAspect, attributeAspect));
-    assertThat(targetDep.getAspectConfigurations()).containsExactlyEntriesIn(twoAspectMap);
+    assertThat(targetDep.getAspectConfiguration(simpleAspect)).isEqualTo(getTargetConfiguration());
+    assertThat(targetDep.getAspectConfiguration(attributeAspect))
+        .isEqualTo(getHostConfiguration());
 
     try {
       targetDep.getTransition();
@@ -182,24 +165,26 @@ public class DependencyTest extends AnalysisTestCase {
     Dependency dep =
         Dependency.withConfiguredAspects(
             Label.parseAbsolute("//a"), getTargetConfiguration(),
+            AspectCollection.EMPTY,
             ImmutableMap.<AspectDescriptor, BuildConfiguration>of());
     // Here we're also checking that this doesn't throw an exception. No boom? OK. Good.
-    assertThat(dep.getAspects()).isEmpty();
-    assertThat(dep.getAspectConfigurations()).isEmpty();
+    assertThat(dep.getAspects().getAllAspects()).isEmpty();
   }
 
   @Test
   public void withTransitionAndAspects_BasicAccessors() throws Exception {
     AspectDescriptor simpleAspect = new AspectDescriptor(TestAspects.SIMPLE_ASPECT);
     AspectDescriptor attributeAspect = new AspectDescriptor(TestAspects.ATTRIBUTE_ASPECT);
-    ImmutableSet<AspectDescriptor> twoAspects = ImmutableSet.of(simpleAspect, attributeAspect);
+    AspectCollection twoAspects = AspectCollection.createForTests(
+        ImmutableSet.of(simpleAspect, attributeAspect));
     Dependency hostDep =
         Dependency.withTransitionAndAspects(
             Label.parseAbsolute("//a"), ConfigurationTransition.HOST, twoAspects);
 
     assertThat(hostDep.getLabel()).isEqualTo(Label.parseAbsolute("//a"));
     assertThat(hostDep.hasStaticConfiguration()).isFalse();
-    assertThat(hostDep.getAspects()).containsExactlyElementsIn(twoAspects);
+    assertThat(hostDep.getAspects().getAllAspects())
+        .containsExactlyElementsIn(twoAspects.getAllAspects());
     assertThat(hostDep.getTransition()).isEqualTo(ConfigurationTransition.HOST);
 
     try {
@@ -210,9 +195,17 @@ public class DependencyTest extends AnalysisTestCase {
     }
 
     try {
-      hostDep.getAspectConfigurations();
+      hostDep.getAspectConfiguration(simpleAspect);
+      fail("withTransitionAndAspects-created Dependencies should throw ISE on "
+          + "getAspectConfiguration()");
+    } catch (IllegalStateException ex) {
+      // good. you're so predictable.
+    }
+
+    try {
+      hostDep.getAspectConfiguration(attributeAspect);
       fail("withTransitionAndAspects-created Dependencies should throw ISE on "
-          + "getAspectConfigurations()");
+          + "getAspectConfiguration()");
     } catch (IllegalStateException ex) {
       // good. you're so predictable.
     }
@@ -224,9 +217,9 @@ public class DependencyTest extends AnalysisTestCase {
     Dependency dep =
         Dependency.withTransitionAndAspects(
             Label.parseAbsolute("//a"), ConfigurationTransition.HOST,
-            ImmutableSet.<AspectDescriptor>of());
+            AspectCollection.EMPTY);
     // Here we're also checking that this doesn't throw an exception. No boom? OK. Good.
-    assertThat(dep.getAspects()).isEmpty();
+    assertThat(dep.getAspects().getAllAspects()).isEmpty();
   }
 
   @Test
@@ -254,10 +247,13 @@ public class DependencyTest extends AnalysisTestCase {
     AspectDescriptor attributeAspect = new AspectDescriptor(TestAspects.ATTRIBUTE_ASPECT);
     AspectDescriptor errorAspect = new AspectDescriptor(TestAspects.ERROR_ASPECT);
 
-    ImmutableSet<AspectDescriptor> twoAspects = ImmutableSet.of(simpleAspect, attributeAspect);
-    ImmutableSet<AspectDescriptor> inverseAspects = ImmutableSet.of(attributeAspect, simpleAspect);
-    ImmutableSet<AspectDescriptor> differentAspects = ImmutableSet.of(attributeAspect, errorAspect);
-    ImmutableSet<AspectDescriptor> noAspects = ImmutableSet.<AspectDescriptor>of();
+    AspectCollection twoAspects =
+        AspectCollection.createForTests(simpleAspect, attributeAspect);
+    AspectCollection inverseAspects =
+        AspectCollection.createForTests(attributeAspect, simpleAspect);
+    AspectCollection differentAspects =
+        AspectCollection.createForTests(attributeAspect, errorAspect);
+    AspectCollection noAspects = AspectCollection.EMPTY;
 
     ImmutableMap<AspectDescriptor, BuildConfiguration> twoAspectsHostMap =
         ImmutableMap.of(simpleAspect, host, attributeAspect, host);
@@ -277,21 +273,21 @@ public class DependencyTest extends AnalysisTestCase {
             Dependency.withConfigurationAndAspects(aExplicit, host, twoAspects),
             Dependency.withConfigurationAndAspects(a, host, inverseAspects),
             Dependency.withConfigurationAndAspects(aExplicit, host, inverseAspects),
-            Dependency.withConfiguredAspects(a, host, twoAspectsHostMap),
-            Dependency.withConfiguredAspects(aExplicit, host, twoAspectsHostMap))
+            Dependency.withConfiguredAspects(a, host, twoAspects, twoAspectsHostMap),
+            Dependency.withConfiguredAspects(aExplicit, host, twoAspects, twoAspectsHostMap))
         .addEqualityGroup(
             // base set but with label //b
             Dependency.withConfigurationAndAspects(b, host, twoAspects),
             Dependency.withConfigurationAndAspects(b, host, inverseAspects),
-            Dependency.withConfiguredAspects(b, host, twoAspectsHostMap))
+            Dependency.withConfiguredAspects(b, host, twoAspects, twoAspectsHostMap))
         .addEqualityGroup(
             // base set but with target configuration
             Dependency.withConfigurationAndAspects(a, target, twoAspects),
             Dependency.withConfigurationAndAspects(aExplicit, target, twoAspects),
             Dependency.withConfigurationAndAspects(a, target, inverseAspects),
             Dependency.withConfigurationAndAspects(aExplicit, target, inverseAspects),
-            Dependency.withConfiguredAspects(a, target, twoAspectsTargetMap),
-            Dependency.withConfiguredAspects(aExplicit, target, twoAspectsTargetMap))
+            Dependency.withConfiguredAspects(a, target, twoAspects, twoAspectsTargetMap),
+            Dependency.withConfiguredAspects(aExplicit, target, twoAspects, twoAspectsTargetMap))
         .addEqualityGroup(
             // base set but with null configuration
             Dependency.withNullConfiguration(a),
@@ -300,56 +296,63 @@ public class DependencyTest extends AnalysisTestCase {
             // base set but with different aspects
             Dependency.withConfigurationAndAspects(a, host, differentAspects),
             Dependency.withConfigurationAndAspects(aExplicit, host, differentAspects),
-            Dependency.withConfiguredAspects(a, host, differentAspectsHostMap),
-            Dependency.withConfiguredAspects(aExplicit, host, differentAspectsHostMap))
+            Dependency.withConfiguredAspects(
+                a, host, differentAspects, differentAspectsHostMap),
+            Dependency.withConfiguredAspects(
+                aExplicit, host, differentAspects, differentAspectsHostMap))
         .addEqualityGroup(
             // base set but with label //b and target configuration
             Dependency.withConfigurationAndAspects(b, target, twoAspects),
             Dependency.withConfigurationAndAspects(b, target, inverseAspects),
-            Dependency.withConfiguredAspects(b, target, twoAspectsTargetMap))
+            Dependency.withConfiguredAspects(b, target,
+                twoAspects, twoAspectsTargetMap))
         .addEqualityGroup(
             // base set but with label //b and null configuration
             Dependency.withNullConfiguration(b))
         .addEqualityGroup(
             // base set but with label //b and different aspects
             Dependency.withConfigurationAndAspects(b, host, differentAspects),
-            Dependency.withConfiguredAspects(b, host, differentAspectsHostMap))
+            Dependency.withConfiguredAspects(
+                b, host, differentAspects, differentAspectsHostMap))
         .addEqualityGroup(
             // base set but with target configuration and different aspects
             Dependency.withConfigurationAndAspects(a, target, differentAspects),
             Dependency.withConfigurationAndAspects(aExplicit, target, differentAspects),
-            Dependency.withConfiguredAspects(a, target, differentAspectsTargetMap),
-            Dependency.withConfiguredAspects(aExplicit, target, differentAspectsTargetMap))
+            Dependency.withConfiguredAspects(
+                a, target, differentAspects, differentAspectsTargetMap),
+            Dependency.withConfiguredAspects(
+                aExplicit, target, differentAspects, differentAspectsTargetMap))
         .addEqualityGroup(
             // inverse of base set: //b, target configuration, different aspects
             Dependency.withConfigurationAndAspects(b, target, differentAspects),
-            Dependency.withConfiguredAspects(b, target, differentAspectsTargetMap))
+            Dependency.withConfiguredAspects(
+                b, target, differentAspects, differentAspectsTargetMap))
         .addEqualityGroup(
             // base set but with no aspects
             Dependency.withConfiguration(a, host),
             Dependency.withConfiguration(aExplicit, host),
             Dependency.withConfigurationAndAspects(a, host, noAspects),
             Dependency.withConfigurationAndAspects(aExplicit, host, noAspects),
-            Dependency.withConfiguredAspects(a, host, noAspectsMap),
-            Dependency.withConfiguredAspects(aExplicit, host, noAspectsMap))
+            Dependency.withConfiguredAspects(a, host, noAspects, noAspectsMap),
+            Dependency.withConfiguredAspects(aExplicit, host, noAspects, noAspectsMap))
         .addEqualityGroup(
             // base set but with label //b and no aspects
             Dependency.withConfiguration(b, host),
             Dependency.withConfigurationAndAspects(b, host, noAspects),
-            Dependency.withConfiguredAspects(b, host, noAspectsMap))
+            Dependency.withConfiguredAspects(b, host, noAspects, noAspectsMap))
         .addEqualityGroup(
             // base set but with target configuration and no aspects
             Dependency.withConfiguration(a, target),
             Dependency.withConfiguration(aExplicit, target),
             Dependency.withConfigurationAndAspects(a, target, noAspects),
             Dependency.withConfigurationAndAspects(aExplicit, target, noAspects),
-            Dependency.withConfiguredAspects(a, target, noAspectsMap),
-            Dependency.withConfiguredAspects(aExplicit, target, noAspectsMap))
+            Dependency.withConfiguredAspects(a, target, noAspects, noAspectsMap),
+            Dependency.withConfiguredAspects(aExplicit, target, noAspects, noAspectsMap))
         .addEqualityGroup(
             // inverse of base set: //b, target configuration, no aspects
             Dependency.withConfiguration(b, target),
             Dependency.withConfigurationAndAspects(b, target, noAspects),
-            Dependency.withConfiguredAspects(b, target, noAspectsMap))
+            Dependency.withConfiguredAspects(b, target, noAspects, noAspectsMap))
         .addEqualityGroup(
             // base set but with transition HOST
             Dependency.withTransitionAndAspects(a, ConfigurationTransition.HOST, twoAspects),
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java
index 44eff26e7f..4ba7fb8d5b 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java
@@ -1464,7 +1464,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
         "a1p = provider()",
         "def _a1_impl(target,ctx):",
         "  return struct(a1p = a1p(text = 'random'))",
-        "a1 = aspect(_a1_impl, attr_aspects = ['dep'])",
+        "a1 = aspect(_a1_impl, attr_aspects = ['dep'], provides = ['a1p'])",
         "a2p = provider()",
         "def _a2_impl(target,ctx):",
         "  value = []",
@@ -1475,7 +1475,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
         "  else:",
         "     value.append(str(target.label) + str(target.aspect_ids) + '=no')",
         "  return struct(a2p = a2p(value = value))",
-        "a2 = aspect(_a2_impl, attr_aspects = ['dep'])",
+        "a2 = aspect(_a2_impl, attr_aspects = ['dep'], required_aspect_providers = ['a1p'])",
         "def _r1_impl(ctx):",
         "  pass",
         "def _r2_impl(ctx):",
@@ -1499,7 +1499,6 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
         "//test:r0[\"//test:aspect.bzl%a1\"]=yes",
         "//test:r1[]=no");
   }
-
   /**
    * Diamond case.
    * rule r1 depends or r0 with aspect a1.
@@ -1515,11 +1514,11 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
         "test/aspect.bzl",
         "def _a1_impl(target,ctx):",
         "  return struct(a1p = 'text from a1')",
-        "a1 = aspect(_a1_impl, attr_aspects = ['deps'])",
+        "a1 = aspect(_a1_impl, attr_aspects = ['deps'], provides = ['a1p'])",
         "",
         "def _a2_impl(target,ctx):",
         "  return struct(a2p = 'text from a2')",
-        "a2 = aspect(_a2_impl, attr_aspects = ['deps'])",
+        "a2 = aspect(_a2_impl, attr_aspects = ['deps'], provides = ['a2p'])",
         "",
         "def _a3_impl(target,ctx):",
         "  value = []",
@@ -1535,7 +1534,8 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
         "     s += 'a2p'",
         "  value.append(s)",
         "  return struct(a3p = value)",
-        "a3 = aspect(_a3_impl, attr_aspects = ['deps'])",
+        "a3 = aspect(_a3_impl, attr_aspects = ['deps'],",
+        "            required_aspect_providers = [['a1p'], ['a2p']])",
         "def _r1_impl(ctx):",
         "  pass",
         "def _rcollect_impl(ctx):",
@@ -1572,6 +1572,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
    * r1 depends on r2_1 with aspect a1.
    * r2 depends on r1 with aspect a2.
    *
+   * a2 is not interested in a1.
    * There should be just one instance of aspect a2 on r0, and is should *not* see a1.
    */
   @Test
@@ -1581,7 +1582,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
         "a1p = provider()",
         "def _a1_impl(target,ctx):",
         "  return struct(a1p = 'a1p')",
-        "a1 = aspect(_a1_impl, attr_aspects = ['dep'])",
+        "a1 = aspect(_a1_impl, attr_aspects = ['dep'], provides = ['a1p'])",
         "a2p = provider()",
         "def _a2_impl(target,ctx):",
         "  value = []",
@@ -1592,7 +1593,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
         "  else:",
         "     value.append(str(target.label) + str(target.aspect_ids) + '=no')",
         "  return struct(a2p = a2p(value = value))",
-        "a2 = aspect(_a2_impl, attr_aspects = ['dep'])",
+        "a2 = aspect(_a2_impl, attr_aspects = ['dep'], required_aspect_providers = [])",
         "def _r1_impl(ctx):",
         "  pass",
         "def _r2_impl(ctx):",
@@ -1614,7 +1615,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
     // "yes" means that aspect a2 sees a1's providers.
     assertThat(result).containsExactly("//test:r0[]=no",
         "//test:r1[]=no",
-        "//test:r2_1[\"//test:aspect.bzl%a1\"]=yes");
+        "//test:r2_1[]=no");
   }
 
   /**
@@ -1627,7 +1628,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
         "a1p = provider()",
         "def _a1_impl(target,ctx):",
         "  return struct(a1p = a1p(text = 'random'))",
-        "a1 = aspect(_a1_impl, attr_aspects = ['dep'])",
+        "a1 = aspect(_a1_impl, attr_aspects = ['dep'], provides = ['a1p'])",
         "a2p = provider()",
         "def _a2_impl(target,ctx):",
         "  value = []",
@@ -1638,7 +1639,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
         "  else:",
         "     value.append(str(target.label) + str(target.aspect_ids) + '=no')",
         "  return struct(a2p = a2p(value = value))",
-        "a2 = aspect(_a2_impl, attr_aspects = ['dep'])",
+        "a2 = aspect(_a2_impl, attr_aspects = ['dep'], required_aspect_providers = ['a1p'])",
         "def _r1_impl(ctx):",
         "  pass",
         "def _r2_impl(ctx):",
@@ -1664,10 +1665,6 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
         "//test:r1[]=no");
   }
 
-
-  /**
-   * Linear aspects-on-aspects with alias rule.
-   */
   @Test
   public void aspectDescriptions() throws Exception {
     scratch.file(