Skip to content

Commit 883ce21

Browse files
Wyveraldcopybara-github
Wyverald
authored and
copybara-github
committed
Add a conversionContext parameter to option converters
We need to pass flag values through the repo mapping of the main repo, which means that at flag parsing time, we need access to the main repo mapping. To that end, we add a nullable untyped `conversionContext` parameter to the `Converter#convert` method, which is unused in this CL but will be used in a follow-up. Note that we can't directly add a `RepositoryMapping` parameter because the c.g.devtools.common.options package is a transitive dependency of c.g.devtools.build.lib.cmdline (which RepositoryMapping lives in). So this `conversionContext` will unfortunately need to be an Object. Reviewers: Please focus on reviewing changes in the c.g.devtools.common.options package. All the other changes in this CL are simply adding a `conversionContext` parameter to implementors of `Converter`, or passing this parameter to delegates, or superclasses. Work towards #14852 PiperOrigin-RevId: 459278433 Change-Id: I98b3842305c34d2d0c33e7411c1024897fb0170a
1 parent 8e03d82 commit 883ce21

69 files changed

Lines changed: 532 additions & 325 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/Main.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ static Options parseCommandLineOptions(String[] args) throws IOException {
228228
}
229229

230230
/** Validating converter for Paths. A Path is considered valid if it resolves to a file. */
231-
public static class PathConverter implements Converter<Path> {
231+
public static class PathConverter extends Converter.Contextless<Path> {
232232

233233
private final boolean mustExist;
234234

‎src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,8 @@ public int hashCode() {
167167
+ getLocalTestCount() * p * p;
168168
}
169169

170-
public static class ResourceSetConverter implements Converter<ResourceSet> {
170+
/** Converter for {@link ResourceSet}. */
171+
public static class ResourceSetConverter extends Converter.Contextless<ResourceSet> {
171172
private static final Splitter SPLITTER = Splitter.on(',');
172173

173174
@Override

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ public static DetailedExitCode createDetailedExitCode(String message, Code detai
245245
}
246246

247247
/** Converter for {@code --embed_label} which rejects strings that span multiple lines. */
248-
public static final class OneLineStringConverter implements Converter<String> {
248+
public static final class OneLineStringConverter extends Converter.Contextless<String> {
249249

250250
@Override
251251
public String convert(String input) throws OptionsParsingException {

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
* <p>If the compilation happens remotely then the cpu of the remote machine might be different from
2727
* the auto-detected one and the --cpu and --host_cpu options must be set explicitly.
2828
*/
29-
public class AutoCpuConverter implements Converter<String> {
29+
public class AutoCpuConverter extends Converter.Contextless<String> {
3030
@Override
3131
public String convert(String input) throws OptionsParsingException {
3232
if (input.isEmpty()) {
@@ -83,7 +83,10 @@ public String convert(String input) throws OptionsParsingException {
8383
return input;
8484
}
8585

86-
/** Reverses the conversion performed by {@link #convert} to return the matching OS, CPU pair. */
86+
/**
87+
* Reverses the conversion performed by {@link Converter#convert} to return the matching OS, CPU
88+
* pair.
89+
*/
8790
public static Pair<CPU, OS> reverse(String input) {
8891
if (input == null || input.length() == 0 || "unknown".equals(input)) {
8992
// Use the auto-detected values.

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

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public class CoreOptionConverters {
6363
.buildOrThrow();
6464

6565
/** A converter from strings to Starlark int values. */
66-
private static class StarlarkIntConverter implements Converter<StarlarkInt> {
66+
private static class StarlarkIntConverter extends Converter.Contextless<StarlarkInt> {
6767
@Override
6868
public StarlarkInt convert(String input) throws OptionsParsingException {
6969
// Note that Starlark rule attribute values are currently restricted
@@ -85,8 +85,8 @@ public String getTypeDescription() {
8585
/** A converter from strings to Labels. */
8686
public static class LabelConverter implements Converter<Label> {
8787
@Override
88-
public Label convert(String input) throws OptionsParsingException {
89-
return convertOptionsLabel(input);
88+
public Label convert(String input, Object conversionContext) throws OptionsParsingException {
89+
return convertOptionsLabel(input, conversionContext);
9090
}
9191

9292
@Override
@@ -98,10 +98,11 @@ public String getTypeDescription() {
9898
/** A converter from comma-separated strings to Label lists. */
9999
public static class LabelListConverter implements Converter<List<Label>> {
100100
@Override
101-
public List<Label> convert(String input) throws OptionsParsingException {
101+
public List<Label> convert(String input, Object conversionContext)
102+
throws OptionsParsingException {
102103
ImmutableList.Builder<Label> result = ImmutableList.builder();
103104
for (String label : Splitter.on(",").omitEmptyStrings().split(input)) {
104-
result.add(convertOptionsLabel(label));
105+
result.add(convertOptionsLabel(label, conversionContext));
105106
}
106107
return result.build();
107108
}
@@ -119,8 +120,8 @@ public String getTypeDescription() {
119120
public static class EmptyToNullLabelConverter implements Converter<Label> {
120121
@Override
121122
@Nullable
122-
public Label convert(String input) throws OptionsParsingException {
123-
return input.isEmpty() ? null : convertOptionsLabel(input);
123+
public Label convert(String input, Object conversionContext) throws OptionsParsingException {
124+
return input.isEmpty() ? null : convertOptionsLabel(input, conversionContext);
124125
}
125126

126127
@Override
@@ -139,8 +140,8 @@ protected DefaultLabelConverter(String defaultValue) {
139140
}
140141

141142
@Override
142-
public Label convert(String input) throws OptionsParsingException {
143-
return input.isEmpty() ? defaultValue : convertOptionsLabel(input);
143+
public Label convert(String input, Object conversionContext) throws OptionsParsingException {
144+
return input.isEmpty() ? defaultValue : convertOptionsLabel(input, conversionContext);
144145
}
145146

146147
@Override
@@ -152,7 +153,8 @@ public String getTypeDescription() {
152153
/** Flag converter for a map of unique keys with optional labels as values. */
153154
public static class LabelMapConverter implements Converter<Map<String, Label>> {
154155
@Override
155-
public Map<String, Label> convert(String input) throws OptionsParsingException {
156+
public Map<String, Label> convert(String input, Object conversionContext)
157+
throws OptionsParsingException {
156158
// Use LinkedHashMap so we can report duplicate keys more easily while preserving order
157159
Map<String, Label> result = new LinkedHashMap<>();
158160
for (String entry : Splitter.on(",").omitEmptyStrings().trimResults().split(input)) {
@@ -165,7 +167,7 @@ public Map<String, Label> convert(String input) throws OptionsParsingException {
165167
} else {
166168
key = entry.substring(0, sepIndex);
167169
String value = entry.substring(sepIndex + 1);
168-
label = value.isEmpty() ? null : convertOptionsLabel(value);
170+
label = value.isEmpty() ? null : convertOptionsLabel(value, conversionContext);
169171
}
170172
if (result.containsKey(key)) {
171173
throw new OptionsParsingException("Key '" + key + "' appears twice");
@@ -202,7 +204,8 @@ public StrictDepsConverter() {
202204
}
203205
}
204206

205-
private static final Label convertOptionsLabel(String input) throws OptionsParsingException {
207+
private static Label convertOptionsLabel(String input, @Nullable Object conversionContext)
208+
throws OptionsParsingException {
206209
try {
207210
// Check if the input starts with '/'. We don't check for "//" so that
208211
// we get a better error message if the user accidentally tries to use

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ public class CoreOptions extends FragmentOptions implements Cloneable {
254254
public boolean enableAspectHints;
255255

256256
/** Regardless of input, converts to an empty list. For use with affectedByStarlarkTransition */
257-
public static class EmptyListConverter implements Converter<List<String>> {
257+
public static class EmptyListConverter extends Converter.Contextless<List<String>> {
258258
@Override
259259
public List<String> convert(String input) throws OptionsParsingException {
260260
return ImmutableList.of();

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ Pattern pattern() {
5858

5959
/** Constructs an instance of ExecutionInfoModifier by parsing an option string. */
6060
public static class Converter
61-
implements com.google.devtools.common.options.Converter<ExecutionInfoModifier> {
61+
extends com.google.devtools.common.options.Converter.Contextless<ExecutionInfoModifier> {
6262
@Override
6363
public ExecutionInfoModifier convert(String input) throws OptionsParsingException {
6464
if (Strings.isNullOrEmpty(input)) {
@@ -77,7 +77,7 @@ public ExecutionInfoModifier convert(String input) throws OptionsParsingExceptio
7777
// Convert to get a useful exception if it's not a valid pattern, but use the regex
7878
// (see comment in Expression)
7979
new RegexPatternConverter()
80-
.convert(specMatcher.group("pattern"))
80+
.convert(specMatcher.group("pattern"), /*conversionContext=*/ null)
8181
.regexPattern()
8282
.pattern(),
8383
specMatcher.group("sign").equals("-"),

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,13 @@ public final class PerLabelOptions {
3737
private final List<String> optionsList;
3838

3939
/**
40-
* Converts a String to a {@link PerLabelOptions} object. The syntax of the
41-
* string is {@code regex_filter@option_1,option_2,...,option_n}. Where
42-
* regex_filter stands for the String representation of a {@link RegexFilter},
43-
* and {@code option_1} to {@code option_n} stand for arbitrary command line
44-
* options. If an option contains a comma it has to be quoted with a
45-
* backslash. Options can contain @. Only the first @ is used to split the
46-
* string.
40+
* Converts a String to a {@link PerLabelOptions} object. The syntax of the string is {@code
41+
* regex_filter@option_1,option_2,...,option_n}. Where regex_filter stands for the String
42+
* representation of a {@link RegexFilter}, and {@code option_1} to {@code option_n} stand for
43+
* arbitrary command line options. If an option contains a comma it has to be quoted with a
44+
* backslash. Options can contain @. Only the first @ is used to split the string.
4745
*/
48-
public static class PerLabelOptionsConverter implements Converter<PerLabelOptions> {
46+
public static class PerLabelOptionsConverter extends Converter.Contextless<PerLabelOptions> {
4947

5048
@Override
5149
public PerLabelOptions convert(String input) throws OptionsParsingException {

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,11 @@
2626
import java.util.Objects;
2727
import javax.annotation.Nullable;
2828

29-
/**
30-
* --run_under options converter.
31-
*/
29+
/** --run_under options converter. */
3230
public class RunUnderConverter implements Converter<RunUnder> {
3331
@Override
34-
public RunUnder convert(final String input) throws OptionsParsingException {
32+
public RunUnder convert(final String input, Object conversionContext)
33+
throws OptionsParsingException {
3534
final List<String> runUnderList = new ArrayList<>();
3635
try {
3736
ShellUtils.tokenize(runUnderList, input);

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,12 +308,13 @@ private static BuildOptions applyTransition(
308308
// of returning either a scalar or list.
309309
List<?> optionValueAsList = (List<?>) optionValue;
310310
if (optionValueAsList.isEmpty()) {
311-
convertedValue = def.getDefaultValue();
311+
convertedValue = ImmutableList.of();
312312
} else {
313313
convertedValue =
314314
def.getConverter()
315315
.convert(
316-
optionValueAsList.stream().map(Object::toString).collect(joining(",")));
316+
optionValueAsList.stream().map(Object::toString).collect(joining(",")),
317+
/*conversionContext=*/ null);
317318
}
318319
} else if (def.getType() == List.class && optionValue == null) {
319320
throw ValidationException.format(
@@ -325,7 +326,8 @@ private static BuildOptions applyTransition(
325326
} else if (def.getType().equals(boolean.class) && optionValue instanceof Boolean) {
326327
convertedValue = optionValue;
327328
} else if (optionValue instanceof String) {
328-
convertedValue = def.getConverter().convert((String) optionValue);
329+
convertedValue =
330+
def.getConverter().convert((String) optionValue, /*conversionContext=*/ null);
329331
} else {
330332
throw ValidationException.format("Invalid value type for option '%s'", optionName);
331333
}

0 commit comments

Comments
 (0)