Update ExecGroup to use a Builder interface.

Also fix tests that access ExecGroup to use the correct Subject.

Part of work on optional toolchains (#14726).

Closes #14758.

PiperOrigin-RevId: 427265632

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

@@ -1054,6 +1054,10 @@ public ExecGroup execGroup(
     ImmutableSet<Label> toolchainTypes = ImmutableSet.copyOf(parseToolchains(toolchains, thread));
     ImmutableSet<Label> constraints =
         ImmutableSet.copyOf(parseExecCompatibleWith(execCompatibleWith, thread));
-    return ExecGroup.create(toolchainTypes, constraints);
+    return ExecGroup.builder()
+        .requiredToolchains(toolchainTypes)
+        .execCompatibleWith(constraints)
+        .copyFrom(null)
+        .build();
   }
 }

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

@@ -18,7 +18,6 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.starlarkbuildapi.ExecGroupApi;
-import java.util.Set;
 import javax.annotation.Nullable;
 
 /** Resolves the appropriate toolchains for the given parameters. */
@@ -29,30 +28,29 @@ public abstract class ExecGroup implements ExecGroupApi {
   // users can't create a group with the same name.
   public static final String DEFAULT_EXEC_GROUP_NAME = "default-exec-group";
 
-  /** Create an exec group that inherits from the default exec group. */
-  public static ExecGroup copyFromDefault() {
-    return create(ImmutableSet.of(), ImmutableSet.of(), /* copyFrom= */ DEFAULT_EXEC_GROUP_NAME);
-  }
-
-  /** Create an exec group with the given toolchains and execution constraints. */
-  public static ExecGroup create(Set<Label> requiredToolchains, Set<Label> execCompatibleWith) {
-    return create(requiredToolchains, execCompatibleWith, /* copyFrom= */ null);
+  /** Returns a builder for a new ExecGroup. */
+  public static Builder builder() {
+    return new AutoValue_ExecGroup.Builder()
+        .requiredToolchains(ImmutableSet.of())
+        .execCompatibleWith(ImmutableSet.of());
   }
 
-  private static ExecGroup create(
-      Set<Label> requiredToolchains, Set<Label> execCompatibleWith, @Nullable String copyFrom) {
-    return new AutoValue_ExecGroup(
-        ImmutableSet.copyOf(requiredToolchains), ImmutableSet.copyOf(execCompatibleWith), copyFrom);
+  /** Create an exec group that inherits from the default exec group. */
+  public static ExecGroup copyFromDefault() {
+    return builder().copyFrom(DEFAULT_EXEC_GROUP_NAME).build();
   }
 
+  /** Returns the required toolchain types for this exec group. */
   public abstract ImmutableSet<Label> requiredToolchains();
 
+  /** Returns the execution constraints for this exec group. */
   public abstract ImmutableSet<Label> execCompatibleWith();
 
+  /** Returns the name of another exec group in the same rule to copy data from. */
   @Nullable
   public abstract String copyFrom();
 
-  /** Creates a new exec group that inherits from the given group. */
+  /** Creates a new exec group that inherits from the given group and this group. */
   public ExecGroup inheritFrom(ExecGroup other) {
     ImmutableSet<Label> requiredToolchains =
         new ImmutableSet.Builder<Label>()
@@ -64,6 +62,27 @@ public ExecGroup inheritFrom(ExecGroup other) {
             .addAll(this.execCompatibleWith())
             .addAll(other.execCompatibleWith())
             .build();
-    return create(requiredToolchains, execCompatibleWith);
+    return builder()
+        .requiredToolchains(requiredToolchains)
+        .execCompatibleWith(execCompatibleWith)
+        .copyFrom(null)
+        .build();
+  }
+
+  /** A builder interface to create ExecGroup instances. */
+  @AutoValue.Builder
+  public interface Builder {
+
+    /** Sets the required toolchain types. */
+    Builder requiredToolchains(ImmutableSet<Label> toolchainTypes);
+
+    /** Sets the execution constraints. */
+    Builder execCompatibleWith(ImmutableSet<Label> execCompatibleWith);
+
+    /** Sets the name of another exec group in the same rule to copy from. */
+    Builder copyFrom(@Nullable String copyfrom);
+
+    /** Returns the new ExecGroup instance. */
+    ExecGroup build();
   }
 }

src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java

@@ -559,7 +559,11 @@ public static ComputedToolchainContexts computeUnloadedToolchainContexts(
 
     // Create a merged version of the exec groups that handles exec group inheritance properly.
     ExecGroup defaultExecGroup =
-        ExecGroup.create(requiredDefaultToolchains, defaultExecConstraintLabels);
+        ExecGroup.builder()
+            .requiredToolchains(requiredDefaultToolchains)
+            .execCompatibleWith(defaultExecConstraintLabels)
+            .copyFrom(null)
+            .build();
     ExecGroupCollection.Builder execGroupCollectionBuilder =
         ExecGroupCollection.builder(defaultExecGroup, rule.getRuleClassObject().getExecGroups());
 

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

@@ -199,8 +199,17 @@ public void testExecGroupsAreInherited() throws Exception {
     Label mockToolchainType = Label.parseAbsoluteUnchecked("//mock_toolchain_type");
     Label mockConstraint = Label.parseAbsoluteUnchecked("//mock_constraint");
     ExecGroup parentGroup =
-        ExecGroup.create(ImmutableSet.of(mockToolchainType), ImmutableSet.of(mockConstraint));
-    ExecGroup childGroup = ExecGroup.create(ImmutableSet.of(), ImmutableSet.of());
+        ExecGroup.builder()
+            .requiredToolchains(ImmutableSet.of(mockToolchainType))
+            .execCompatibleWith(ImmutableSet.of(mockConstraint))
+            .copyFrom(null)
+            .build();
+    ExecGroup childGroup =
+        ExecGroup.builder()
+            .requiredToolchains(ImmutableSet.of())
+            .execCompatibleWith(ImmutableSet.of())
+            .copyFrom(null)
+            .build();
     RuleClass parent =
         new RuleClass.Builder("$parent", RuleClassType.ABSTRACT, false)
             .add(attr("tags", STRING_LIST))
@@ -249,17 +258,25 @@ public void testDuplicateExecGroupsThrowsError() throws Exception {
             .addExecGroups(
                 ImmutableMap.of(
                     "blueberry",
-                    ExecGroup.create(
-                        ImmutableSet.of(Label.parseAbsoluteUnchecked("//some/toolchain")),
-                        ImmutableSet.of())))
+                    ExecGroup.builder()
+                        .requiredToolchains(
+                            ImmutableSet.of(Label.parseAbsoluteUnchecked("//some/toolchain")))
+                        .execCompatibleWith(ImmutableSet.of())
+                        .copyFrom(null)
+                        .build()))
             .add(attr("tags", STRING_LIST))
             .build();
     RuleClass b =
         new RuleClass.Builder("ruleB", RuleClassType.NORMAL, false)
             .factory(DUMMY_CONFIGURED_TARGET_FACTORY)
             .addExecGroups(
                 ImmutableMap.of(
-                    "blueberry", ExecGroup.create(ImmutableSet.of(), ImmutableSet.of())))
+                    "blueberry",
+                    ExecGroup.builder()
+                        .requiredToolchains(ImmutableSet.of())
+                        .execCompatibleWith(ImmutableSet.of())
+                        .copyFrom(null)
+                        .build()))
             .add(attr("tags", STRING_LIST))
             .build();
     IllegalArgumentException e =

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

@@ -15,6 +15,7 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
+import static com.google.devtools.build.lib.analysis.testing.ExecGroupSubject.assertThat;
 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;
@@ -1081,15 +1082,18 @@ public void testExecGroups() throws Exception {
 
     ruleClassBuilder.addExecGroups(
         ImmutableMap.of(
-            "cherry", ExecGroup.create(ImmutableSet.of(toolchain), ImmutableSet.of(constraint))));
+            "cherry",
+            ExecGroup.builder()
+                .requiredToolchains(ImmutableSet.of(toolchain))
+                .execCompatibleWith(ImmutableSet.of(constraint))
+                .copyFrom(null)
+                .build()));
 
     RuleClass ruleClass = ruleClassBuilder.build();
 
     assertThat(ruleClass.getExecGroups()).hasSize(1);
-    assertThat(ruleClass.getExecGroups().get("cherry").requiredToolchains())
-        .containsExactly(toolchain);
-    assertThat(ruleClass.getExecGroups().get("cherry").execCompatibleWith())
-        .containsExactly(constraint);
+    assertThat(ruleClass.getExecGroups().get("cherry")).hasRequiredToolchain(toolchain);
+    assertThat(ruleClass.getExecGroups().get("cherry")).hasExecCompatibleWith(constraint);
   }
 
   @Test

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

