Skip to content

Commit d2e21ce

Browse files
katrekatre
committed
Renamed ExecGroupCollection to clarify that it is only for Starlark usage.
Non-starlark usage can go via the ToolchainCollection directly. PiperOrigin-RevId: 367461392
1 parent 0cbb8a8 commit d2e21ce

8 files changed

Lines changed: 52 additions & 30 deletions

File tree

‎src/main/java/com/google/devtools/build/lib/analysis/BUILD‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,6 @@ java_library(
188188
"DependencyResolver.java",
189189
"EmptyConfiguredTarget.java",
190190
"EventHandlingErrorReporter.java",
191-
"ExecGroupCollection.java",
192191
"Expander.java",
193192
"ExtraActionUtils.java",
194193
"ExtraActionsVisitor.java",
@@ -348,6 +347,7 @@ java_library(
348347
":starlark/function_transition_util",
349348
":starlark/starlark_api_provider",
350349
":starlark/starlark_command_line",
350+
":starlark/starlark_exec_group_collection",
351351
":starlark/starlark_late_bound_default",
352352
":template_variable_info",
353353
":test/analysis_failure",
@@ -2146,6 +2146,20 @@ java_library(
21462146
],
21472147
)
21482148

2149+
java_library(
2150+
name = "starlark/starlark_exec_group_collection",
2151+
srcs = ["starlark/StarlarkExecGroupCollection.java"],
2152+
deps = [
2153+
":resolved_toolchain_context",
2154+
":toolchain_collection",
2155+
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform",
2156+
"//src/main/java/net/starlark/java/eval",
2157+
"//src/main/java/net/starlark/java/syntax",
2158+
"//third_party:auto_value",
2159+
"//third_party:guava",
2160+
],
2161+
)
2162+
21492163
java_library(
21502164
name = "starlark/starlark_error_reporter",
21512165
srcs = ["starlark/StarlarkErrorReporter.java"],

‎src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public static ResolvedToolchainContext load(
110110
abstract ImmutableMap<RepositoryName, RepositoryName> repoMapping();
111111

112112
/** Returns a description of the target being used, for error messaging. */
113-
abstract String targetDescription();
113+
public abstract String targetDescription();
114114

115115
/** Sets the map from requested {@link Label} to toolchain type provider. */
116116
abstract ImmutableMap<Label, ToolchainTypeInfo> requestedToolchainTypeLabels();

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import com.google.devtools.build.lib.actions.extra.SpawnInfo;
3131
import com.google.devtools.build.lib.analysis.BashCommandConstructor;
3232
import com.google.devtools.build.lib.analysis.CommandHelper;
33-
import com.google.devtools.build.lib.analysis.ExecGroupCollection;
3433
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
3534
import com.google.devtools.build.lib.analysis.PseudoAction;
3635
import com.google.devtools.build.lib.analysis.RuleContext;
@@ -618,7 +617,7 @@ private void registerStarlarkAction(
618617

619618
if (execGroupUnchecked != Starlark.NONE) {
620619
String execGroup = (String) execGroupUnchecked;
621-
if (!ExecGroupCollection.isValidGroupName(execGroup)
620+
if (!StarlarkExecGroupCollection.isValidGroupName(execGroup)
622621
|| !ruleContext.hasToolchainContext(execGroup)) {
623622
throw Starlark.errorf("Action declared for non-existent exec group '%s'.", execGroup);
624623
}

src/main/java/com/google/devtools/build/lib/analysis/ExecGroupCollection.java renamed to src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkExecGroupCollection.java

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,17 @@
1111
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
14-
package com.google.devtools.build.lib.analysis;
14+
package com.google.devtools.build.lib.analysis.starlark;
1515

1616
import static com.google.devtools.build.lib.analysis.ToolchainCollection.DEFAULT_EXEC_GROUP_NAME;
1717

1818
import com.google.auto.value.AutoValue;
1919
import com.google.common.annotations.VisibleForTesting;
2020
import com.google.common.collect.ImmutableMap;
21+
import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
22+
import com.google.devtools.build.lib.analysis.ToolchainCollection;
2123
import com.google.devtools.build.lib.starlarkbuildapi.platform.ExecGroupCollectionApi;
24+
import com.google.devtools.build.lib.starlarkbuildapi.platform.ToolchainContextApi;
2225
import java.util.List;
2326
import java.util.stream.Collectors;
2427
import net.starlark.java.eval.EvalException;
@@ -33,12 +36,15 @@
3336
* starlark.
3437
*/
3538
@AutoValue
36-
public abstract class ExecGroupCollection implements ExecGroupCollectionApi {
39+
public abstract class StarlarkExecGroupCollection implements ExecGroupCollectionApi {
3740

38-
/** Returns a new {@link ExecGroupCollection} backed by the given {@code toolchainCollection}. */
39-
public static ExecGroupCollection create(
41+
/**
42+
* Returns a new {@link StarlarkExecGroupCollection} backed by the given {@code
43+
* toolchainCollection}.
44+
*/
45+
public static StarlarkExecGroupCollection create(
4046
ToolchainCollection<ResolvedToolchainContext> toolchainCollection) {
41-
return new AutoValue_ExecGroupCollection(toolchainCollection);
47+
return new AutoValue_StarlarkExecGroupCollection(toolchainCollection);
4248
}
4349

4450
protected abstract ToolchainCollection<ResolvedToolchainContext> toolchainCollection();
@@ -60,12 +66,13 @@ public boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalE
6066
}
6167

6268
/**
63-
* This creates a new {@link ExecGroupContext} object every time this is called. This seems better
64-
* than pre-creating and storing all {@link ExecGroupContext}s since they're just thin wrappers
65-
* around {@link ResolvedToolchainContext} objects.
69+
* This creates a new {@link StarlarkExecGroupContext} object every time this is called. This
70+
* seems better than pre-creating and storing all {@link StarlarkExecGroupContext}s since they're
71+
* just thin wrappers around {@link ResolvedToolchainContext} objects.
6672
*/
6773
@Override
68-
public ExecGroupContext getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
74+
public StarlarkExecGroupContext getIndex(StarlarkSemantics semantics, Object key)
75+
throws EvalException {
6976
String execGroup = castGroupName(key);
7077
if (!containsKey(semantics, key)) {
7178
throw Starlark.errorf(
@@ -74,7 +81,8 @@ public ExecGroupContext getIndex(StarlarkSemantics semantics, Object key) throws
7481
execGroup,
7582
String.join(", ", getScrubbedExecGroups()));
7683
}
77-
return new ExecGroupContext(toolchainCollection().getToolchainContext(execGroup));
84+
ToolchainContextApi toolchainContext = toolchainCollection().getToolchainContext(execGroup);
85+
return new StarlarkExecGroupContext(toolchainContext);
7886
}
7987

8088
private static String castGroupName(Object key) throws EvalException {
@@ -105,16 +113,16 @@ private List<String> getScrubbedExecGroups() {
105113
* The starlark object that is returned by ctx.exec_groups[<name>]. Gives information about that
106114
* exec group.
107115
*/
108-
public static class ExecGroupContext implements ExecGroupContextApi {
109-
ResolvedToolchainContext resolvedToolchainContext;
116+
public static class StarlarkExecGroupContext implements ExecGroupContextApi {
117+
ToolchainContextApi toolchainContext;
110118

111-
private ExecGroupContext(ResolvedToolchainContext resolvedToolchainContext) {
112-
this.resolvedToolchainContext = resolvedToolchainContext;
119+
private StarlarkExecGroupContext(ToolchainContextApi toolchainContext) {
120+
this.toolchainContext = toolchainContext;
113121
}
114122

115123
@Override
116-
public ResolvedToolchainContext toolchains() {
117-
return resolvedToolchainContext;
124+
public ToolchainContextApi toolchains() {
125+
return toolchainContext;
118126
}
119127

120128
@Override

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import com.google.common.collect.ImmutableSet;
3535
import com.google.devtools.build.lib.actions.Artifact;
3636
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
37-
import com.google.devtools.build.lib.analysis.ExecGroupCollection;
3837
import com.google.devtools.build.lib.analysis.RuleDefinitionContext;
3938
import com.google.devtools.build.lib.analysis.TemplateVariableInfo;
4039
import com.google.devtools.build.lib.analysis.config.ConfigAwareRuleClassBuilder;
@@ -383,7 +382,7 @@ public StarlarkCallable rule(
383382
Dict.cast(execGroups, String.class, ExecGroup.class, "exec_group");
384383
for (String group : execGroupDict.keySet()) {
385384
// TODO(b/151742236): document this in the param documentation.
386-
if (!ExecGroupCollection.isValidGroupName(group)) {
385+
if (!StarlarkExecGroupCollection.isValidGroupName(group)) {
387386
throw Starlark.errorf("Exec group name '%s' is not a valid name.", group);
388387
}
389388
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import com.google.devtools.build.lib.analysis.CommandHelper;
3535
import com.google.devtools.build.lib.analysis.ConfigurationMakeVariableContext;
3636
import com.google.devtools.build.lib.analysis.DefaultInfo;
37-
import com.google.devtools.build.lib.analysis.ExecGroupCollection;
3837
import com.google.devtools.build.lib.analysis.FileProvider;
3938
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
4039
import com.google.devtools.build.lib.analysis.LabelExpander;
@@ -697,9 +696,9 @@ public boolean targetPlatformHasConstraint(ConstraintValueInfo constraintValue)
697696
}
698697

699698
@Override
700-
public ExecGroupCollection execGroups() {
699+
public StarlarkExecGroupCollection execGroups() {
701700
// Create a thin wrapper around the toolchain collection, to expose the Starlark API.
702-
return ExecGroupCollection.create(ruleContext.getToolchainContexts());
701+
return StarlarkExecGroupCollection.create(ruleContext.getToolchainContexts());
703702
}
704703

705704
@Override

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ java_test(
4141
"//src/main/java/com/google/devtools/build/lib/analysis:file_provider",
4242
"//src/main/java/com/google/devtools/build/lib/analysis:resolved_toolchain_context",
4343
"//src/main/java/com/google/devtools/build/lib/analysis:starlark/args",
44+
"//src/main/java/com/google/devtools/build/lib/analysis:starlark/starlark_exec_group_collection",
4445
"//src/main/java/com/google/devtools/build/lib/analysis:test/analysis_failure",
4546
"//src/main/java/com/google/devtools/build/lib/analysis:test/analysis_failure_info",
4647
"//src/main/java/com/google/devtools/build/lib/analysis:test/analysis_test_result_info",

‎src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@
3232
import com.google.devtools.build.lib.analysis.ActionsProvider;
3333
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
3434
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
35-
import com.google.devtools.build.lib.analysis.ExecGroupCollection;
3635
import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
3736
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
3837
import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
3938
import com.google.devtools.build.lib.analysis.actions.StarlarkAction;
4039
import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTarget;
40+
import com.google.devtools.build.lib.analysis.starlark.StarlarkExecGroupCollection;
4141
import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext;
4242
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
4343
import com.google.devtools.build.lib.analysis.util.MockRule;
@@ -2858,9 +2858,10 @@ public void testExecGroup_toolchain() throws Exception {
28582858
Label.parseAbsoluteUnchecked("//something:defs.bzl"), "result"));
28592859
assertThat(info).isNotNull();
28602860
assertThat(info.getValue("toolchain_value")).isEqualTo("foo");
2861-
assertThat(info.getValue("exec_groups")).isInstanceOf(ExecGroupCollection.class);
2861+
assertThat(info.getValue("exec_groups")).isInstanceOf(StarlarkExecGroupCollection.class);
28622862
ImmutableMap<String, ResolvedToolchainContext> toolchainContexts =
2863-
((ExecGroupCollection) info.getValue("exec_groups")).getToolchainCollectionForTesting();
2863+
((StarlarkExecGroupCollection) info.getValue("exec_groups"))
2864+
.getToolchainCollectionForTesting();
28642865
assertThat(toolchainContexts.keySet()).containsExactly(DEFAULT_EXEC_GROUP_NAME, "dragonfruit");
28652866
assertThat(toolchainContexts.get(DEFAULT_EXEC_GROUP_NAME).requiredToolchainTypes()).isEmpty();
28662867
assertThat(toolchainContexts.get("dragonfruit").resolvedToolchainLabels())
@@ -2914,9 +2915,10 @@ public void testExecGroup_duplicateToolchainType() throws Exception {
29142915
Label.parseAbsoluteUnchecked("//something:defs.bzl"), "result"));
29152916
assertThat(info).isNotNull();
29162917
assertThat(info.getValue("toolchain_value")).isEqualTo("foo");
2917-
assertThat(info.getValue("exec_groups")).isInstanceOf(ExecGroupCollection.class);
2918+
assertThat(info.getValue("exec_groups")).isInstanceOf(StarlarkExecGroupCollection.class);
29182919
ImmutableMap<String, ResolvedToolchainContext> toolchainContexts =
2919-
((ExecGroupCollection) info.getValue("exec_groups")).getToolchainCollectionForTesting();
2920+
((StarlarkExecGroupCollection) info.getValue("exec_groups"))
2921+
.getToolchainCollectionForTesting();
29202922
assertThat(toolchainContexts.keySet())
29212923
.containsExactly(DEFAULT_EXEC_GROUP_NAME, "dragonfruit", "passionfruit");
29222924
assertThat(toolchainContexts.get(DEFAULT_EXEC_GROUP_NAME).requiredToolchainTypes()).isEmpty();

0 commit comments

Comments
 (0)