Add exec_group_compatible_with attribute

This change implements the [Per-target execution platform constraints for exec groups](https://docs.google.com/document/d/1u4zP5MLU3HOt-qlyiNnlxcu4SqOB5tnm32v02vLnd5U/edit) proposal by adding the `exec_group_compatible_with` attribute.

Part of #24964.

RELNOTES: The new exec_group_compatible_with attribute on all rules accepts a dictionary mapping exec group names to lists of additional constraints to request from the exec group's execution platform.
PiperOrigin-RevId: 721529975
Change-Id: Ibcd6e9772684272d48a39907eaa9f7503fbaaa6c

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

@@ -774,6 +774,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/packages:exec_group",
         "//src/main/java/com/google/devtools/build/lib/skyframe:sane_analysis_exception",
         "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+        "//src/main/java/net/starlark/java/spelling",
         "//src/main/protobuf:failure_details_java_proto",
         "//third_party:auto_value",
         "//third_party:guava",

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

@@ -14,13 +14,14 @@
 
 package com.google.devtools.build.lib.analysis;
 
-import static com.google.devtools.build.lib.analysis.test.ExecutionInfo.DEFAULT_TEST_RUNNER_EXEC_GROUP;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.BuildType.DISTRIBUTIONS;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
 import static com.google.devtools.build.lib.packages.BuildType.LICENSE;
 import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL_LIST;
+import static com.google.devtools.build.lib.packages.RuleClass.DEFAULT_TEST_RUNNER_EXEC_GROUP;
+import static com.google.devtools.build.lib.packages.RuleClass.DEFAULT_TEST_RUNNER_EXEC_GROUP_NAME;
 import static com.google.devtools.build.lib.packages.Type.BOOLEAN;
 import static com.google.devtools.build.lib.packages.Type.INTEGER;
 import static com.google.devtools.build.lib.packages.Type.STRING;
@@ -214,7 +215,8 @@ public static final class TestBaseRule implements RuleDefinition {
     @Override
     public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
       builder
-          .addExecGroup(DEFAULT_TEST_RUNNER_EXEC_GROUP)
+          .addExecGroups(
+              ImmutableMap.of(DEFAULT_TEST_RUNNER_EXEC_GROUP_NAME, DEFAULT_TEST_RUNNER_EXEC_GROUP))
           .requiresConfigurationFragments(TestConfiguration.class)
           // TestConfiguration only needed to create TestAction and TestProvider
           // Only necessary at top-level and can be skipped if trimmed.
@@ -536,6 +538,14 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
                       "exec_compatible_with exists for constraint checking, not to create an"
                           + " actual dependency")
                   .value(ImmutableList.of()))
+          .add(
+              attr(RuleClass.EXEC_GROUP_COMPATIBLE_WITH_ATTR, BuildType.LABEL_LIST_DICT)
+                  .allowedFileTypes()
+                  .nonconfigurable("Used in toolchain resolution")
+                  .tool(
+                      "exec_group_compatible_with exists for constraint checking, not to create an"
+                          + " actual dependency")
+                  .value(ImmutableMap.of()))
           .build();
     }
 

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

@@ -18,9 +18,12 @@
 
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableTable;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Table;
 import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
@@ -36,6 +39,7 @@
 import java.util.LinkedHashMap;
 import java.util.Map;
 import javax.annotation.Nullable;
