Deduplicate additional exec constraint logic

Centralize the logic around `exec_compatible_with` in a single place to simplify the introduction of `exec_group_compatible_with`. Work towards bazelbuild#23802 Closes bazelbuild#23803. PiperOrigin-RevId: 684089779 Change-Id: Idb4a651906ce730cba99b884c7487aaf3d4a0f80
aiuto · Oct 9, 2024 · 849014a · 849014a
1 parent c68c9ba
commit 849014a
Show file tree

Hide file tree

Showing 7 changed files with 202 additions and 218 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD
@@ -19,11 +19,13 @@ java_library(
             "BuildConfigurationKeyMapProducer.java",
             "BuildConfigurationKeyProducer.java",
             "PlatformProducer.java",
+            "UnloadedToolchainContextsInputs.java",
         ],
     ),
     deps = [
         ":build_configuration_key_map_producer",
         ":platform_producer",
+        ":unloaded_toolchain_contexts_inputs",
         "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:aspect_collection",
@@ -35,7 +37,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/invalid_configuration_exception",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/starlark_transition_cache",
-        "//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/composing_transition_factory",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition",
@@ -45,7 +46,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value",
         "//src/main/java/com/google/devtools/build/lib/analysis:constraints/incompatible_target_checker",
         "//src/main/java/com/google/devtools/build/lib/analysis:dependency_kind",
-        "//src/main/java/com/google/devtools/build/lib/analysis:exec_group_collection",
         "//src/main/java/com/google/devtools/build/lib/analysis:inconsistent_aspect_order_exception",
         "//src/main/java/com/google/devtools/build/lib/analysis:inconsistent_null_config_exception",
         "//src/main/java/com/google/devtools/build/lib/analysis:invalid_visibility_dependency_exception",
@@ -81,6 +81,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:no_matching_platform_exception",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:platform_lookup_util",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:toolchain_context_key",
+        "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:toolchain_context_util",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:toolchain_exception",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:unloaded_toolchain_context",
         "//src/main/java/com/google/devtools/build/lib/util",
@@ -134,3 +135,16 @@ java_library(
         "//third_party:jsr305",
     ],
 )
+
+java_library(
+    name = "unloaded_toolchain_contexts_inputs",
+    srcs = ["UnloadedToolchainContextsInputs.java"],
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib/analysis:exec_group_collection",
+        "//src/main/java/com/google/devtools/build/lib/packages:exec_group",
+        "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:toolchain_context_key",
+        "//third_party:auto_value",
+        "//third_party:guava",
+        "//third_party:jsr305",
+    ],
+)
diff --git a/...java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java b/...java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java
@@ -19,10 +19,8 @@
 
 import com.google.auto.value.AutoOneOf;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionLookupKey;
 import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
-import com.google.devtools.build.lib.analysis.ExecGroupCollection;
 import com.google.devtools.build.lib.analysis.InconsistentNullConfigException;
 import com.google.devtools.build.lib.analysis.PlatformConfiguration;
 import com.google.devtools.build.lib.analysis.PlatformOptions;
@@ -36,7 +34,6 @@
 import com.google.devtools.build.lib.analysis.config.CoreOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.analysis.config.StarlarkTransitionCache;
-import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
 import com.google.devtools.build.lib.analysis.config.transitions.ComposingTransitionFactory;
 import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
 import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
@@ -47,19 +44,13 @@
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId.ConfigurationId;
 import com.google.devtools.build.lib.causes.AnalysisFailedCause;
 import com.google.devtools.build.lib.causes.Cause;
-import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
-import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
-import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
-import com.google.devtools.build.lib.packages.RawAttributeMapper;
-import com.google.devtools.build.lib.packages.Rule;
-import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleTransitionData;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TargetUtils;
@@ -70,7 +61,7 @@
 import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey;
 import com.google.devtools.build.lib.skyframe.config.PlatformMappingException;
 import com.google.devtools.build.lib.skyframe.toolchains.PlatformLookupUtil.InvalidPlatformException;
-import com.google.devtools.build.lib.skyframe.toolchains.ToolchainContextKey;
+import com.google.devtools.build.lib.skyframe.toolchains.ToolchainContextUtil;
 import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.build.skyframe.SkyValue;
 import com.google.devtools.build.skyframe.state.StateMachine;
