From d2e21cec31f09b27ef3589f47b0779f34077ca7e Mon Sep 17 00:00:00 2001 From: John Cater Date: Thu, 8 Apr 2021 10:44:57 -0700 Subject: [PATCH] Renamed ExecGroupCollection to clarify that it is only for Starlark usage. Non-starlark usage can go via the ToolchainCollection directly. PiperOrigin-RevId: 367461392 --- .../google/devtools/build/lib/analysis/BUILD | 16 +++++++- .../analysis/ResolvedToolchainContext.java | 2 +- .../starlark/StarlarkActionFactory.java | 3 +- .../StarlarkExecGroupCollection.java} | 40 +++++++++++-------- .../starlark/StarlarkRuleClassFunctions.java | 3 +- .../starlark/StarlarkRuleContext.java | 5 +-- .../google/devtools/build/lib/starlark/BUILD | 1 + .../lib/starlark/StarlarkRuleContextTest.java | 12 +++--- 8 files changed, 52 insertions(+), 30 deletions(-) rename src/main/java/com/google/devtools/build/lib/analysis/{ExecGroupCollection.java => starlark/StarlarkExecGroupCollection.java} (70%) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index c515d2c2d59d08..0c473184eea086 100644 --- 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 index d7c8e837f02ac7..a707abd7c0687d 100644 --- 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 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 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 index 31266ec9550829..5082581ea32735 100644 --- 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/ExecGroupCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkExecGroupCollection.java similarity index 70% rename from src/main/java/com/google/devtools/build/lib/analysis/ExecGroupCollection.java rename to src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkExecGroupCollection.java index 6059857ee5b554..17e5652840990b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ExecGroupCollection.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 toolchainCollection) { - return new AutoValue_ExecGroupCollection(toolchainCollection); + return new AutoValue_StarlarkExecGroupCollection(toolchainCollection); } protected abstract ToolchainCollection 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 getScrubbedExecGroups() { * The starlark object that is returned by ctx.exec_groups[]. 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 index fe5bd9d0833777..64825ca315aada 100644 --- 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 index 304db499323989..bb445cd7f104f9 100644 --- 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 index 468654bdacd6ac..1d56e77698e3fe 100644 --- 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 index de8144e062f013..0df3ddb2a64682 100644 --- 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 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 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();