+import net.starlark.java.spelling.SpellChecker;
 
 /**
  * A container class for groups of {@link ExecGroup} instances. This correctly handles exec group
@@ -52,6 +56,7 @@ public abstract class ExecGroupCollection {
   public static ImmutableMap<String, ExecGroup> process(
       ImmutableMap<String, ExecGroup> execGroups,
       ImmutableSet<Label> defaultExecWith,
+      ImmutableMultimap<String, Label> execGroupExecWith,
       ImmutableSet<ToolchainTypeRequirement> defaultToolchainTypes,
       boolean useAutoExecGroups) {
     var processedGroups =
@@ -70,18 +75,36 @@ public static ImmutableMap<String, ExecGroup> process(
                   .toolchainTypes(defaultToolchainTypes)
                   .build();
       }
+      ImmutableCollection<Label> extraExecWith = execGroupExecWith.get(name);
+      if (!extraExecWith.isEmpty()) {
+        execGroup =
+            execGroup.toBuilder()
+                .execCompatibleWith(
+                    ImmutableSet.<Label>builder()
+                        .addAll(execGroup.execCompatibleWith())
+                        .addAll(extraExecWith)
+                        .build())
+                .build();
+      }
 
       processedGroups.put(name, execGroup);
     }
 
     if (useAutoExecGroups) {
       // Creates one exec group for each toolchain (automatic exec groups).
       for (ToolchainTypeRequirement toolchainType : defaultToolchainTypes) {
+        ImmutableSet<Label> execCompatibleWith = defaultExecWith;
+        ImmutableCollection<Label> extraExecWith =
+            execGroupExecWith.get(toolchainType.toolchainType().getUnambiguousCanonicalForm());
+        if (!extraExecWith.isEmpty()) {
+          execCompatibleWith =
+              ImmutableSet.<Label>builder().addAll(defaultExecWith).addAll(extraExecWith).build();
+        }
         processedGroups.put(
             toolchainType.toolchainType().toString(),
             ExecGroup.builder()
                 .addToolchainType(toolchainType)
-                .execCompatibleWith(defaultExecWith)
+                .execCompatibleWith(execCompatibleWith)
                 .build());
       }
     }
@@ -134,7 +157,10 @@ private static ImmutableTable<String, String, String> computeCombinedExecPropert
               .filter(name -> !execGroupNames.contains(name))
               .collect(toImmutableSet());
       if (!unknownTargetExecGroupNames.isEmpty()) {
-        throw new InvalidExecGroupException(unknownTargetExecGroupNames);
+        throw new InvalidExecGroupException(
+            "exec_properties",
+            unknownTargetExecGroupNames,
+            Iterables.concat(execGroupNames, ImmutableSet.of(DEFAULT_EXEC_GROUP_NAME)));
       }
     }
 
@@ -257,11 +283,18 @@ private static ImmutableTable<String, String, String> parseExecProperties(
   /** An error for when the user tries to access a non-existent exec group. */
   public static final class InvalidExecGroupException extends AbstractSaneAnalysisException {
 
-    public InvalidExecGroupException(Collection<String> invalidNames) {
+    public InvalidExecGroupException(
+        String what, Collection<String> invalidNames, Iterable<String> validNames) {
       super(
           String.format(
-              "Tried to set properties for non-existent exec groups: %s.",
-              String.join(",", invalidNames)));
+              "Tried to set %s for non-existent exec groups: %s%s",
+              what,
+              String.join(",", invalidNames),
+              invalidNames.stream()
+                  .map(invalidName -> SpellChecker.didYouMean(invalidName, validNames))
+                  .filter(s -> !s.isEmpty())
+                  .findFirst()
+                  .orElse("")));
     }
 
     @Override

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

@@ -17,8 +17,8 @@
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.devtools.build.lib.analysis.constraints.ConstraintConstants.OS_TO_CONSTRAINTS;
-import static com.google.devtools.build.lib.analysis.test.ExecutionInfo.DEFAULT_TEST_RUNNER_EXEC_GROUP;
 import static com.google.devtools.build.lib.packages.ExecGroup.DEFAULT_EXEC_GROUP_NAME;
+import static com.google.devtools.build.lib.packages.RuleClass.DEFAULT_TEST_RUNNER_EXEC_GROUP_NAME;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
@@ -435,7 +435,7 @@ public ActionOwner getTestActionOwner() {
       testExecProperties = testExecutionPlatform.execProperties();
     } else {
       testExecutionPlatform = null;
-      testExecProperties = getExecGroups().getExecProperties(DEFAULT_TEST_RUNNER_EXEC_GROUP);
+      testExecProperties = getExecGroups().getExecProperties(DEFAULT_TEST_RUNNER_EXEC_GROUP_NAME);
     }
 
     ActionOwner actionOwner =

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

@@ -46,6 +46,7 @@ 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",

src/main/java/com/google/devtools/build/lib/analysis/producers/RuleTransitionApplier.java

