Renamed ExecGroupCollection to clarify that it is only for Starlark usage.

katre · katre · commit d2e21cec31f0 · 2021-07-13T16:40:05.000-04:00
Non-starlark usage can go via the ToolchainCollection directly.

PiperOrigin-RevId: 367461392
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -188,7 +188,6 @@ java_library(
         "DependencyResolver.java",
         "EmptyConfiguredTarget.java",
         "EventHandlingErrorReporter.java",
-        "ExecGroupCollection.java",
         "Expander.java",
         "ExtraActionUtils.java",
         "ExtraActionsVisitor.java",
@@ -348,6 +347,7 @@ java_library(
         ":starlark/function_transition_util",
         ":starlark/starlark_api_provider",
         ":starlark/starlark_command_line",
+        ":starlark/starlark_exec_group_collection",
         ":starlark/starlark_late_bound_default",
         ":template_variable_info",
         ":test/analysis_failure",
@@ -2146,6 +2146,20 @@ java_library(
     ],
 )
 
+java_library(
+    name = "starlark/starlark_exec_group_collection",
+    srcs = ["starlark/StarlarkExecGroupCollection.java"],
+    deps = [
+        ":resolved_toolchain_context",
+        ":toolchain_collection",
+        "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform",
+        "//src/main/java/net/starlark/java/eval",
+        "//src/main/java/net/starlark/java/syntax",
+        "//third_party:auto_value",
+        "//third_party:guava",
+    ],
+)
+
 java_library(
     name = "starlark/starlark_error_reporter",
     srcs = ["starlark/StarlarkErrorReporter.java"],
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java b/src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java
@@ -110,7 +110,7 @@ public static ResolvedToolchainContext load(
   abstract ImmutableMap<RepositoryName, RepositoryName> repoMapping();
 
   /** Returns a description of the target being used, for error messaging. */
-  abstract String targetDescription();
+  public abstract String targetDescription();
 
   /** Sets the map from requested {@link Label} to toolchain type provider. */
   abstract ImmutableMap<Label, ToolchainTypeInfo> requestedToolchainTypeLabels();
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java
@@ -30,7 +30,6 @@
 import com.google.devtools.build.lib.actions.extra.SpawnInfo;
 import com.google.devtools.build.lib.analysis.BashCommandConstructor;
 import com.google.devtools.build.lib.analysis.CommandHelper;
-import com.google.devtools.build.lib.analysis.ExecGroupCollection;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.PseudoAction;
 import com.google.devtools.build.lib.analysis.RuleContext;
@@ -618,7 +617,7 @@ private void registerStarlarkAction(
 
     if (execGroupUnchecked != Starlark.NONE) {
       String execGroup = (String) execGroupUnchecked;
-      if (!ExecGroupCollection.isValidGroupName(execGroup)
+      if (!StarlarkExecGroupCollection.isValidGroupName(execGroup)
           || !ruleContext.hasToolchainContext(execGroup)) {
         throw Starlark.errorf("Action declared for non-existent exec group '%s'.", execGroup);
       }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkExecGroupCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkExecGroupCollection.java
@@ -11,14 +11,17 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
-package com.google.devtools.build.lib.analysis;
+package com.google.devtools.build.lib.analysis.starlark;
 
 import static com.google.devtools.build.lib.analysis.ToolchainCollection.DEFAULT_EXEC_GROUP_NAME;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
+import com.google.devtools.build.lib.analysis.ToolchainCollection;
 import com.google.devtools.build.lib.starlarkbuildapi.platform.ExecGroupCollectionApi;
+import com.google.devtools.build.lib.starlarkbuildapi.platform.ToolchainContextApi;
 import java.util.List;
 import java.util.stream.Collectors;
 import net.starlark.java.eval.EvalException;
@@ -33,12 +36,15 @@
  * starlark.
  */
 @AutoValue
-public abstract class ExecGroupCollection implements ExecGroupCollectionApi {
+public abstract class StarlarkExecGroupCollection implements ExecGroupCollectionApi {
 
-  /** Returns a new {@link ExecGroupCollection} backed by the given {@code toolchainCollection}. */
-  public static ExecGroupCollection create(
+  /**
+   * Returns a new {@link StarlarkExecGroupCollection} backed by the given {@code
+   * toolchainCollection}.
+   */
+  public static StarlarkExecGroupCollection create(
       ToolchainCollection<ResolvedToolchainContext> toolchainCollection) {
-    return new AutoValue_ExecGroupCollection(toolchainCollection);
+    return new AutoValue_StarlarkExecGroupCollection(toolchainCollection);
   }
 
   protected abstract ToolchainCollection<ResolvedToolchainContext> toolchainCollection();
@@ -60,12 +66,13 @@ public boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalE
   }
 
   /**
-   * This creates a new {@link ExecGroupContext} object every time this is called. This seems better
-   * than pre-creating and storing all {@link ExecGroupContext}s since they're just thin wrappers
-   * around {@link ResolvedToolchainContext} objects.
+   * This creates a new {@link StarlarkExecGroupContext} object every time this is called. This
+   * seems better than pre-creating and storing all {@link StarlarkExecGroupContext}s since they're
+   * just thin wrappers around {@link ResolvedToolchainContext} objects.
    */
   @Override
-  public ExecGroupContext getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
+  public StarlarkExecGroupContext getIndex(StarlarkSemantics semantics, Object key)
+      throws EvalException {
     String execGroup = castGroupName(key);
     if (!containsKey(semantics, key)) {
       throw Starlark.errorf(
@@ -74,7 +81,8 @@ public ExecGroupContext getIndex(StarlarkSemantics semantics, Object key) throws
           execGroup,
           String.join(", ", getScrubbedExecGroups()));
     }
-    return new ExecGroupContext(toolchainCollection().getToolchainContext(execGroup));
+    ToolchainContextApi toolchainContext = toolchainCollection().getToolchainContext(execGroup);
+    return new StarlarkExecGroupContext(toolchainContext);
   }
 
   private static String castGroupName(Object key) throws EvalException {
@@ -105,16 +113,16 @@ private List<String> getScrubbedExecGroups() {
    * The starlark object that is returned by ctx.exec_groups[<name>]. Gives information about that
    * exec group.
    */
-  public static class ExecGroupContext implements ExecGroupContextApi {
-    ResolvedToolchainContext resolvedToolchainContext;
+  public static class StarlarkExecGroupContext implements ExecGroupContextApi {
+    ToolchainContextApi toolchainContext;
 
-    private ExecGroupContext(ResolvedToolchainContext resolvedToolchainContext) {
-      this.resolvedToolchainContext = resolvedToolchainContext;
+    private StarlarkExecGroupContext(ToolchainContextApi toolchainContext) {
+      this.toolchainContext = toolchainContext;
     }
 
     @Override
-    public ResolvedToolchainContext toolchains() {
-      return resolvedToolchainContext;
+    public ToolchainContextApi toolchains() {
+      return toolchainContext;
     }
 
     @Override
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
@@ -34,7 +34,6 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
-import com.google.devtools.build.lib.analysis.ExecGroupCollection;
 import com.google.devtools.build.lib.analysis.RuleDefinitionContext;
 import com.google.devtools.build.lib.analysis.TemplateVariableInfo;
 import com.google.devtools.build.lib.analysis.config.ConfigAwareRuleClassBuilder;
@@ -383,7 +382,7 @@ public StarlarkCallable rule(
           Dict.cast(execGroups, String.class, ExecGroup.class, "exec_group");
       for (String group : execGroupDict.keySet()) {
         // TODO(b/151742236): document this in the param documentation.
-        if (!ExecGroupCollection.isValidGroupName(group)) {
+        if (!StarlarkExecGroupCollection.isValidGroupName(group)) {
           throw Starlark.errorf("Exec group name '%s' is not a valid name.", group);
         }
       }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java
@@ -34,7 +34,6 @@
 import com.google.devtools.build.lib.analysis.CommandHelper;
 import com.google.devtools.build.lib.analysis.ConfigurationMakeVariableContext;
 import com.google.devtools.build.lib.analysis.DefaultInfo;
-import com.google.devtools.build.lib.analysis.ExecGroupCollection;
 import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.LabelExpander;
@@ -697,9 +696,9 @@ public boolean targetPlatformHasConstraint(ConstraintValueInfo constraintValue)
   }
 
   @Override
-  public ExecGroupCollection execGroups() {
+  public StarlarkExecGroupCollection execGroups() {
     // Create a thin wrapper around the toolchain collection, to expose the Starlark API.
-    return ExecGroupCollection.create(ruleContext.getToolchainContexts());
+    return StarlarkExecGroupCollection.create(ruleContext.getToolchainContexts());
   }
 
   @Override
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/BUILD b/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:file_provider",
         "//src/main/java/com/google/devtools/build/lib/analysis:resolved_toolchain_context",
         "//src/main/java/com/google/devtools/build/lib/analysis:starlark/args",
+        "//src/main/java/com/google/devtools/build/lib/analysis:starlark/starlark_exec_group_collection",
         "//src/main/java/com/google/devtools/build/lib/analysis:test/analysis_failure",
         "//src/main/java/com/google/devtools/build/lib/analysis:test/analysis_failure_info",
         "//src/main/java/com/google/devtools/build/lib/analysis:test/analysis_test_result_info",
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java
@@ -32,12 +32,12 @@
 import com.google.devtools.build.lib.analysis.ActionsProvider;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.ExecGroupCollection;
 import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.StarlarkAction;
 import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTarget;
+import com.google.devtools.build.lib.analysis.starlark.StarlarkExecGroupCollection;
 import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.analysis.util.MockRule;
@@ -2858,9 +2858,10 @@ public void testExecGroup_toolchain() throws Exception {
                     Label.parseAbsoluteUnchecked("//something:defs.bzl"), "result"));
     assertThat(info).isNotNull();
     assertThat(info.getValue("toolchain_value")).isEqualTo("foo");
-    assertThat(info.getValue("exec_groups")).isInstanceOf(ExecGroupCollection.class);
+    assertThat(info.getValue("exec_groups")).isInstanceOf(StarlarkExecGroupCollection.class);
     ImmutableMap<String, ResolvedToolchainContext> toolchainContexts =
-        ((ExecGroupCollection) info.getValue("exec_groups")).getToolchainCollectionForTesting();
+        ((StarlarkExecGroupCollection) info.getValue("exec_groups"))
+            .getToolchainCollectionForTesting();
     assertThat(toolchainContexts.keySet()).containsExactly(DEFAULT_EXEC_GROUP_NAME, "dragonfruit");
     assertThat(toolchainContexts.get(DEFAULT_EXEC_GROUP_NAME).requiredToolchainTypes()).isEmpty();
     assertThat(toolchainContexts.get("dragonfruit").resolvedToolchainLabels())
@@ -2914,9 +2915,10 @@ public void testExecGroup_duplicateToolchainType() throws Exception {
                     Label.parseAbsoluteUnchecked("//something:defs.bzl"), "result"));
     assertThat(info).isNotNull();
     assertThat(info.getValue("toolchain_value")).isEqualTo("foo");
-    assertThat(info.getValue("exec_groups")).isInstanceOf(ExecGroupCollection.class);
+    assertThat(info.getValue("exec_groups")).isInstanceOf(StarlarkExecGroupCollection.class);
     ImmutableMap<String, ResolvedToolchainContext> toolchainContexts =
-        ((ExecGroupCollection) info.getValue("exec_groups")).getToolchainCollectionForTesting();
+        ((StarlarkExecGroupCollection) info.getValue("exec_groups"))
+            .getToolchainCollectionForTesting();
     assertThat(toolchainContexts.keySet())
         .containsExactly(DEFAULT_EXEC_GROUP_NAME, "dragonfruit", "passionfruit");
     assertThat(toolchainContexts.get(DEFAULT_EXEC_GROUP_NAME).requiredToolchainTypes()).isEmpty();
