Skip to content

Commit

Permalink
Renamed ExecGroupCollection to clarify that it is only for Starlark u…
Browse files Browse the repository at this point in the history
…sage.

Non-starlark usage can go via the ToolchainCollection directly.

PiperOrigin-RevId: 367461392
  • Loading branch information
katre committed Jul 13, 2021
1 parent 0cbb8a8 commit d2e21ce
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 30 deletions.
16 changes: 15 additions & 1 deletion src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ java_library(
"DependencyResolver.java",
"EmptyConfiguredTarget.java",
"EventHandlingErrorReporter.java",
"ExecGroupCollection.java",
"Expander.java",
"ExtraActionUtils.java",
"ExtraActionsVisitor.java",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand All @@ -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(
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/starlark/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit d2e21ce

Please sign in to comment.