Rollback of commit ac71950.

*** Reason for rollback ***

Uses tests that don't run on Bazel

*** Original change description ***

Implement aspect(...) Skylark function.

--
MOS_MIGRATED_REVID=105808850

src/main/java/com/google/devtools/build/lib/analysis/Aspect.java

@@ -22,7 +22,6 @@
 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 static class Builder {
     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 Builder addProvider(
       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 Builder addOutputGroup(String name, NestedSet<Artifact> artifacts) {
       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 Aspect build() {
         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));
     }
   }

src/main/java/com/google/devtools/build/lib/analysis/BuildView.java

@@ -42,7 +42,6 @@
 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.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 ConfiguredTargetKey apply(TargetAndConfiguration node) {
 
     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");
       }
     }
 

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java

@@ -286,14 +286,12 @@ private String missingFragmentError(
    * {@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(),

src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java

@@ -22,6 +22,7 @@
 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 @@ private RuleConfiguredTarget(RuleConfiguredTarget base, Iterable<Aspect> aspects
     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 @@ private RuleConfiguredTarget(RuleConfiguredTarget base, Iterable<Aspect> aspects
         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,44 +168,16 @@ private RuleConfiguredTarget(RuleConfiguredTarget base, Iterable<Aspect> aspects
         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;
     this.configConditions = base.configConditions;
     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 <P extends TransitiveInfoProvider> P getProvider(Class<P> providerClass)
    */
   @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 Rule getTarget() {
     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 String errorMessage(String name) {
   @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();
   }
 }

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.