@@ -17,6 +17,7 @@
 import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.analysis.ExecGroupCollection;
 import com.google.devtools.build.lib.analysis.PlatformConfiguration;
 import com.google.devtools.build.lib.analysis.PlatformOptions;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -148,15 +149,20 @@ public StateMachine step(Tasks tasks) throws InterruptedException {
       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(),
-                  targetAndConfigurationData.getToolchainTaggedTrimmingTransition()));
+      try {
+        unloadedToolchainContextsInputs =
+            ToolchainContextUtil.getUnloadedToolchainContextsInputs(
+                target,
+                preRuleTransitionKey.getConfigurationKey().getOptions().get(CoreOptions.class),
+                platformConfiguration,
+                preRuleTransitionKey.getExecutionPlatformLabel(),
+                computeToolchainConfigurationKey(
+                    preRuleTransitionKey.getConfigurationKey().getOptions(),
+                    targetAndConfigurationData.getToolchainTaggedTrimmingTransition()));
+      } catch (ExecGroupCollection.InvalidExecGroupException e) {
+        emitErrorMessage(e.getMessage());
+        return runAfter;
+      }
     }
 
     if (unloadedToolchainContextsInputs.targetToolchainContextKey() != null) {

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

@@ -29,6 +29,7 @@
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.StructImpl;
 import com.google.devtools.build.lib.packages.StructProvider;
 import com.google.devtools.build.lib.packages.Type;
@@ -230,6 +231,13 @@ public static Object convertAttributeValue(
         return null;
       }
 
+      // Don't expose exec_group_compatible_with to Starlark. There is no reason for it to be used
+      // by the rule implementation function and its type (LABEL_LIST_DICT) is not available to
+      // Starlark.
+      if (a.getName().equals(RuleClass.EXEC_GROUP_COMPATIBLE_WITH_ATTR)) {
+        return null;
+      }
+
       if (type == BuildType.DORMANT_LABEL) {
         return val == null
             ? Starlark.NONE

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

@@ -21,12 +21,13 @@
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.RUN_UNDER_TARGET_CONFIG;
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.TIMEOUT_DEFAULT;
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.getTestRuntimeLabelList;
-import static com.google.devtools.build.lib.analysis.test.ExecutionInfo.DEFAULT_TEST_RUNNER_EXEC_GROUP;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
 import static com.google.devtools.build.lib.packages.BuildType.LICENSE;
 import static com.google.devtools.build.lib.packages.BuiltinRestriction.allowlistEntry;
+import static com.google.devtools.build.lib.packages.RuleClass.DEFAULT_TEST_RUNNER_EXEC_GROUP;
+import static com.google.devtools.build.lib.packages.RuleClass.DEFAULT_TEST_RUNNER_EXEC_GROUP_NAME;
 import static com.google.devtools.build.lib.packages.Type.BOOLEAN;
 import static com.google.devtools.build.lib.packages.Type.INTEGER;
 import static com.google.devtools.build.lib.packages.Type.STRING;
@@ -170,6 +171,14 @@ public class StarlarkRuleClassFunctions implements StarlarkRuleFunctionsApi {
                       "exec_compatible_with exists for constraint checking, not to create an"
                           + " actual dependency")
                   .value(ImmutableList.of()))
+          .add(
+              attr(RuleClass.EXEC_GROUP_COMPATIBLE_WITH_ATTR, BuildType.LABEL_LIST_DICT)
+                  .allowedFileTypes()
+                  .nonconfigurable("Used in toolchain resolution")
+                  .tool(
+                      "exec_group_compatible_with exists for constraint checking, not to create an"
+                          + " actual dependency")
+                  .value(ImmutableMap.of()))
           .add(
               attr(RuleClass.TARGET_COMPATIBLE_WITH_ATTR, LABEL_LIST)
                   .mandatoryProviders(ConstraintValueInfo.PROVIDER.id())
@@ -187,8 +196,9 @@ public class StarlarkRuleClassFunctions implements StarlarkRuleFunctionsApi {
           .removeAttribute(":action_listener")
           .removeAttribute("aspect_hints")
           .removeAttribute("toolchains")
-          .removeAttribute("exec_compatible_with")
-          .removeAttribute("target_compatible_with")
+          .removeAttribute(RuleClass.EXEC_COMPATIBLE_WITH_ATTR)
+          .removeAttribute(RuleClass.EXEC_GROUP_COMPATIBLE_WITH_ATTR)
+          .removeAttribute(RuleClass.TARGET_COMPATIBLE_WITH_ATTR)
           .removeAttribute("compatible_with")
           .removeAttribute("restricted_to")
           .removeAttribute("$config_dependencies")
@@ -928,8 +938,9 @@ public static StarlarkRuleFunction createRule(
       }
       builder.addExecGroups(execGroupDict);
     }
-    if (test && !builder.hasExecGroup(DEFAULT_TEST_RUNNER_EXEC_GROUP)) {
-      builder.addExecGroup(DEFAULT_TEST_RUNNER_EXEC_GROUP);
+    if (test && !builder.hasExecGroup(DEFAULT_TEST_RUNNER_EXEC_GROUP_NAME)) {
+      builder.addExecGroups(
+          ImmutableMap.of(DEFAULT_TEST_RUNNER_EXEC_GROUP_NAME, DEFAULT_TEST_RUNNER_EXEC_GROUP));
     }
 
     if (!buildSetting.equals(Starlark.NONE) && !cfg.equals(Starlark.NONE)) {

src/main/java/com/google/devtools/build/lib/analysis/test/ExecutionInfo.java

@@ -18,6 +18,7 @@
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.BuiltinProvider;
 import com.google.devtools.build.lib.packages.NativeInfo;
+import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.starlarkbuildapi.test.ExecutionInfoApi;
 import java.util.Map;
 import net.starlark.java.eval.Dict;
@@ -30,17 +31,14 @@
 @Immutable
 public final class ExecutionInfo extends NativeInfo implements ExecutionInfoApi {
 
-  // TODO(bazel-team): Find a better location for this constant.
-  public static final String DEFAULT_TEST_RUNNER_EXEC_GROUP = "test";
-
   /** Starlark constructor and identifier for ExecutionInfo. */
   public static final ExecutionInfoProvider PROVIDER = new ExecutionInfoProvider();
 
   private final ImmutableMap<String, String> executionInfo;
   private final String execGroup;
 
   public ExecutionInfo(Map<String, String> requirements) {
-    this(requirements, DEFAULT_TEST_RUNNER_EXEC_GROUP);
+    this(requirements, RuleClass.DEFAULT_TEST_RUNNER_EXEC_GROUP_NAME);
   }
 
   public ExecutionInfo(Map<String, String> requirements, String execGroup) {

src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java

@@ -14,8 +14,8 @@
 
 package com.google.devtools.build.lib.analysis.test;
 
-import static com.google.devtools.build.lib.analysis.test.ExecutionInfo.DEFAULT_TEST_RUNNER_EXEC_GROUP;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL;
+import static com.google.devtools.build.lib.packages.RuleClass.DEFAULT_TEST_RUNNER_EXEC_GROUP_NAME;
 
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
@@ -159,7 +159,7 @@ private ActionOwner getTestActionOwner(boolean useTargetPlatformForTests) {
     var execGroup =
         this.executionRequirements != null
             ? this.executionRequirements.getExecGroup()
-            : DEFAULT_TEST_RUNNER_EXEC_GROUP;
+            : DEFAULT_TEST_RUNNER_EXEC_GROUP_NAME;
     var owner = ruleContext.getActionOwner(execGroup);
     if (owner != null) {
       return owner;

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

@@ -79,6 +79,10 @@ public ToolchainTypeRequirement toolchainType(Label label) {
     return toolchainTypesMap().get(label);
   }
 
+  public Builder toBuilder() {
+    return new AutoBuilder_ExecGroup_Builder(this);
+  }
+
   /** A builder interface to create ExecGroup instances. */
   @AutoBuilder
   public interface Builder {

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

@@ -161,6 +161,9 @@ public class RuleClass implements RuleClassData {
 
   public static final String APPLICABLE_METADATA_ATTR_ALT = "applicable_licenses";
 
+  public static final String DEFAULT_TEST_RUNNER_EXEC_GROUP_NAME = "test";
+  public static final ExecGroup DEFAULT_TEST_RUNNER_EXEC_GROUP = ExecGroup.COPY_FROM_DEFAULT;
+
   /** Interface for determining whether a rule needs toolchain resolution or not. */
   @FunctionalInterface
   public interface ToolchainResolutionMode extends Serializable {
@@ -258,10 +261,16 @@ public RuleErrorException(String message, Throwable cause) {
 
   /**
    * For Bazel's constraint system: the attribute that declares the list of constraints that the
-   * execution platform must satisfy to be considered compatible.
+   * default exec group's execution platform must satisfy to be considered compatible.
    */
   public static final String EXEC_COMPATIBLE_WITH_ATTR = "exec_compatible_with";
 
+  /**
+   * For Bazel's constraint system: the attribute that declares the list of constraints that the
+   * given exec groups' execution platforms must satisfy to be considered compatible.
+   */
+  public static final String EXEC_GROUP_COMPATIBLE_WITH_ATTR = "exec_group_compatible_with";
+
   /**
    * The attribute that declares execution properties that should be added to actions created by
    * this target.
@@ -1622,11 +1631,6 @@ public Builder addExecGroups(Map<String, ExecGroup> execGroups) {
       return this;
     }
 
-    /** Adds an exec group that copies its toolchains and constraints from the rule. */
-    public Builder addExecGroup(String name) {
-      return addExecGroups(ImmutableMap.of(name, ExecGroup.COPY_FROM_DEFAULT));
-    }
-
     /** An error to help report {@link ExecGroup}s with the same name */
     static class DuplicateExecGroupError extends RuntimeException {
       private final String duplicateGroup;