API change discussed in bazelbuild#12006.

Allow users to set exec_group(copy_from_rule=true) on starlark exec groups which signifies that this exec group inherits toolchains and constraints from the rule to which it's attached.

Implementation detail: When processing parent rules, if parent rules have exec groups that are marked as inheriting from the rule, make them empty before adding them to the child rule. That way they can later be re-filled with the appropriate child rule's requirements instead of having the parent rule's requirements.

PiperOrigin-RevId: 340295132

site/docs/exec-groups.md

@@ -38,10 +38,7 @@ During rule definition, rule authors can
 a set of execution groups. On each execution group, the rule author can specify
 everything needed to select an execution platform for that execution group,
 namely any constraints via `exec_compatible_with` and toolchain types via
-`toolchain`. If an execution group is created as empty (no specified toolchains
-or constraints) it will automatically inherit these
-[parameters](https://docs.bazel.build/versions/master/skylark/lib/globals.html#rule)
-from the rule to which the group is attached.
+`toolchain`.
 
 ```python
 # foo.bzl
@@ -51,7 +48,6 @@ my_rule = rule(
         “link”: exec_group(
             exec_compatible_with = [ "@platforms//os:linux" ]
             toolchains = ["//foo:toolchain_type"],
-
         ),
         “test”: exec_group(
             toolchains = ["//foo_tools:toolchain_type"],
@@ -136,3 +132,33 @@ All actions with `exec_group = "link"` would see the exec properties
 dictionary as `{"mem": "16g"}`. As you see here, execution-group-level
 settings override target-level settings.
 
+### Creating exec groups to set exec properties
+
+Sometimes you want to use an exec group to give specific actions different exec
+properties but don't actually want different toolchains or constraints than the
+rule. For these situations, you can create exec groups using the `copy_from_rule`
+parameter:
+
+```python
+# foo.bzl
+
+# Creating an exec group with `copy_from_rule=True` is the same as explicitly
+# setting the exec group's toolchains and constraints to the same values as the
+# rule's respective parameters.
+my_rule = rule(
+    _impl,
+    exec_compatible_with = [ "@platforms//os:linux" ],
+    toolchains = ["//foo:toolchain_type"],
+    exec_groups = {
+        # The following two groups have the same toolchains and constraints:
+        “foo”: exec_group(copy_from_rule = True),
+        "bar": exec_group(
+            exec_compatible_with = [ "@platforms//os:linux" ],
+            toolchains = ["//foo:toolchain_type"],
+        ),
+    },
+)
+
+#
+```
+

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

@@ -927,8 +927,19 @@ public Label label(String labelString, Boolean relativeToCallerRepository, Starl
 
   @Override
   public ExecGroup execGroup(
-      Sequence<?> toolchains, Sequence<?> execCompatibleWith, StarlarkThread thread)
+      Sequence<?> toolchains,
+      Sequence<?> execCompatibleWith,
+      Boolean copyFromRule,
+      StarlarkThread thread)
       throws EvalException {
+    if (copyFromRule) {
+      if (!toolchains.isEmpty() || !execCompatibleWith.isEmpty()) {
+        throw Starlark.errorf(
+            "An exec group cannot set copy_from_rule=True and declare toolchains or constraints.");
+      }
+      return ExecGroup.COPY_FROM_RULE_EXEC_GROUP;
+    }
+
     ImmutableSet<Label> toolchainTypes = ImmutableSet.copyOf(parseToolchains(toolchains, thread));
     ImmutableSet<Label> constraints =
         ImmutableSet.copyOf(parseExecCompatibleWith(execCompatibleWith, thread));

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

@@ -24,14 +24,32 @@
 @AutoValue
 public abstract class ExecGroup implements ExecGroupApi {
 
-  static final ExecGroup EMPTY_EXEC_GROUP = create(ImmutableSet.of(), ImmutableSet.of());
+  // An exec group that inherits requirements from the rule.
+  public static final ExecGroup COPY_FROM_RULE_EXEC_GROUP =
+      createCopied(ImmutableSet.of(), ImmutableSet.of());
 
+  // Create an exec group that is marked as copying from the rule.
+  public static ExecGroup createCopied(
+      Set<Label> requiredToolchains, Set<Label> execCompatibleWith) {
+    return create(requiredToolchains, execCompatibleWith, /* copyFromRule= */ true);
+  }
+
+  // Create an exec group.
   public static ExecGroup create(Set<Label> requiredToolchains, Set<Label> execCompatibleWith) {
+    return create(requiredToolchains, execCompatibleWith, /* copyFromRule= */ false);
+  }
+
+  private static ExecGroup create(
+      Set<Label> requiredToolchains, Set<Label> execCompatibleWith, boolean copyFromRule) {
     return new AutoValue_ExecGroup(
-        ImmutableSet.copyOf(requiredToolchains), ImmutableSet.copyOf(execCompatibleWith));
+        ImmutableSet.copyOf(requiredToolchains),
+        ImmutableSet.copyOf(execCompatibleWith),
+        copyFromRule);
   }
 
   public abstract ImmutableSet<Label> requiredToolchains();
 
   public abstract ImmutableSet<Label> execCompatibleWith();
+
+  public abstract boolean copyFromRule();
 }

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

@@ -17,7 +17,7 @@
 import static com.google.devtools.build.lib.packages.Attribute.ANY_RULE;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
-import static com.google.devtools.build.lib.packages.ExecGroup.EMPTY_EXEC_GROUP;
+import static com.google.devtools.build.lib.packages.ExecGroup.COPY_FROM_RULE_EXEC_GROUP;
 import static com.google.devtools.build.lib.packages.Type.BOOLEAN;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -763,7 +763,24 @@ public Builder(String name, RuleClassType type, boolean starlark, RuleClass... p
         useToolchainTransition = parent.useToolchainTransition;
         addExecutionPlatformConstraints(parent.getExecutionPlatformConstraints());
         try {
-          addExecGroups(parent.getExecGroups());
+          ImmutableMap.Builder<String, ExecGroup> cleanedExecGroups = new ImmutableMap.Builder<>();
+          // For exec groups that copied toolchains and constraints from the rule, clear
+          // the toolchains and constraints. This prevents multiple inherited rule-copying exec
+          // groups with the same name from different parents from clashing. The toolchains and
+          // constraints will be overwritten with the rule's toolchains and constraints later
+          // anyway (see {@link #build}).
+          // For example, every rule that creates c++ linking actions inherits the rule-copying
+          // exec group "cpp_link". For rules that are the child of multiple of these rules,
+          // we need to clear out whatever toolchains and constraints have been copied from the rule
+          // in order to prevent clashing and fill with the the child's toolchain and constraints.
+          for (Map.Entry<String, ExecGroup> execGroup : parent.getExecGroups().entrySet()) {
+            if (execGroup.getValue().copyFromRule()) {
+              cleanedExecGroups.put(execGroup.getKey(), COPY_FROM_RULE_EXEC_GROUP);
+            } else {
+              cleanedExecGroups.put(execGroup);
+            }
+          }
+          addExecGroups(cleanedExecGroups.build());
         } catch (DuplicateExecGroupError e) {
           throw new IllegalArgumentException(
               String.format(
@@ -857,14 +874,15 @@ public RuleClass build(String name, String key) {
       // toolchains and constraints. Note that this isn't the same as a target's constraints which
       // also read from attributes and configuration.
       Map<String, ExecGroup> execGroupsWithInheritance = new HashMap<>();
-      ExecGroup inherited = null;
+      ExecGroup copiedFromRule = null;
       for (Map.Entry<String, ExecGroup> groupEntry : execGroups.entrySet()) {
         ExecGroup group = groupEntry.getValue();
-        if (group.equals(EMPTY_EXEC_GROUP)) {
-          if (inherited == null) {
-            inherited = ExecGroup.create(requiredToolchains, executionPlatformConstraints);
+        if (group.copyFromRule()) {
+          if (copiedFromRule == null) {
+            copiedFromRule =
+                ExecGroup.createCopied(requiredToolchains, executionPlatformConstraints);
           }
-          execGroupsWithInheritance.put(groupEntry.getKey(), inherited);
+          execGroupsWithInheritance.put(groupEntry.getKey(), copiedFromRule);
         } else {
           execGroupsWithInheritance.put(groupEntry.getKey(), group);
         }
@@ -1471,12 +1489,9 @@ public Builder addExecGroups(Map<String, ExecGroup> execGroups) {
       return this;
     }
 
-    /**
-     * Adds an exec group that is empty. During {@code build()} this will be replaced by an exec
-     * group that inherits its toolchains and constraints from the rule.
-     */
+    /** Adds an exec group that copies its toolchains and constraints from the rule. */
     public Builder addExecGroup(String name) {
-      return addExecGroups(ImmutableMap.of(name, EMPTY_EXEC_GROUP));
+      return addExecGroups(ImmutableMap.of(name, COPY_FROM_RULE_EXEC_GROUP));
     }
 
     /** An error to help report {@link ExecGroup}s with the same name */

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java

@@ -560,9 +560,21 @@ Label label(String labelString, Boolean relativeToCallerRepository, StarlarkThre
             positional = false,
             defaultValue = "[]",
             doc = "<i>Experimental</i> A list of constraints on the execution platform."),
+        @Param(
+            name = "copy_from_rule",
+            defaultValue = "False",
+            named = true,
+            positional = false,
+            doc =
+                "<i>Experimental</i> If set to true, this exec group inherits the toolchains and "
+                    + "constraints of the rule to which this group is attached. If set to any "
+                    + "other string this will throw an error.")
       },
       useStarlarkThread = true)
   ExecGroupApi execGroup(
-      Sequence<?> execCompatibleWith, Sequence<?> toolchains, StarlarkThread thread)
+      Sequence<?> execCompatibleWith,
+      Sequence<?> toolchains,
+      Boolean copyFromRule,
+      StarlarkThread thread)
       throws EvalException;
 }

src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java

@@ -226,7 +226,10 @@ public StarlarkAspectApi aspect(
 
   @Override
   public ExecGroupApi execGroup(
-      Sequence<?> execCompatibleWith, Sequence<?> toolchains, StarlarkThread thread) {
+      Sequence<?> execCompatibleWith,
+      Sequence<?> toolchains,
+      Boolean copyFromRule,
+      StarlarkThread thread) {
     return new FakeExecGroup();
   }
 

src/test/java/com/google/devtools/build/lib/analysis/StarlarkExecGroupTest.java

@@ -366,7 +366,7 @@ public void testSetUnknownExecGroup() throws Exception {
   }
 
   @Test
-  public void testEmptyExecGroupInherits() throws Exception {
+  public void testInheritsRuleRequirements() throws Exception {
     createToolchainsAndPlatforms();
     scratch.file(
         "test/defs.bzl",
@@ -376,7 +376,7 @@ public void testEmptyExecGroupInherits() throws Exception {
         "my_rule = rule(",
         "  implementation = _impl,",
         "  exec_groups = {",
-        "    'watermelon': exec_group(),",
+        "    'watermelon': exec_group(copy_from_rule = True),",
         "  },",
         "  exec_compatible_with = ['//platform:constraint_1'],",
         "  toolchains = ['//rule:toolchain_type_1'],",
@@ -387,7 +387,7 @@ public void testEmptyExecGroupInherits() throws Exception {
     assertThat(getRuleContext(ct).getRule().getRuleClassObject().getExecGroups())
         .containsExactly(
             "watermelon",
-            ExecGroup.create(
+            ExecGroup.createCopied(
                 ImmutableSet.of(Label.parseAbsoluteUnchecked("//rule:toolchain_type_1")),
                 ImmutableSet.of(Label.parseAbsoluteUnchecked("//platform:constraint_1"))));
   }

src/test/java/com/google/devtools/build/lib/packages/RuleClassBuilderTest.java

@@ -15,6 +15,7 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
+import static com.google.devtools.build.lib.packages.ExecGroup.COPY_FROM_RULE_EXEC_GROUP;
 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;
@@ -216,25 +217,33 @@ public void testExecGroupsAreInherited() throws Exception {
   }
 
   @Test
-  public void testDuplicateExecGroupsThatAreEqualIsOk() throws Exception {
-    ExecGroup execGroup =
-        ExecGroup.create(
-            ImmutableSet.of(Label.parseAbsoluteUnchecked("//some/toolchain")),
-            ImmutableSet.of(Label.parseAbsoluteUnchecked("//some/constraint")));
+  public void testDuplicateExecGroupsThatInheritFromRuleIsOk() throws Exception {
+    Label aToolchain = Label.parseAbsoluteUnchecked("//some/toolchain");
     RuleClass a =
         new RuleClass.Builder("ruleA", RuleClassType.NORMAL, false)
             .factory(DUMMY_CONFIGURED_TARGET_FACTORY)
-            .addExecGroups(ImmutableMap.of("blueberry", execGroup))
+            .addExecGroups(ImmutableMap.of("blueberry", COPY_FROM_RULE_EXEC_GROUP))
             .add(attr("tags", STRING_LIST))
+            .addRequiredToolchains(Label.parseAbsoluteUnchecked("//some/toolchain"))
             .build();
+    Label bToolchain = Label.parseAbsoluteUnchecked("//some/other/toolchain");
     RuleClass b =
         new RuleClass.Builder("ruleB", RuleClassType.NORMAL, false)
             .factory(DUMMY_CONFIGURED_TARGET_FACTORY)
-            .addExecGroups(ImmutableMap.of("blueberry", execGroup))
+            .addExecGroups(ImmutableMap.of("blueberry", COPY_FROM_RULE_EXEC_GROUP))
             .add(attr("tags", STRING_LIST))
+            .addRequiredToolchains(Label.parseAbsoluteUnchecked("//some/other/toolchain"))
             .build();
-    RuleClass c = new RuleClass.Builder("$ruleC", RuleClassType.ABSTRACT, false, a, b).build();
-    assertThat(c.getExecGroups()).containsExactly("blueberry", execGroup);
+    Label cToolchain = Label.parseAbsoluteUnchecked("//actual/toolchain/we/care/about");
+    RuleClass c =
+        new RuleClass.Builder("$ruleC", RuleClassType.ABSTRACT, false, a, b)
+            .addRequiredToolchains(cToolchain)
+            .build();
+    assertThat(c.getExecGroups())
+        .containsExactly(
+            "blueberry",
+            ExecGroup.createCopied(
+                ImmutableSet.of(aToolchain, bToolchain, cToolchain), ImmutableSet.of()));
   }
 
   @Test