@@ -76,6 +76,7 @@ java_test(
         "//src/main/java/net/starlark/java/syntax",
         "//src/main/protobuf:failure_details_java_proto",
         "//src/test/java/com/google/devtools/build/lib/actions/util",
+        "//src/test/java/com/google/devtools/build/lib/analysis/testing",
         "//src/test/java/com/google/devtools/build/lib/analysis/util",
         "//src/test/java/com/google/devtools/build/lib/exec/util",
         "//src/test/java/com/google/devtools/build/lib/starlark/util",

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

@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.starlark;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.analysis.testing.ExecGroupSubject.assertThat;
 import static org.junit.Assert.assertThrows;
 
 import com.google.common.base.Joiner;
@@ -2337,13 +2338,10 @@ public void testRuleAddExecGroup() throws Exception {
         ")");
     RuleClass plum = ((StarlarkRuleFunction) ev.lookup("plum")).getRuleClass();
     assertThat(plum.getRequiredToolchains()).isEmpty();
-    assertThat(plum.getExecGroups().get("group").requiredToolchains())
-        .containsExactly(Label.parseAbsoluteUnchecked("//test:my_toolchain_type"));
+    assertThat(plum.getExecGroups().get("group")).hasRequiredToolchain("//test:my_toolchain_type");
     assertThat(plum.getExecutionPlatformConstraints()).isEmpty();
-    assertThat(plum.getExecGroups().get("group").execCompatibleWith())
-        .containsExactly(
-            Label.parseAbsoluteUnchecked("//constraint:cv1"),
-            Label.parseAbsoluteUnchecked("//constraint:cv2"));
+    assertThat(plum.getExecGroups().get("group")).hasExecCompatibleWith("//constraint:cv1");
+    assertThat(plum.getExecGroups().get("group")).hasExecCompatibleWith("//constraint:cv2");
   }
 
   @Test
@@ -2390,12 +2388,9 @@ public void testCreateExecGroup() throws Exception {
         "  exec_compatible_with=['//constraint:cv1', '//constraint:cv2'],",
         ")");
     ExecGroup group = ((ExecGroup) ev.lookup("group"));
-    assertThat(group.requiredToolchains())
-        .containsExactly(Label.parseAbsoluteUnchecked("//test:my_toolchain_type"));
-    assertThat(group.execCompatibleWith())
-        .containsExactly(
-            Label.parseAbsoluteUnchecked("//constraint:cv1"),
-            Label.parseAbsoluteUnchecked("//constraint:cv2"));
+    assertThat(group).hasRequiredToolchain("//test:my_toolchain_type");
+    assertThat(group).hasExecCompatibleWith("//constraint:cv1");
+    assertThat(group).hasExecCompatibleWith("//constraint:cv2");
   }
 
   @Test