Roll forward of https://github.com/bazelbuild/bazel/commit/943c83aa58731c4f9561d79c458f254427a8f24c: Command line aspect-on-aspect

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.

NEW:
- Add `incompatible_ignore_duplicate_top_level_aspects` flag to allow duplicates in `--aspects` list. The flag is set to true by default, otherwise a validation error will be thrown in case of duplicates in top-level aspects.
- Fix the error reporting for duplicate native aspects in `--aspects` list to be reported as a SkyFunction exception instead of crashing with assertion error.

Automated rollback of commit 7b4f9826d2d38ac7d071a4ada7b8a40a7a78226d.

*** Reason for rollback ***

Guard the validation against duplicate aspects in `--aspects` list by a flag to avoid breaking builds with duplicate aspects.

*** Original change description ***

Automated rollback of commit 7649f610c45190735fd7de433b15679b21b2d91b.

*** Reason for rollback ***

The added validation to prevent duplicate aspects in --aspects list breaks //production/datapush/modular/implementations/build:buildtarget_test

*** Original change description ***

Roll forward of https://github.com/bazelbuild/bazel/commit/943c83aa58731c4f9561d79c458f254427a8f24c: Command line aspect-on-aspect

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_a...

***

PiperOrigin-RevId: 389217989
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisFailureEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisFailureEvent.java
index 1e8a0ef..2a8445a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisFailureEvent.java
+++ b/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;
+import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import java.util.Collection;
 import javax.annotation.Nullable;
@@ -40,7 +40,7 @@
  * target cannot be completed because of an error in one of its dependencies.
  */
 public class AnalysisFailureEvent implements BuildEvent {
-  @Nullable private final AspectValueKey failedAspect;
+  @Nullable private final AspectKey failedAspect;
   private final ConfiguredTargetKey failedTarget;
   private final BuildEventId configuration;
   private final NestedSet<Cause> rootCauses;
@@ -48,12 +48,12 @@
   public AnalysisFailureEvent(
       ActionLookupKey failedTarget, BuildEventId configuration, NestedSet<Cause> rootCauses) {
     Preconditions.checkArgument(
-        failedTarget instanceof ConfiguredTargetKey || failedTarget instanceof AspectValueKey);
+        failedTarget instanceof ConfiguredTargetKey || failedTarget instanceof AspectKey);
     if (failedTarget instanceof ConfiguredTargetKey) {
       this.failedAspect = null;
       this.failedTarget = (ConfiguredTargetKey) failedTarget;
     } else {
-      this.failedAspect = (AspectValueKey) failedTarget;
+      this.failedAspect = (AspectKey) failedTarget;
       this.failedTarget = failedAspect.getBaseConfiguredTargetKey();
     }
     if (configuration != null) {
@@ -65,7 +65,7 @@
   }
 
   public AnalysisFailureEvent(
-      AspectValueKey failedAspect, BuildEventId configuration, NestedSet<Cause> rootCauses) {
+      AspectKey failedAspect, BuildEventId configuration, NestedSet<Cause> rootCauses) {
     this.failedAspect = failedAspect;
     this.failedTarget = failedAspect.getBaseConfiguredTargetKey();
     if (configuration != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java
index e8662a2..a39ddb6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java
@@ -166,4 +166,19 @@
               + " be used. Example value: \"HOST_CPUS*0.5\".",
       converter = CpuResourceConverter.class)
   public int oomSensitiveSkyFunctionsSemaphoreSize;
+
+  @Option(
+      name = "incompatible_ignore_duplicate_top_level_aspects",
+      defaultValue = "true",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      metadataTags = {
+        OptionMetadataTag.INCOMPATIBLE_CHANGE,
+        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      help =
+          "If true, remove duplicates from --aspects list by keeping only the first occurrence of"
+              + " every aspect. Otherwise, throw validation error if duplicate aspects are"
+              + " encountered.")
+  public boolean ignoreDuplicateTopLevelAspects;
 }
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 bec43a1..2e289f3 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
@@ -55,13 +55,12 @@
 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;
@@ -75,6 +74,7 @@
 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,7 +86,6 @@
 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;
@@ -275,10 +274,11 @@
             .map(TargetAndConfiguration::getConfiguredTargetKey)
             .collect(Collectors.toList());
 
-    Multimap<Pair<Label, String>, BuildConfiguration> aspectConfigurations =
-        ArrayListMultimap.create();
-
-    List<AspectValueKey> aspectKeys = new ArrayList<>();
+    ImmutableList.Builder<AspectClass> aspectClassesBuilder = ImmutableList.builder();
+    if (viewOptions.ignoreDuplicateTopLevelAspects) {
+      // remove duplicates from aspects list
+      aspects = ImmutableSet.copyOf(aspects).asList();
+    }
     for (String aspect : aspects) {
       // Syntax: label%aspect
       int delimiterPosition = aspect.indexOf('%');
@@ -318,38 +318,14 @@
               createFailureDetail(errorMessage, Analysis.Code.ASPECT_LABEL_SYNTAX_ERROR),
               e);
         }
-
         String starlarkFunctionName = aspect.substring(delimiterPosition + 1);
-        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));
-        }
+        aspectClassesBuilder.add(new StarlarkAspectClass(starlarkFileLabel, starlarkFunctionName));
       } else {
         final NativeAspectClass aspectFactoryClass =
             ruleClassProvider.getNativeAspectClassMap().get(aspect);
 
         if (aspectFactoryClass != null) {
-          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));
-          }
+          aspectClassesBuilder.add(aspectFactoryClass);
         } else {
           String errorMessage = "Aspect '" + aspect + "' is unknown";
           throw new ViewCreationFailedException(
@@ -358,6 +334,25 @@
       }
     }
 
+    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(
@@ -382,7 +377,7 @@
           skyframeBuildView.configureTargets(
               eventHandler,
               topLevelCtKeys,
-              aspectKeys,
+              aspectsKeys.build(),
               Suppliers.memoize(configurationLookupSupplier),
               topLevelOptions,
               eventBus,