@@ -92,6 +83,7 @@ public final class TargetAndConfigurationProducer
         StateMachine.ValueOrExceptionSink<InvalidConfigurationException>,
         Consumer<SkyValue>,
         TargetProducer.ResultSink {
+
   /** Accepts results of this producer. */
   public interface ResultSink {
     void acceptTargetAndConfiguration(TargetAndConfiguration value, ConfiguredTargetKey fullKey);
@@ -277,14 +269,26 @@ private class RuleTransitionApplier
 
     @Override
     public StateMachine step(Tasks tasks) throws InterruptedException {
-
-      UnloadedToolchainContextsInputs unloadedToolchainContextsInputs =
-          getUnloadedToolchainContextsInputs(
-              target, preRuleTransitionKey.getExecutionPlatformLabel());
+      UnloadedToolchainContextsInputs unloadedToolchainContextsInputs;
+      PlatformConfiguration platformConfiguration = null;
+      var platformOptions =
+          preRuleTransitionKey.getConfigurationKey().getOptions().get(PlatformOptions.class);
+      if (platformOptions == null) {
+        unloadedToolchainContextsInputs = UnloadedToolchainContextsInputs.empty();
+      } else {
+        platformConfiguration = new PlatformConfiguration(platformOptions);
+        unloadedToolchainContextsInputs =
+            ToolchainContextUtil.getUnloadedToolchainContextsInputs(
+                target,
+                preRuleTransitionKey.getConfigurationKey().getOptions().get(CoreOptions.class),
+                platformConfiguration,
+                preRuleTransitionKey.getExecutionPlatformLabel(),
+                computeToolchainConfigurationKey(
+                    preRuleTransitionKey.getConfigurationKey().getOptions(),
+                    toolchainTaggedTrimmingTransition));
+      }
 
       if (unloadedToolchainContextsInputs.targetToolchainContextKey() != null) {
-        PlatformConfiguration platformConfiguration =
-            new PlatformConfiguration(preRuleTransitionKey.getConfigurationKey().getOptions());
         tasks.enqueue(
             new PlatformProducer(
                 platformConfiguration.getTargetPlatform(),
@@ -298,84 +302,6 @@ public StateMachine step(Tasks tasks) throws InterruptedException {
       return DONE;
     }
 
-    // TODO: @aranguyen b/297077082
-    public UnloadedToolchainContextsInputs getUnloadedToolchainContextsInputs(
-        Target target, @Nullable Label parentExecutionPlatformLabel) throws InterruptedException {
-      Rule rule = target.getAssociatedRule();
-      if (rule == null) {
-        return UnloadedToolchainContextsInputs.empty();
-      }
-
-      var platformOptions =
-          preRuleTransitionKey.getConfigurationKey().getOptions().get(PlatformOptions.class);
-      if (platformOptions == null) {
-        return UnloadedToolchainContextsInputs.empty();
-      }
-      PlatformConfiguration platformConfiguration = new PlatformConfiguration(platformOptions);
-      var defaultExecConstraintLabels =
-          getExecutionPlatformConstraints(rule, platformConfiguration);
-      var ruleClass = rule.getRuleClassObject();
-      boolean useAutoExecGroups =
-          rule.getRuleClassObject()
-              .getAutoExecGroupsMode()
-              .isEnabled(
-                  RawAttributeMapper.of(rule),
-                  preRuleTransitionKey
-                      .getConfigurationKey()
-                      .getOptions()
-                      .get(CoreOptions.class)
-                      .useAutoExecGroups);
-
-      var processedExecGroups =
-          ExecGroupCollection.process(
-              ruleClass.getExecGroups(),
-              defaultExecConstraintLabels,
-              ruleClass.getToolchainTypes(),
-              useAutoExecGroups);
-
-      if (!rule.useToolchainResolution()) {
-        return UnloadedToolchainContextsInputs.create(processedExecGroups, null);
-      }
-
-      return UnloadedToolchainContextsInputs.create(
-          processedExecGroups,
-          createDefaultToolchainContextKey(
-              computeToolchainConfigurationKey(
-                  preRuleTransitionKey.getConfigurationKey().getOptions(),
-                  toolchainTaggedTrimmingTransition),
-              defaultExecConstraintLabels,
-              /* debugTarget= */ platformConfiguration.debugToolchainResolution(rule.getLabel()),
-              /* useAutoExecGroups= */ useAutoExecGroups,
-              ruleClass.getToolchainTypes(),
-              parentExecutionPlatformLabel));
-    }
-
-    public ToolchainContextKey createDefaultToolchainContextKey(
-        BuildConfigurationKey configurationKey,
-        ImmutableSet<Label> defaultExecConstraintLabels,
-        boolean debugTarget,
-        boolean useAutoExecGroups,
-        ImmutableSet<ToolchainTypeRequirement> toolchainTypes,
-        @Nullable Label parentExecutionPlatformLabel) {
-      ToolchainContextKey.Builder toolchainContextKeyBuilder =
-          ToolchainContextKey.key()
-              .configurationKey(configurationKey)
-              .execConstraintLabels(defaultExecConstraintLabels)
-              .debugTarget(debugTarget);
-
-      // Add toolchain types only if automatic exec groups are not created for this target.
-      if (!useAutoExecGroups) {
-        toolchainContextKeyBuilder.toolchainTypes(toolchainTypes);
-      }
-
-      if (parentExecutionPlatformLabel != null) {
-        // Find out what execution platform the parent used, and force that.
-        // This should only be set for direct toolchain dependencies.
-        toolchainContextKeyBuilder.forceExecutionPlatform(parentExecutionPlatformLabel);
-      }
-      return toolchainContextKeyBuilder.build();
-    }
-
     private BuildConfigurationKey computeToolchainConfigurationKey(
         BuildOptions buildOptions, PatchTransition toolchainTaggedTrimmingTransition)
         throws InterruptedException {
@@ -409,27 +335,6 @@ private BuildConfigurationKey computeToolchainConfigurationKey(
       return BuildConfigurationKey.create(toolchainOptions);
     }
 
-    private ImmutableSet<Label> getExecutionPlatformConstraints(
-        Rule rule, @Nullable PlatformConfiguration platformConfiguration) {
-      if (platformConfiguration == null) {
-        return ImmutableSet.of(); // See NoConfigTransition.
-      }
-      NonconfigurableAttributeMapper mapper = NonconfigurableAttributeMapper.of(rule);
-      ImmutableSet.Builder<Label> execConstraintLabels = new ImmutableSet.Builder<>();
-
-      execConstraintLabels.addAll(rule.getRuleClassObject().getExecutionPlatformConstraints());
-      if (rule.getRuleClassObject()
-          .hasAttr(RuleClass.EXEC_COMPATIBLE_WITH_ATTR, BuildType.LABEL_LIST)) {
-        execConstraintLabels.addAll(
-            mapper.get(RuleClass.EXEC_COMPATIBLE_WITH_ATTR, BuildType.LABEL_LIST));
-      }
-
-      execConstraintLabels.addAll(
-          platformConfiguration.getAdditionalExecutionConstraintsFor(rule.getLabel()));
-
-      return execConstraintLabels.build();
-    }
-
     @CanIgnoreReturnValue
     public StateMachine computeConfigConditions(Tasks tasks) {
       // TODO @aranguyen b/297077082

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
@@ -14,7 +14,6 @@
 package com.google.devtools.build.lib.skyframe;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
-import static com.google.devtools.build.lib.skyframe.DependencyResolver.createDefaultToolchainContextKey;
 import static com.google.devtools.build.lib.skyframe.DependencyResolver.getPrioritizedDetailedExitCode;
 import static com.google.devtools.build.lib.skyframe.SkyValueRetrieverUtils.maybeFetchSkyValueRemotely;
 import static com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.INITIAL_STATE;
@@ -92,6 +91,7 @@
 import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.SerializationState;
 import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.SerializationStateProvider;
 import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingDependenciesProvider;
+import com.google.devtools.build.lib.skyframe.toolchains.ToolchainContextUtil;
 import com.google.devtools.build.lib.skyframe.toolchains.ToolchainException;
 import com.google.devtools.build.lib.skyframe.toolchains.UnloadedToolchainContext;
 import com.google.devtools.build.lib.util.OrderedSetMultimap;
@@ -811,7 +811,6 @@ static StarlarkDefinedAspect loadAspectFromBzl(
     return (StarlarkDefinedAspect) starlarkValue;
   }
 
-  @Nullable
   private static UnloadedToolchainContextsInputs getUnloadedToolchainContextsInputs(
       AspectDefinition aspectDefinition,
       @Nullable BuildConfigurationKey configurationKey,
@@ -832,7 +831,7 @@ private static UnloadedToolchainContextsInputs getUnloadedToolchainContextsInput
     // Note: `configuration.getOptions().hasNoConfig()` is handled early in #compute.
     return UnloadedToolchainContextsInputs.create(
         processedExecGroups,
-        createDefaultToolchainContextKey(
+        ToolchainContextUtil.createDefaultToolchainContextKey(
             configurationKey,
             aspectDefinition.execCompatibleWith(),
             /* debugTarget= */ false,

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -255,7 +255,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:config/options_diff",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/starlark_exec_transition_loader",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/starlark_transition_cache",
-        "//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/transition_collector",
@@ -291,6 +290,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis/platform",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform:platform_function",
         "//src/main/java/com/google/devtools/build/lib/analysis/producers",
+        "//src/main/java/com/google/devtools/build/lib/analysis/producers:unloaded_toolchain_contexts_inputs",
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common",
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_extension",
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:repo_rule_creator",
@@ -354,6 +354,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:registered_toolchains_function",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:single_toolchain_resolution_function",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:toolchain_context_key",
+        "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:toolchain_context_util",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:toolchain_exception",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:toolchain_resolution_function",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:unloaded_toolchain_context",