Skip to content

Commit 883ce21

Browse files
Wyveraldcopybara-github
authored andcommitted
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

File tree

69 files changed

+532
-325
lines changed

Some content is hidden

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

69 files changed

+532
-325
lines changed

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
}

src/main/java/com/google/devtools/build/lib/bazel/commands/ModqueryOptions.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ static TargetModule create(String name, Version version) {
218218
}
219219

220220
/** Converts a module target argument string to a properly typed {@link TargetModule} */
221-
static class TargetModuleConverter implements Converter<TargetModule> {
221+
static class TargetModuleConverter extends Converter.Contextless<TargetModule> {
222222

223223
@Override
224224
public TargetModule convert(String input) throws OptionsParsingException {
@@ -260,17 +260,19 @@ public String getTypeDescription() {
260260
}
261261

262262
/** Converts a comma-separated module list argument (i.e. [email protected],[email protected]) */
263-
public static class TargetModuleListConverter implements Converter<ImmutableList<TargetModule>> {
263+
public static class TargetModuleListConverter
264+
extends Converter.Contextless<ImmutableList<TargetModule>> {
264265

265266
@Override
266267
public ImmutableList<TargetModule> convert(String input) throws OptionsParsingException {
267268
CommaSeparatedNonEmptyOptionListConverter listConverter =
268269
new CommaSeparatedNonEmptyOptionListConverter();
269270
TargetModuleConverter targetModuleConverter = new TargetModuleConverter();
270-
ImmutableList<String> targetStrings = listConverter.convert(input);
271+
ImmutableList<String> targetStrings =
272+
listConverter.convert(input, /*conversionContext=*/ null);
271273
ImmutableList.Builder<TargetModule> targetModules = new ImmutableList.Builder<>();
272274
for (String targetInput : targetStrings) {
273-
targetModules.add(targetModuleConverter.convert(targetInput));
275+
targetModules.add(targetModuleConverter.convert(targetInput, /*conversionContext=*/ null));
274276
}
275277
return targetModules.build();
276278
}

src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,8 @@ public Converter() {
257257
/**
258258
* Converts from an equals-separated pair of strings into RepositoryName->PathFragment mapping.
259259
*/
260-
public static class RepositoryOverrideConverter implements Converter<RepositoryOverride> {
260+
public static class RepositoryOverrideConverter
261+
extends Converter.Contextless<RepositoryOverride> {
261262

262263
@Override
263264
public RepositoryOverride convert(String input) throws OptionsParsingException {

src/main/java/com/google/devtools/build/lib/packages/EnumFilterConverter.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,16 @@
2222
import java.util.Set;
2323

2424
/**
25-
* Converter that translates a string of the form "value1,value2,-value3,value4"
26-
* into a corresponding set of allowed Enum values.
25+
* Converter that translates a string of the form "value1,value2,-value3,value4" into a
26+
* corresponding set of allowed Enum values.
2727
*
28-
* <p>Values preceded by '-' are excluded from this set. So "value1,-value2,value3"
29-
* translates to the set [EnumType.value1, EnumType.value3].
28+
* <p>Values preceded by '-' are excluded from this set. So "value1,-value2,value3" translates to
29+
* the set [EnumType.value1, EnumType.value3].
3030
*
31-
* <p>If *all* values are exclusions (e.g. "-value1,-value2,-value3"), the returned
32-
* set contains all values for the Enum type *except* those specified.
31+
* <p>If *all* values are exclusions (e.g. "-value1,-value2,-value3"), the returned set contains all
32+
* values for the Enum type *except* those specified.
3333
*/
34-
class EnumFilterConverter<E extends Enum<E>> implements Converter<Set<E>> {
34+
class EnumFilterConverter<E extends Enum<E>> extends Converter.Contextless<Set<E>> {
3535

3636
private final Set<String> allowedValues = new LinkedHashSet<>();
3737
private final Class<E> typeClass;
@@ -54,7 +54,7 @@ class EnumFilterConverter<E extends Enum<E>> implements Converter<Set<E>> {
5454
/**
5555
* Returns the set of allowed values for the option.
5656
*
57-
* Implements {@link #convert(String)}.
57+
* <p>Implements {@link Converter#convert(String, Object)}.
5858
*/
5959
@Override
6060
public Set<E> convert(String input) throws OptionsParsingException {

src/main/java/com/google/devtools/build/lib/packages/TestSize.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,9 @@ public TestSizeFilterConverter() {
121121
/**
122122
* {@inheritDoc}
123123
*
124-
* <p>This override is necessary to prevent OptionsData
125-
* from throwing a "must be assignable from the converter return type" exception.
126-
* OptionsData doesn't recognize the generic type and actual type are the same.
124+
* <p>This override is necessary to prevent OptionsData from throwing a "must be assignable from
125+
* the converter return type" exception. OptionsData doesn't recognize the generic type and
126+
* actual type are the same.
127127
*/
128128
@Override
129129
public final Set<TestSize> convert(String input) throws OptionsParsingException {

src/main/java/com/google/devtools/build/lib/packages/TestTimeout.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -190,10 +190,9 @@ public static TestTimeout getSuggestedTestTimeout(int timeInSeconds) {
190190
return SUGGESTED_TIMEOUT.get(timeInSeconds);
191191
}
192192

193-
/**
194-
* Converter for the --test_timeout option.
195-
*/
196-
public static class TestTimeoutConverter implements Converter<Map<TestTimeout, Duration>> {
193+
/** Converter for the --test_timeout option. */
194+
public static class TestTimeoutConverter
195+
extends Converter.Contextless<Map<TestTimeout, Duration>> {
197196
public TestTimeoutConverter() {}
198197

199198
@Override
@@ -250,9 +249,9 @@ public TestTimeoutFilterConverter() {
250249
/**
251250
* {@inheritDoc}
252251
*
253-
* <p>This override is necessary to prevent OptionsData
254-
* from throwing a "must be assignable from the converter return type" exception.
255-
* OptionsData doesn't recognize the generic type and actual type are the same.
252+
* <p>This override is necessary to prevent OptionsData from throwing a "must be assignable from
253+
* the converter return type" exception. OptionsData doesn't recognize the generic type and
254+
* actual type are the same.
256255
*/
257256
@Override
258257
public final Set<TestTimeout> convert(String input) throws OptionsParsingException {

src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,8 @@
3838
/** Options for configuring Packages -- loading and default behaviors. */
3939
public class PackageOptions extends OptionsBase {
4040

41-
/**
42-
* Converter for the {@code --default_visibility} option.
43-
*/
44-
public static class DefaultVisibilityConverter implements Converter<RuleVisibility> {
41+
/** Converter for the {@code --default_visibility} option. */
42+
public static class DefaultVisibilityConverter extends Converter.Contextless<RuleVisibility> {
4543
@Override
4644
public RuleVisibility convert(String input) throws OptionsParsingException {
4745
if (input.equals("public")) {
@@ -194,11 +192,9 @@ public ParallelismConverter() throws OptionsParsingException {
194192
+ "previous run's cache.")
195193
public boolean checkOutputFiles;
196194

197-
/**
198-
* A converter from strings containing comma-separated names of packages to lists of strings.
199-
*/
195+
/** A converter from strings containing comma-separated names of packages to lists of strings. */
200196
public static class CommaSeparatedPackageNameListConverter
201-
implements Converter<List<PackageIdentifier>> {
197+
extends Converter.Contextless<List<PackageIdentifier>> {
202198

203199
private static final Splitter COMMA_SPLITTER = Splitter.on(',');
204200

src/main/java/com/google/devtools/build/lib/profiler/MemoryProfiler.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,8 @@ private MemoryProfileStableHeapParameters(int numTimesToDoGc, Duration timeToSle
140140

141141
/** Converter for {@code MemoryProfileStableHeapParameters} option. */
142142
public static class Converter
143-
implements com.google.devtools.common.options.Converter<MemoryProfileStableHeapParameters> {
143+
extends com.google.devtools.common.options.Converter.Contextless<
144+
MemoryProfileStableHeapParameters> {
144145
private static final Splitter SPLITTER = Splitter.on(',');
145146

146147
@Override

src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,15 +218,15 @@ public final class RemoteOptions extends OptionsBase {
218218
public String remoteBytestreamUriPrefix;
219219

220220
/** Returns the specified duration. Assumes seconds if unitless. */
221-
public static class RemoteTimeoutConverter implements Converter<Duration> {
221+
public static class RemoteTimeoutConverter extends Converter.Contextless<Duration> {
222222
private static final Pattern UNITLESS_REGEX = Pattern.compile("^[0-9]+$");
223223

224224
@Override
225225
public Duration convert(String input) throws OptionsParsingException {
226226
if (UNITLESS_REGEX.matcher(input).matches()) {
227227
input += "s";
228228
}
229-
return new Converters.DurationConverter().convert(input);
229+
return new Converters.DurationConverter().convert(input, /*conversionContext=*/ null);
230230
}
231231

232232
@Override

0 commit comments

Comments
 (0)