Skip to content

Commit 41877d0

Browse files
katrekatre
committed
Extract a separate StarlarkToolchainContext for starlark-only operations.
Also make ToolchainContextApi use starlark threads. PiperOrigin-RevId: 367515900
1 parent b5f1d63 commit 41877d0

File tree

6 files changed

+151
-94
lines changed

6 files changed

+151
-94
lines changed

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ java_library(
349349
":starlark/starlark_command_line",
350350
":starlark/starlark_exec_group_collection",
351351
":starlark/starlark_late_bound_default",
352+
":starlark/starlark_toolchain_context",
352353
":template_variable_info",
353354
":test/analysis_failure",
354355
":test/analysis_failure_info",
@@ -990,8 +991,6 @@ java_library(
990991
"//src/main/java/com/google/devtools/build/lib/skyframe:toolchain_context_key",
991992
"//src/main/java/com/google/devtools/build/lib/skyframe:toolchain_exception",
992993
"//src/main/java/com/google/devtools/build/lib/skyframe:unloaded_toolchain_context",
993-
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform",
994-
"//src/main/java/net/starlark/java/eval",
995994
"//src/main/protobuf:failure_details_java_proto",
996995
"//third_party:auto_value",
997996
"//third_party:guava",
@@ -2151,6 +2150,7 @@ java_library(
21512150
srcs = ["starlark/StarlarkExecGroupCollection.java"],
21522151
deps = [
21532152
":resolved_toolchain_context",
2153+
":starlark/starlark_toolchain_context",
21542154
":toolchain_collection",
21552155
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform",
21562156
"//src/main/java/net/starlark/java/eval",
@@ -2197,6 +2197,20 @@ java_library(
21972197
],
21982198
)
21992199

2200+
java_library(
2201+
name = "starlark/starlark_toolchain_context",
2202+
srcs = ["starlark/StarlarkToolchainContext.java"],
2203+
deps = [
2204+
":resolved_toolchain_context",
2205+
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
2206+
"//src/main/java/com/google/devtools/build/lib/cmdline",
2207+
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform",
2208+
"//src/main/java/net/starlark/java/eval",
2209+
"//third_party:auto_value",
2210+
"//third_party:jsr305",
2211+
],
2212+
)
2213+
22002214
# TODO(b/144899336): This should be lib/analysis/test/BUILD
22012215
java_library(
22022216
name = "test/analysis_failure",

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

Lines changed: 4 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -14,29 +14,21 @@
1414

1515
package com.google.devtools.build.lib.analysis;
1616

17-
import static java.util.stream.Collectors.joining;
18-
1917
import com.google.auto.value.AutoValue;
2018
import com.google.common.collect.ImmutableList;
2119
import com.google.common.collect.ImmutableMap;
2220
import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
2321
import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
2422
import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
2523
import com.google.devtools.build.lib.cmdline.Label;
26-
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
2724
import com.google.devtools.build.lib.cmdline.RepositoryName;
2825
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
2926
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
3027
import com.google.devtools.build.lib.server.FailureDetails.Toolchain.Code;
3128
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
3229
import com.google.devtools.build.lib.skyframe.ToolchainException;
3330
import com.google.devtools.build.lib.skyframe.UnloadedToolchainContext;
34-
import com.google.devtools.build.lib.starlarkbuildapi.platform.ToolchainContextApi;
3531
import javax.annotation.Nullable;
36-
import net.starlark.java.eval.EvalException;
37-
import net.starlark.java.eval.Printer;
38-
import net.starlark.java.eval.Starlark;
39-
import net.starlark.java.eval.StarlarkSemantics;
4032

4133
/**
4234
* Represents the data needed for a specific target's use of toolchains and platforms, including
@@ -45,7 +37,7 @@
4537
@AutoValue
4638
@Immutable
4739
@ThreadSafe
48-
public abstract class ResolvedToolchainContext implements ToolchainContextApi, ToolchainContext {
40+
public abstract class ResolvedToolchainContext implements ToolchainContext {
4941

5042
/**
5143
* Finishes preparing the {@link ResolvedToolchainContext} by finding the specific toolchain
@@ -107,15 +99,15 @@ public static ResolvedToolchainContext load(
10799
}
108100

109101
/** Returns the repository mapping applied by the Starlark 'in' operator to string-form labels. */
110-
abstract ImmutableMap<RepositoryName, RepositoryName> repoMapping();
102+
public abstract ImmutableMap<RepositoryName, RepositoryName> repoMapping();
111103

112104
/** Returns a description of the target being used, for error messaging. */
113105
public abstract String targetDescription();
114106

115107
/** Sets the map from requested {@link Label} to toolchain type provider. */
116-
abstract ImmutableMap<Label, ToolchainTypeInfo> requestedToolchainTypeLabels();
108+
public abstract ImmutableMap<Label, ToolchainTypeInfo> requestedToolchainTypeLabels();
117109

118-
abstract ImmutableMap<ToolchainTypeInfo, ToolchainInfo> toolchains();
110+
public abstract ImmutableMap<ToolchainTypeInfo, ToolchainInfo> toolchains();
119111

120112
/** Returns the template variables that these toolchains provide. */
121113
public abstract ImmutableList<TemplateVariableInfo> templateVariableProviders();
@@ -138,67 +130,6 @@ public ToolchainInfo forToolchainType(ToolchainTypeInfo toolchainType) {
138130
return toolchains().get(toolchainType);
139131
}
140132

141-
@Override
142-
public boolean isImmutable() {
143-
return true;
144-
}
145-
146-
@Override
147-
public void repr(Printer printer) {
148-
printer.append("<toolchain_context.resolved_labels: ");
149-
printer.append(
150-
toolchains().keySet().stream()
151-
.map(ToolchainTypeInfo::typeLabel)
152-
.map(Label::toString)
153-
.collect(joining(", ")));
154-
printer.append(">");
155-
}
156-
157-
private static Label transformKey(
158-
Object key, ImmutableMap<RepositoryName, RepositoryName> repoMapping) throws EvalException {
159-
if (key instanceof Label) {
160-
return (Label) key;
161-
} else if (key instanceof ToolchainTypeInfo) {
162-
return ((ToolchainTypeInfo) key).typeLabel();
163-
} else if (key instanceof String) {
164-
try {
165-
return Label.parseAbsolute((String) key, repoMapping);
166-
} catch (LabelSyntaxException e) {
167-
throw Starlark.errorf("Unable to parse toolchain label '%s': %s", key, e.getMessage());
168-
}
169-
} else {
170-
throw Starlark.errorf(
171-
"Toolchains only supports indexing by toolchain type, got %s instead",
172-
Starlark.type(key));
173-
}
174-
}
175-
176-
@Override
177-
public ToolchainInfo getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
178-
Label toolchainTypeLabel = transformKey(key, repoMapping());
179-
180-
if (!containsKey(semantics, key)) {
181-
// TODO(bazel-configurability): The list of available toolchain types is confusing in the
182-
// presence of aliases, since it only contains the actual label, not the alias passed to the
183-
// rule definition.
184-
throw Starlark.errorf(
185-
"In %s, toolchain type %s was requested but only types [%s] are configured",
186-
targetDescription(),
187-
toolchainTypeLabel,
188-
requiredToolchainTypes().stream()
189-
.map(ToolchainTypeInfo::typeLabel)
190-
.map(Label::toString)
191-
.collect(joining(", ")));
192-
}
193-
return forToolchainType(toolchainTypeLabel);
194-
}
195-
196-
@Override
197-
public boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalException {
198-
Label toolchainTypeLabel = transformKey(key, repoMapping());
199-
return requestedToolchainTypeLabels().containsKey(toolchainTypeLabel);
200-
}
201-
202133
/**
203134
* Exception used when a toolchain type is required but the resolved target does not have
204135
* ToolchainInfo.

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ public StarlarkExecGroupContext getIndex(StarlarkSemantics semantics, Object key
8181
execGroup,
8282
String.join(", ", getScrubbedExecGroups()));
8383
}
84-
ToolchainContextApi toolchainContext = toolchainCollection().getToolchainContext(execGroup);
84+
ToolchainContextApi toolchainContext =
85+
StarlarkToolchainContext.create(toolchainCollection().getToolchainContext(execGroup));
8586
return new AutoValue_StarlarkExecGroupCollection_StarlarkExecGroupContext(toolchainContext);
8687
}
8788

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

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import com.google.devtools.build.lib.analysis.LabelExpander;
4040
import com.google.devtools.build.lib.analysis.LabelExpander.NotUniqueExpansionException;
4141
import com.google.devtools.build.lib.analysis.LocationExpander;
42-
import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
4342
import com.google.devtools.build.lib.analysis.RuleContext;
4443
import com.google.devtools.build.lib.analysis.Runfiles;
4544
import com.google.devtools.build.lib.analysis.RunfilesProvider;
@@ -688,22 +687,7 @@ public Dict<String, String> var() throws EvalException {
688687
@Override
689688
public ToolchainContextApi toolchains() throws EvalException {
690689
checkMutable("toolchains");
691-
ResolvedToolchainContext toolchainContext = ruleContext.getToolchainContext();
692-
if (toolchainContext == null) {
693-
// Starlark rules are easier if this cannot be null, so return a no-op value instead.
694-
return new ToolchainContextApi() {
695-
@Override
696-
public Object getIndex(StarlarkSemantics semantics, Object key) {
697-
return Starlark.NONE;
698-
}
699-
700-
@Override
701-
public boolean containsKey(StarlarkSemantics semantics, Object key) {
702-
return false;
703-
}
704-
};
705-
}
706-
return toolchainContext;
690+
return StarlarkToolchainContext.create(ruleContext.getToolchainContext());
707691
}
708692

709693
@Override

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

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
// Copyright 2021 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.lib.analysis.starlark;
15+
16+
import static java.util.stream.Collectors.joining;
17+
18+
import com.google.auto.value.AutoValue;
19+
import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
20+
import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
21+
import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
22+
import com.google.devtools.build.lib.cmdline.Label;
23+
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
24+
import com.google.devtools.build.lib.starlarkbuildapi.platform.ToolchainContextApi;
25+
import javax.annotation.Nullable;
26+
import net.starlark.java.eval.EvalException;
27+
import net.starlark.java.eval.Printer;
28+
import net.starlark.java.eval.Starlark;
29+
import net.starlark.java.eval.StarlarkSemantics;
30+
import net.starlark.java.eval.StarlarkThread;
31+
32+
/**
33+
* An implementation of ToolchainContextApi that can better handle converting strings into Labels.
34+
*/
35+
@AutoValue
36+
public abstract class StarlarkToolchainContext implements ToolchainContextApi {
37+
38+
private static final ToolchainContextApi NO_OP =
39+
new ToolchainContextApi() {
40+
@Override
41+
public Object getIndex(
42+
StarlarkThread starlarkThread, StarlarkSemantics semantics, Object key) {
43+
// TODO(jcater): throw Starlark.errorf instead of returning NONE.
44+
return Starlark.NONE;
45+
}
46+
47+
@Override
48+
public boolean containsKey(
49+
StarlarkThread starlarkThread, StarlarkSemantics semantics, Object key) {
50+
return false;
51+
}
52+
};
53+
54+
public static ToolchainContextApi create(@Nullable ResolvedToolchainContext toolchainContext) {
55+
if (toolchainContext == null) {
56+
return NO_OP;
57+
}
58+
59+
return new AutoValue_StarlarkToolchainContext(toolchainContext);
60+
}
61+
62+
protected abstract ResolvedToolchainContext toolchainContext();
63+
64+
@Override
65+
public boolean isImmutable() {
66+
return true;
67+
}
68+
69+
@Override
70+
public void repr(Printer printer) {
71+
printer.append("<toolchain_context.resolved_labels: ");
72+
printer.append(
73+
toolchainContext().toolchains().keySet().stream()
74+
.map(ToolchainTypeInfo::typeLabel)
75+
.map(Label::toString)
76+
.collect(joining(", ")));
77+
printer.append(">");
78+
}
79+
80+
private Label transformKey(StarlarkThread starlarkThread, Object key) throws EvalException {
81+
if (key instanceof Label) {
82+
return (Label) key;
83+
} else if (key instanceof ToolchainTypeInfo) {
84+
return ((ToolchainTypeInfo) key).typeLabel();
85+
} else if (key instanceof String) {
86+
try {
87+
// TODO(jcater): Use LabelConversionContext and StarlarkThread to convert this instead.
88+
// Also remove repoMapping from ResolvedToolchainContext.
89+
return Label.parseAbsolute((String) key, toolchainContext().repoMapping());
90+
} catch (LabelSyntaxException e) {
91+
throw Starlark.errorf("Unable to parse toolchain label '%s': %s", key, e.getMessage());
92+
}
93+
} else {
94+
throw Starlark.errorf(
95+
"Toolchains only supports indexing by toolchain type, got %s instead",
96+
Starlark.type(key));
97+
}
98+
}
99+
100+
@Override
101+
public ToolchainInfo getIndex(
102+
StarlarkThread starlarkThread, StarlarkSemantics semantics, Object key) throws EvalException {
103+
Label toolchainTypeLabel = transformKey(starlarkThread, key);
104+
105+
if (!containsKey(starlarkThread, semantics, key)) {
106+
// TODO(bazel-configurability): The list of available toolchain types is confusing in the
107+
// presence of aliases, since it only contains the actual label, not the alias passed to the
108+
// rule definition.
109+
throw Starlark.errorf(
110+
"In %s, toolchain type %s was requested but only types [%s] are configured",
111+
toolchainContext().targetDescription(),
112+
toolchainTypeLabel,
113+
toolchainContext().requiredToolchainTypes().stream()
114+
.map(ToolchainTypeInfo::typeLabel)
115+
.map(Label::toString)
116+
.collect(joining(", ")));
117+
}
118+
return toolchainContext().forToolchainType(toolchainTypeLabel);
119+
}
120+
121+
@Override
122+
public boolean containsKey(StarlarkThread starlarkThread, StarlarkSemantics semantics, Object key)
123+
throws EvalException {
124+
Label toolchainTypeLabel = transformKey(starlarkThread, key);
125+
return toolchainContext().requestedToolchainTypeLabels().containsKey(toolchainTypeLabel);
126+
}
127+
}

‎src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform/ToolchainContextApi.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,4 @@
2727
"Holds toolchains available for a particular exec group. Toolchain targets are accessed by"
2828
+ " indexing with the toolchain type, as in"
2929
+ " <code>context[\"//pkg:my_toolchain_type\"]</code>.")
30-
public interface ToolchainContextApi extends StarlarkValue, StarlarkIndexable {}
30+
public interface ToolchainContextApi extends StarlarkValue, StarlarkIndexable.Threaded {}

0 commit comments

Comments
 (0)