Update AspectDefinition and StarlarkDefinedAspect to work with

ToolchainTypeRequirement.

Part of Optional Toolchains (#14726).

Closes #14946.

PiperOrigin-RevId: 432197702

src/main/java/com/google/devtools/build/lib/analysis/BUILD

@@ -298,6 +298,7 @@ java_library(
         ":config/per_label_options",
         ":config/run_under",
         ":config/starlark_defined_config_transition",
+        ":config/toolchain_type_requirement",
         ":config/transition_factories",
         ":config/transitions/composing_transition",
         ":config/transitions/composing_transition_factory",

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.analysis.starlark;
 
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.RUN_UNDER;
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.TEST_RUNNER_EXEC_GROUP;
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.TIMEOUT_DEFAULT;
@@ -43,6 +44,7 @@
 import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
 import com.google.devtools.build.lib.analysis.config.HostTransition;
 import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
+import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
 import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
 import com.google.devtools.build.lib.analysis.config.transitions.StarlarkExposedRuleTransitionFactory;
 import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
@@ -696,6 +698,7 @@ public StarlarkAspect aspect(
           "An aspect cannot simultaneously have required providers and apply to generating rules.");
     }
 
+    ImmutableList<Label> toolchainTypes = parseToolchains(toolchains, thread);
     return new StarlarkDefinedAspect(
         implementation,
         attrAspects.build(),
@@ -709,7 +712,9 @@ public StarlarkAspect aspect(
         ImmutableSet.copyOf(Sequence.cast(fragments, String.class, "fragments")),
         HostTransition.INSTANCE,
         ImmutableSet.copyOf(Sequence.cast(hostFragments, String.class, "host_fragments")),
-        parseToolchains(toolchains, thread),
+        toolchainTypes.stream()
+            .map(tt -> ToolchainTypeRequirement.create(tt))
+            .collect(toImmutableSet()),
         useToolchainTransition,
         applyToGeneratingRules);
   }

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

@@ -31,15 +31,14 @@
 import com.google.devtools.build.lib.packages.Type.LabelClass;
 import com.google.devtools.build.lib.packages.Type.LabelVisitor;
 import java.io.Serializable;
-import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
-import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.BiConsumer;
 import java.util.function.BiPredicate;
-import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
 /**
@@ -115,7 +114,6 @@ private AspectDefinition(
     this.advertisedProviders = advertisedProviders;
     this.requiredProviders = requiredProviders;
     this.requiredProvidersForAspects = requiredProvidersForAspects;
-
     this.attributes = attributes;
     this.toolchainTypes = toolchainTypes;
     this.useToolchainTransition = useToolchainTransition;
@@ -281,7 +279,7 @@ public static final class Builder {
         new ConfigurationFragmentPolicy.Builder();
     private boolean applyToFiles = false;
     private boolean applyToGeneratingRules = false;
-    private final List<ToolchainTypeRequirement> toolchainTypes = new ArrayList<>();
+    private final Set<ToolchainTypeRequirement> toolchainTypes = new HashSet<>();
     private boolean useToolchainTransition = false;
     private ImmutableSet<AspectClass> requiredAspectClasses = ImmutableSet.of();
 
@@ -587,25 +585,16 @@ public Builder applyToGeneratingRules(boolean applyToGeneratingRules) {
     }
 
     /** Adds the given toolchains as requirements for this aspect. */
-    public Builder addToolchainType(ToolchainTypeRequirement... toolchainTypes) {
-      return this.addToolchainType(ImmutableList.copyOf(toolchainTypes));
+    public Builder addToolchainTypes(ToolchainTypeRequirement... toolchainTypes) {
+      return this.addToolchainTypes(ImmutableSet.copyOf(toolchainTypes));
     }
 
     /** Adds the given toolchains as requirements for this aspect. */
-    public Builder addToolchainType(List<ToolchainTypeRequirement> toolchainTypes) {
+    public Builder addToolchainTypes(Collection<ToolchainTypeRequirement> toolchainTypes) {
       this.toolchainTypes.addAll(toolchainTypes);
       return this;
     }
 
-    /** Adds the given toolchains as requirements for this aspect. */
-    // TODO(katre): Remove this once all callers use addToolchainType.
-    public Builder addRequiredToolchains(List<Label> requiredToolchains) {
-      return addToolchainType(
-          requiredToolchains.stream()
-              .map(label -> ToolchainTypeRequirement.create(label))
-              .collect(Collectors.toList()));
-    }
-
     public Builder useToolchainTransition(boolean useToolchainTransition) {
       this.useToolchainTransition = useToolchainTransition;
       return this;

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

@@ -20,6 +20,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
 import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.EventHandler;
@@ -46,7 +47,7 @@ public final class StarlarkDefinedAspect implements StarlarkExportable, Starlark
   private final ImmutableSet<String> fragments;
   private final ConfigurationTransition hostTransition;
   private final ImmutableSet<String> hostFragments;
-  private final ImmutableList<Label> requiredToolchains;
+  private final ImmutableSet<ToolchainTypeRequirement> toolchainTypes;
   private final boolean useToolchainTransition;
   private final boolean applyToGeneratingRules;
 
@@ -71,7 +72,7 @@ public StarlarkDefinedAspect(
       // The host transition is in lib.analysis, so we can't reference it directly here.
       ConfigurationTransition hostTransition,
       ImmutableSet<String> hostFragments,
-      ImmutableList<Label> requiredToolchains,
+      ImmutableSet<ToolchainTypeRequirement> toolchainTypes,
       boolean useToolchainTransition,
       boolean applyToGeneratingRules) {
     this.implementation = implementation;
@@ -85,7 +86,7 @@ public StarlarkDefinedAspect(
     this.fragments = fragments;
     this.hostTransition = hostTransition;
     this.hostFragments = hostFragments;
-    this.requiredToolchains = requiredToolchains;
+    this.toolchainTypes = toolchainTypes;
     this.useToolchainTransition = useToolchainTransition;
     this.applyToGeneratingRules = applyToGeneratingRules;
   }
@@ -177,7 +178,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParams) {
     builder.advertiseProvider(advertisedStarlarkProviders.build());
     builder.requiresConfigurationFragmentsByStarlarkBuiltinName(fragments);
     builder.requiresConfigurationFragmentsByStarlarkBuiltinName(hostTransition, hostFragments);
-    builder.addRequiredToolchains(requiredToolchains);
+    builder.addToolchainTypes(toolchainTypes);
     builder.useToolchainTransition(useToolchainTransition);
     builder.applyToGeneratingRules(applyToGeneratingRules);
     ImmutableSet.Builder<AspectClass> requiredAspectsClasses = ImmutableSet.builder();
@@ -343,8 +344,8 @@ private Boolean parseBooleanParameter(String name, String value) throws EvalExce
         getName(), name, value);
   }
 
-  public ImmutableList<Label> getRequiredToolchains() {
-    return requiredToolchains;
+  public ImmutableSet<ToolchainTypeRequirement> getToolchainTypes() {
+    return toolchainTypes;
   }
 
   public boolean useToolchainTransition() {
@@ -395,7 +396,7 @@ public boolean equals(Object o) {
         && Objects.equals(fragments, that.fragments)
         && Objects.equals(hostTransition, that.hostTransition)
         && Objects.equals(hostFragments, that.hostFragments)
-        && Objects.equals(requiredToolchains, that.requiredToolchains)
+        && Objects.equals(toolchainTypes, that.toolchainTypes)
         && useToolchainTransition == that.useToolchainTransition
         && Objects.equals(aspectClass, that.aspectClass);
   }
@@ -414,7 +415,7 @@ public int hashCode() {
         fragments,
         hostTransition,
         hostFragments,
-        requiredToolchains,
+        toolchainTypes,
         useToolchainTransition,
         aspectClass);
   }

src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java

@@ -183,7 +183,7 @@ public AspectDefinition getDefinition(AspectParameters params) {
                     // For proto_lang_toolchain rules, where we just want to get at their runtime
                     // deps.
                     ImmutableSet.of(ProtoLangToolchainProvider.class)))
-            .addToolchainType(
+            .addToolchainTypes(
                 ToolchainTypeRequirement.create(
                     Label.parseAbsoluteUnchecked(toolsRepository + sdkToolchainLabel)))
             // Parse labels since we don't have RuleDefinitionEnvironment.getLabel like in a rule

src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java

@@ -125,7 +125,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParameters) {
             .propagateAlongAttribute("deps")
             .requiresConfigurationFragments(CppConfiguration.class, ProtoConfiguration.class)
             .requireStarlarkProviders(ProtoInfo.PROVIDER.id())
-            .addToolchainType(
+            .addToolchainTypes(
                 ToolchainTypeRequirement.builder(ccToolchainType)
                     // TODO(https://github.com/bazelbuild/bazel/issues/14727): Evaluate whether this
                     // can be optional.

src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java

@@ -153,7 +153,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParameters) {
             J2ObjcConfiguration.class,
             ObjcConfiguration.class,
             ProtoConfiguration.class)
-        .addToolchainType(
+        .addToolchainTypes(
             ToolchainTypeRequirement.builder(ccToolchainType)
                 // TODO(https://github.com/bazelbuild/bazel/issues/14727): Evaluate whether this can
                 // be optional.

src/test/java/com/google/devtools/build/lib/starlark/BUILD

@@ -41,6 +41,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/host_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/starlark_defined_config_transition",
+        "//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
         "//src/main/java/com/google/devtools/build/lib/analysis:file_provider",

src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java

@@ -25,6 +25,7 @@
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
 import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
 import com.google.devtools.build.lib.analysis.starlark.StarlarkAttrModule;
 import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleClassFunctions.StarlarkRuleFunction;
@@ -659,8 +660,11 @@ public void testAspectAddToolchain() throws Exception {
     evalAndExport(
         ev, "def _impl(ctx): pass", "a1 = aspect(_impl, toolchains=['//test:my_toolchain_type'])");
     StarlarkDefinedAspect a = (StarlarkDefinedAspect) ev.lookup("a1");
-    assertThat(a.getRequiredToolchains())
-        .containsExactly(Label.parseAbsoluteUnchecked("//test:my_toolchain_type"));
+    // TODO(https://github.com/bazelbuild/bazel/issues/14726): Add tests of optional toolchains.
+    assertThat(a.getToolchainTypes())
+        .containsExactly(
+            ToolchainTypeRequirement.create(
+                Label.parseAbsoluteUnchecked("//test:my_toolchain_type")));
   }
 
   @Test
@@ -2300,6 +2304,7 @@ public void testRuleAddToolchain() throws Exception {
         ev,
         "def impl(ctx): return None",
         "r1 = rule(impl, toolchains=['//test:my_toolchain_type'])");
+    // TODO(https://github.com/bazelbuild/bazel/issues/14726): Add tests of optional toolchains.
     RuleClass c = ((StarlarkRuleFunction) ev.lookup("r1")).getRuleClass();
     assertThat(c).hasToolchainType("//test:my_toolchain_type");
   }