Automated rollback of commit 943c83a.

*** Reason for rollback ***

This CL causes memory regression for single top-level aspect described in
b/193314770

*** Original change description ***

Command line aspect-on-aspect

This CL supports aspect-on-aspect for command line aspects. Command line aspects specified via `--aspects` option will support a top-level aspect requiring aspect providers via `required_aspect_providers` to get their values from other top-level aspects advertising it that come before it in the `--aspects` list.

PiperOrigin-RevId: 384430487

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

@@ -30,7 +30,7 @@
 import com.google.devtools.build.lib.causes.Cause;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
+import com.google.devtools.build.lib.skyframe.AspectValueKey;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import java.util.Collection;
 import javax.annotation.Nullable;
@@ -40,20 +40,20 @@
  * target cannot be completed because of an error in one of its dependencies.
  */
 public class AnalysisFailureEvent implements BuildEvent {
-  @Nullable private final AspectKey failedAspect;
+  @Nullable private final AspectValueKey failedAspect;
   private final ConfiguredTargetKey failedTarget;
   private final BuildEventId configuration;
   private final NestedSet<Cause> rootCauses;
 
   public AnalysisFailureEvent(
       ActionLookupKey failedTarget, BuildEventId configuration, NestedSet<Cause> rootCauses) {
     Preconditions.checkArgument(
-        failedTarget instanceof ConfiguredTargetKey || failedTarget instanceof AspectKey);
+        failedTarget instanceof ConfiguredTargetKey || failedTarget instanceof AspectValueKey);
     if (failedTarget instanceof ConfiguredTargetKey) {
       this.failedAspect = null;
       this.failedTarget = (ConfiguredTargetKey) failedTarget;
     } else {
-      this.failedAspect = (AspectKey) failedTarget;
+      this.failedAspect = (AspectValueKey) failedTarget;
       this.failedTarget = failedAspect.getBaseConfiguredTargetKey();
     }
     if (configuration != null) {
@@ -65,7 +65,7 @@ public AnalysisFailureEvent(
   }
 
   public AnalysisFailureEvent(
-      AspectKey failedAspect, BuildEventId configuration, NestedSet<Cause> rootCauses) {
+      AspectValueKey failedAspect, BuildEventId configuration, NestedSet<Cause> rootCauses) {
     this.failedAspect = failedAspect;
     this.failedTarget = failedAspect.getBaseConfiguredTargetKey();
     if (configuration != null) {

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

@@ -55,12 +55,13 @@
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.AspectClass;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
+import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.NativeAspectClass;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.Rule;
-import com.google.devtools.build.lib.packages.StarlarkAspectClass;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.pkgcache.PackageManager;
@@ -74,7 +75,6 @@
 import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns.Code;
 import com.google.devtools.build.lib.skyframe.AspectValueKey;
 import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
-import com.google.devtools.build.lib.skyframe.AspectValueKey.TopLevelAspectsKey;
 import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import com.google.devtools.build.lib.skyframe.CoverageReportValue;
@@ -86,6 +86,7 @@
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.RegexFilter;
 import com.google.devtools.build.skyframe.WalkableGraph;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -274,7 +275,10 @@ public AnalysisResult update(
             .map(TargetAndConfiguration::getConfiguredTargetKey)
             .collect(Collectors.toList());
 
-    ImmutableList.Builder<AspectClass> aspectClassesBuilder = ImmutableList.builder();
+    Multimap<Pair<Label, String>, BuildConfiguration> aspectConfigurations =
+        ArrayListMultimap.create();
+
+    List<AspectValueKey> aspectKeys = new ArrayList<>();
     for (String aspect : aspects) {
       // Syntax: label%aspect
       int delimiterPosition = aspect.indexOf('%');
@@ -314,14 +318,38 @@ public AnalysisResult update(
               createFailureDetail(errorMessage, Analysis.Code.ASPECT_LABEL_SYNTAX_ERROR),
               e);
         }
+
         String starlarkFunctionName = aspect.substring(delimiterPosition + 1);
-        aspectClassesBuilder.add(new StarlarkAspectClass(starlarkFileLabel, starlarkFunctionName));
+        for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
+          aspectConfigurations.put(
+              Pair.of(targetSpec.getLabel(), aspect), targetSpec.getConfiguration());
+          aspectKeys.add(
+              AspectValueKey.createStarlarkAspectKey(
+                  targetSpec.getLabel(),
+                  // For invoking top-level aspects, use the top-level configuration for both the
+                  // aspect and the base target while the top-level configuration is untrimmed.
+                  targetSpec.getConfiguration(),
+                  targetSpec.getConfiguration(),
+                  starlarkFileLabel,
+                  starlarkFunctionName));
+        }
       } else {
         final NativeAspectClass aspectFactoryClass =
             ruleClassProvider.getNativeAspectClassMap().get(aspect);
 
         if (aspectFactoryClass != null) {
-          aspectClassesBuilder.add(aspectFactoryClass);
+          for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
+            // For invoking top-level aspects, use the top-level configuration for both the
+            // aspect and the base target while the top-level configuration is untrimmed.
+            BuildConfiguration configuration = targetSpec.getConfiguration();
+            aspectConfigurations.put(Pair.of(targetSpec.getLabel(), aspect), configuration);
+            aspectKeys.add(
+                AspectValueKey.createAspectKey(
+                    targetSpec.getLabel(),
+                    configuration,
+                    new AspectDescriptor(aspectFactoryClass, AspectParameters.EMPTY),
+                    configuration));
+          }
         } else {
           String errorMessage = "Aspect '" + aspect + "' is unknown";
           throw new ViewCreationFailedException(
@@ -330,25 +358,6 @@ public AnalysisResult update(
       }
     }
 
-    Multimap<Pair<Label, String>, BuildConfiguration> aspectConfigurations =
-        ArrayListMultimap.create();
-    ImmutableList<AspectClass> aspectClasses = aspectClassesBuilder.build();
-    ImmutableList.Builder<TopLevelAspectsKey> aspectsKeys = ImmutableList.builder();
-    for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
-      BuildConfiguration configuration = targetSpec.getConfiguration();
-      for (AspectClass aspectClass : aspectClasses) {
-        aspectConfigurations.put(
-            Pair.of(targetSpec.getLabel(), aspectClass.getName()), configuration);
-      }
-      // For invoking top-level aspects, use the top-level configuration for both the
-      // aspect and the base target while the top-level configuration is untrimmed.
-      if (!aspectClasses.isEmpty()) {
-        aspectsKeys.add(
-            AspectValueKey.createTopLevelAspectsKey(
-                aspectClasses, targetSpec.getLabel(), configuration));
-      }
-    }
-
     for (Pair<Label, String> target : aspectConfigurations.keys()) {
       eventBus.post(
           new AspectConfiguredEvent(
@@ -373,7 +382,7 @@ public AnalysisResult update(
           skyframeBuildView.configureTargets(
               eventHandler,
               topLevelCtKeys,
-              aspectsKeys.build(),
+              aspectKeys,
               Suppliers.memoize(configurationLookupSupplier),
               topLevelOptions,
               eventBus,

src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java

@@ -297,18 +297,11 @@ public class BuildRequestOptions extends OptionsBase {
       effectTags = {OptionEffectTag.UNKNOWN},
       allowMultiple = true,
       help =
-          "Comma-separated list of aspects to be applied to top-level targets. All aspects are"
-              + " applied to all top-level targets. If aspect <code>some_aspect</code> specifies"
-              + " required aspect providers via <code>required_aspect_providers</code>,"
-              + " <code>some_aspect</code> will run after every aspect that was mentioned before it"
-              + " in the aspects list and whose advertised providers satisfy"
-              + " <code>some_aspect</code> required aspect providers. <code>some_aspect</code> will"
-              + " then have access to the values of those aspects' providers. Aspects that do not"
-              + " have such dependency will run independently on the top-level targets."
-              + ""
-              + " Aspects are specified in the form <bzl-file-label>%<aspect_name>, for example"
-              + " '//tools:my_def.bzl%my_aspect', where 'my_aspect' is a top-level value from a"
-              + " file tools/my_def.bzl")
+          "Comma-separated list of aspects to be applied to top-level targets. All aspects "
+              + "are applied to all top-level targets independently. Aspects are specified in "
+              + "the form <bzl-file-label>%<aspect_name>, "
+              + "for example '//tools:my_def.bzl%my_aspect', where 'my_aspect' is a top-level "
+              + "value from from a file tools/my_def.bzl")
   public List<String> aspects;
 
   public BuildRequestOptions() throws OptionsParsingException {}

src/main/java/com/google/devtools/build/lib/packages/StarlarkAspectClass.java

@@ -64,9 +64,4 @@ public boolean equals(Object o) {
   public int hashCode() {
     return Objects.hash(extensionLabel, exportedName);
   }
-
-  @Override
-  public String toString() {
-    return getName();
-  }
 }