Skip to content

Commit 0eb1bb4

Browse files
committed
Support all-supported pseudo command in rc files
Options specified on the `all-supported` command in rc files are applied to all commands that support them. If an option isn't supported by the current command but by some other Bazel command, it is ignored. If it isn't supported by any Bazel command, the usual error message is shown.
1 parent 91584dd commit 0eb1bb4

File tree

10 files changed

+264
-116
lines changed

10 files changed

+264
-116
lines changed

src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.runtime;
1515

16+
import static com.google.common.collect.ImmutableList.toImmutableList;
17+
1618
import com.google.common.annotations.VisibleForTesting;
1719
import com.google.common.base.Joiner;
1820
import com.google.common.collect.ArrayListMultimap;
@@ -33,8 +35,10 @@
3335
import com.google.devtools.build.lib.vfs.Path;
3436
import com.google.devtools.common.options.Converters;
3537
import com.google.devtools.common.options.InvocationPolicyEnforcer;
38+
import com.google.devtools.common.options.OpaqueOptionsData;
3639
import com.google.devtools.common.options.OptionDefinition;
3740
import com.google.devtools.common.options.OptionPriority.PriorityCategory;
41+
import com.google.devtools.common.options.OptionsBase;
3842
import com.google.devtools.common.options.OptionsParser;
3943
import com.google.devtools.common.options.OptionsParsingException;
4044
import com.google.devtools.common.options.OptionsParsingResult;
@@ -66,6 +70,8 @@ public final class BlazeOptionHandler {
6670
"client_env",
6771
"client_cwd");
6872

73+
private static final String ALL_SUPPORTED_PSEUDO_COMMAND = "all-supported";
74+
6975
// Marks an event to indicate a parsing error.
7076
static final String BAD_OPTION_TAG = "invalidOption";
7177
// Separates the invalid tag from the full error message for easier parsing.
@@ -78,6 +84,7 @@ public final class BlazeOptionHandler {
7884
private final Command commandAnnotation;
7985
private final InvocationPolicy invocationPolicy;
8086
private final List<String> rcfileNotes = new ArrayList<>();
87+
private final List<Class<? extends OptionsBase>> allOptionsClasses;
8188

8289
BlazeOptionHandler(
8390
BlazeRuntime runtime,
@@ -92,6 +99,11 @@ public final class BlazeOptionHandler {
9299
this.commandAnnotation = commandAnnotation;
93100
this.optionsParser = optionsParser;
94101
this.invocationPolicy = invocationPolicy;
102+
allOptionsClasses = runtime.getCommandMap().values().stream()
103+
.map(BlazeCommand::getClass)
104+
.flatMap(cmd -> BlazeCommandUtils.getOptions(cmd, runtime.getBlazeModules(),
105+
runtime.getRuleClassProvider()).stream())
106+
.collect(toImmutableList());
95107
}
96108

97109
/**
@@ -191,7 +203,14 @@ void parseRcOptions(
191203
"%s:\n %s'%s' options: %s",
192204
source, inherited, commandToParse, Joiner.on(' ').join(rcArgs.getArgs())));
193205
}
194-
optionsParser.parse(PriorityCategory.RC_FILE, rcArgs.getRcFile(), rcArgs.getArgs());
206+
if (commandToParse.equals(ALL_SUPPORTED_PSEUDO_COMMAND)) {
207+
// Pass in options data for all commands supported by the runtime so that options that
208+
// apply to some but not the current command can be ignored.
209+
optionsParser.parseWithSourceFunction(PriorityCategory.RC_FILE, o -> rcArgs.getRcFile(),
210+
rcArgs.getArgs(), OptionsParser.getFallbackOptionsData(allOptionsClasses));
211+
} else {
212+
optionsParser.parse(PriorityCategory.RC_FILE, rcArgs.getRcFile(), rcArgs.getArgs());
213+
}
195214
}
196215
}
197216
}
@@ -227,7 +246,8 @@ private void parseArgsAndConfigs(List<String> args, ExtendedEventHandler eventHa
227246
optionsParser.parseWithSourceFunction(
228247
PriorityCategory.COMMAND_LINE,
229248
commandOptionSourceFunction,
230-
defaultOverridesAndRcSources.build());
249+
defaultOverridesAndRcSources.build(),
250+
null);
231251

232252
// Command-specific options from .blazerc passed in via --default_override and --rc_source.
233253
ClientOptions rcFileOptions = optionsParser.getOptions(ClientOptions.class);
@@ -241,7 +261,7 @@ private void parseArgsAndConfigs(List<String> args, ExtendedEventHandler eventHa
241261

242262
// Parses the remaining command-line options.
243263
optionsParser.parseWithSourceFunction(
244-
PriorityCategory.COMMAND_LINE, commandOptionSourceFunction, remainingCmdLine.build());
264+
PriorityCategory.COMMAND_LINE, commandOptionSourceFunction, remainingCmdLine.build(), null);
245265

246266
if (commandAnnotation.builds()) {
247267
// splits project files from targets in the traditional sense
@@ -374,12 +394,14 @@ void expandConfigOptions(
374394
commandToRcArgs,
375395
getCommandNamesToParse(commandAnnotation),
376396
rcfileNotes::add,
377-
optionsParser);
397+
optionsParser,
398+
OptionsParser.getFallbackOptionsData(allOptionsClasses));
378399
}
379400

380401
private static List<String> getCommandNamesToParse(Command commandAnnotation) {
381402
List<String> result = new ArrayList<>();
382403
result.add("common");
404+
result.add(ALL_SUPPORTED_PSEUDO_COMMAND);
383405
getCommandNamesToParseHelper(commandAnnotation, result);
384406
return result;
385407
}
@@ -470,7 +492,8 @@ static ListMultimap<String, RcChunkOfArgs> structureRcOptionsAndConfigs(
470492
if (index > 0) {
471493
command = command.substring(0, index);
472494
}
473-
if (!validCommands.contains(command) && !command.equals("common")) {
495+
if (!validCommands.contains(command) && !command.equals("common") && !command.equals(
496+
ALL_SUPPORTED_PSEUDO_COMMAND)) {
474497
eventHandler.handle(
475498
Event.warn(
476499
"while reading option defaults file '"

src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1113,7 +1113,7 @@ private static OptionsParsingResult parseStartupOptions(
11131113

11141114
// Then parse the command line again, this time with the correct option sources
11151115
parser = OptionsParser.builder().optionsClasses(optionClasses).allowResidue(false).build();
1116-
parser.parseWithSourceFunction(PriorityCategory.COMMAND_LINE, sourceFunction, args);
1116+
parser.parseWithSourceFunction(PriorityCategory.COMMAND_LINE, sourceFunction, args, null);
11171117
return parser;
11181118
}
11191119

src/main/java/com/google/devtools/build/lib/runtime/ConfigExpander.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.google.devtools.build.lib.events.Event;
2020
import com.google.devtools.build.lib.events.EventHandler;
2121
import com.google.devtools.build.lib.util.OS;
22+
import com.google.devtools.common.options.OpaqueOptionsData;
2223
import com.google.devtools.common.options.OptionValueDescription;
2324
import com.google.devtools.common.options.OptionsParser;
2425
import com.google.devtools.common.options.OptionsParsingException;
@@ -88,7 +89,8 @@ static void expandConfigOptions(
8889
ListMultimap<String, RcChunkOfArgs> commandToRcArgs,
8990
List<String> commandsToParse,
9091
Consumer<String> rcFileNotesConsumer,
91-
OptionsParser optionsParser)
92+
OptionsParser optionsParser,
93+
OpaqueOptionsData fallbackData)
9294
throws OptionsParsingException {
9395

9496
OptionValueDescription configValueDescription =
@@ -113,7 +115,8 @@ static void expandConfigOptions(
113115
optionsParser.parseArgsAsExpansionOfOption(
114116
configInstance,
115117
String.format("expanded from --config=%s", configValueToExpand),
116-
expansion);
118+
expansion,
119+
fallbackData);
117120
}
118121
}
119122

@@ -131,7 +134,8 @@ static void expandConfigOptions(
131134
optionsParser.parseArgsAsExpansionOfOption(
132135
Iterables.getOnlyElement(enablePlatformSpecificConfigDescription.getCanonicalInstances()),
133136
String.format("enabled by --enable_platform_specific_config"),
134-
expansion);
137+
expansion,
138+
fallbackData);
135139
}
136140

137141
// At this point, we've expanded everything, identify duplicates, if any, to warn about

src/main/java/com/google/devtools/common/options/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ java_library(
4949
],
5050
),
5151
deps = [
52+
"//src/main/java/com/google/devtools/build/lib/util:pair",
5253
"//third_party:auto_value",
5354
"//third_party:caffeine",
5455
"//third_party:guava",

src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -83,22 +83,22 @@ public static ImmutableList<OptionDefinition> getAllOptionDefinitionsForClass(
8383

8484
/**
8585
* Mapping from each options class to its no-arg constructor. Entries appear in the same order
86-
* that they were passed to {@link #from(Collection)}.
86+
* that they were passed to {@link #from(Collection, boolean)}.
8787
*/
8888
private final ImmutableMap<Class<? extends OptionsBase>, Constructor<?>> optionsClasses;
8989

9090
/**
9191
* Mapping from option name to {@code OptionDefinition}. Entries appear ordered first by their
92-
* options class (the order in which they were passed to {@link #from(Collection)}, and then in
93-
* alphabetic order within each options class.
92+
* options class (the order in which they were passed to {@link #from(Collection, boolean)}, and
93+
* then in alphabetic order within each options class.
9494
*/
9595
private final ImmutableMap<String, OptionDefinition> nameToField;
9696

9797
/**
98-
* For options that have an "OldName", this is a mapping from old name to its corresponding {@code
99-
* OptionDefinition}. Entries appear ordered first by their options class (the order in which they
100-
* were passed to {@link #from(Collection)}, and then in alphabetic order within each options
101-
* class.
98+
* For options that have an "OldName", this is a mapping from old name to its corresponding
99+
* {@code OptionDefinition}. Entries appear ordered first by their options class (the order in
100+
* which they were passed to {@link #from(Collection, boolean)}, and then in alphabetic order
101+
* within each options class.
102102
*/
103103
private final ImmutableMap<String, OptionDefinition> oldNameToField;
104104

@@ -136,7 +136,7 @@ protected IsolatedOptionsData(IsolatedOptionsData other) {
136136

137137
/**
138138
* Returns all options classes indexed by this options data object, in the order they were passed
139-
* to {@link #from(Collection)}.
139+
* to {@link #from(Collection, boolean)}.
140140
*/
141141
public Collection<Class<? extends OptionsBase>> getOptionsClasses() {
142142
return optionsClasses.keySet();
@@ -157,8 +157,8 @@ public OptionDefinition getOptionDefinitionFromName(String name) {
157157

158158
/**
159159
* Returns all {@link OptionDefinition} objects loaded, mapped by their canonical names. Entries
160-
* appear ordered first by their options class (the order in which they were passed to {@link
161-
* #from(Collection)}, and then in alphabetic order within each options class.
160+
* appear ordered first by their options class (the order in which they were passed to
161+
* {@link #from(Collection, boolean)}, and then in alphabetic order within each options class.
162162
*/
163163
public Iterable<Map.Entry<String, OptionDefinition>> getAllOptionDefinitions() {
164164
return nameToField.entrySet();
@@ -177,9 +177,15 @@ public boolean getUsesOnlyCoreTypes(Class<? extends OptionsBase> optionsClass) {
177177
* both single-character abbreviations and full names.
178178
*/
179179
private static <A> void checkForCollisions(
180-
Map<A, OptionDefinition> aFieldMap, A optionName, String description)
180+
Map<A, OptionDefinition> aFieldMap, A optionName, OptionDefinition definition,
181+
String description, boolean allowDuplicatesParsingEquivalently)
181182
throws DuplicateOptionDeclarationException {
182183
if (aFieldMap.containsKey(optionName)) {
184+
OptionDefinition otherDefinition = aFieldMap.get(optionName);
185+
if (allowDuplicatesParsingEquivalently && OptionsParserImpl.equivalentForParsing(
186+
otherDefinition, definition)) {
187+
return;
188+
}
183189
throw new DuplicateOptionDeclarationException(
184190
"Duplicate option name, due to " + description + ": --" + optionName);
185191
}
@@ -212,22 +218,27 @@ private static void checkAndUpdateBooleanAliases(
212218
Map<String, OptionDefinition> nameToFieldMap,
213219
Map<String, OptionDefinition> oldNameToFieldMap,
214220
Map<String, String> booleanAliasMap,
215-
String optionName)
221+
String optionName,
222+
OptionDefinition optionDefinition,
223+
boolean allowDuplicatesParsingEquivalently)
216224
throws DuplicateOptionDeclarationException {
217225
// Check that the negating alias does not conflict with existing flags.
218-
checkForCollisions(nameToFieldMap, "no" + optionName, "boolean option alias");
219-
checkForCollisions(oldNameToFieldMap, "no" + optionName, "boolean option alias");
226+
checkForCollisions(nameToFieldMap, "no" + optionName, optionDefinition, "boolean option alias",
227+
allowDuplicatesParsingEquivalently);
228+
checkForCollisions(oldNameToFieldMap, "no" + optionName, optionDefinition,
229+
"boolean option alias", allowDuplicatesParsingEquivalently);
220230

221231
// Record that the boolean option takes up additional namespace for its negating alias.
222232
booleanAliasMap.put("no" + optionName, optionName);
223233
}
224234

225235
/**
226-
* Constructs an {@link IsolatedOptionsData} object for a parser that knows about the given {@link
227-
* OptionsBase} classes. No inter-option analysis is done. Performs basic validity checks on each
228-
* option in isolation.
236+
* Constructs an {@link IsolatedOptionsData} object for a parser that knows about the given
237+
* {@link OptionsBase} classes. No inter-option analysis is done. Performs basic validity checks
238+
* on each option in isolation.
229239
*/
230-
static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes) {
240+
static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes,
241+
boolean allowDuplicatesParsingEquivalently) {
231242
// Mind which fields have to preserve order.
232243
Map<Class<? extends OptionsBase>, Constructor<?>> constructorBuilder = new LinkedHashMap<>();
233244
Map<String, OptionDefinition> nameToFieldBuilder = new LinkedHashMap<>();
@@ -256,15 +267,18 @@ static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes
256267
for (OptionDefinition optionDefinition : optionDefinitions) {
257268
try {
258269
String optionName = optionDefinition.getOptionName();
259-
checkForCollisions(nameToFieldBuilder, optionName, "option name collision");
270+
checkForCollisions(nameToFieldBuilder, optionName, optionDefinition,
271+
"option name collision", allowDuplicatesParsingEquivalently);
260272
checkForCollisions(
261273
oldNameToFieldBuilder,
262274
optionName,
263-
"option name collision with another option's old name");
275+
optionDefinition,
276+
"option name collision with another option's old name",
277+
allowDuplicatesParsingEquivalently);
264278
checkForBooleanAliasCollisions(booleanAliasMap, optionName, "option");
265279
if (optionDefinition.usesBooleanValueSyntax()) {
266-
checkAndUpdateBooleanAliases(
267-
nameToFieldBuilder, oldNameToFieldBuilder, booleanAliasMap, optionName);
280+
checkAndUpdateBooleanAliases(nameToFieldBuilder, oldNameToFieldBuilder, booleanAliasMap,
281+
optionName, optionDefinition, allowDuplicatesParsingEquivalently);
268282
}
269283
nameToFieldBuilder.put(optionName, optionDefinition);
270284

@@ -273,23 +287,27 @@ static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes
273287
checkForCollisions(
274288
nameToFieldBuilder,
275289
oldName,
276-
"old option name collision with another option's canonical name");
290+
optionDefinition,
291+
"old option name collision with another option's canonical name",
292+
allowDuplicatesParsingEquivalently);
277293
checkForCollisions(
278294
oldNameToFieldBuilder,
279295
oldName,
280-
"old option name collision with another old option name");
296+
optionDefinition,
297+
"old option name collision with another old option name",
298+
allowDuplicatesParsingEquivalently);
281299
checkForBooleanAliasCollisions(booleanAliasMap, oldName, "old option name");
282300
// If boolean, repeat the alias dance for the old name.
283301
if (optionDefinition.usesBooleanValueSyntax()) {
284-
checkAndUpdateBooleanAliases(
285-
nameToFieldBuilder, oldNameToFieldBuilder, booleanAliasMap, oldName);
302+
checkAndUpdateBooleanAliases(nameToFieldBuilder, oldNameToFieldBuilder,
303+
booleanAliasMap, oldName, optionDefinition, allowDuplicatesParsingEquivalently);
286304
}
287305
// Now that we've checked for conflicts, confidently store the old name.
288306
oldNameToFieldBuilder.put(oldName, optionDefinition);
289307
}
290308
if (optionDefinition.getAbbreviation() != '\0') {
291-
checkForCollisions(
292-
abbrevToFieldBuilder, optionDefinition.getAbbreviation(), "option abbreviation");
309+
checkForCollisions(abbrevToFieldBuilder, optionDefinition.getAbbreviation(),
310+
optionDefinition, "option abbreviation", allowDuplicatesParsingEquivalently);
293311
abbrevToFieldBuilder.put(optionDefinition.getAbbreviation(), optionDefinition);
294312
}
295313
} catch (DuplicateOptionDeclarationException e) {

src/main/java/com/google/devtools/common/options/OptionsData.java

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,26 @@
2828
@Immutable
2929
final class OptionsData extends IsolatedOptionsData {
3030

31-
/** Mapping from each option to the (unparsed) options it expands to, if any. */
31+
/**
32+
* Mapping from each option to the (unparsed) options it expands to, if any.
33+
*/
3234
private final ImmutableMap<OptionDefinition, ImmutableList<String>> evaluatedExpansions;
3335

34-
/** Construct {@link OptionsData} by extending an {@link IsolatedOptionsData} with new info. */
36+
/**
37+
* Whether this options data has been created with duplicate options definitions allowed as long
38+
* as those options are parsed (but not necessarily evaluated) equivalently.
39+
*/
40+
private final boolean allowDuplicatesParsingEquivalently;
41+
42+
/**
43+
* Construct {@link OptionsData} by extending an {@link IsolatedOptionsData} with new info.
44+
*/
3545
private OptionsData(
36-
IsolatedOptionsData base, Map<OptionDefinition, ImmutableList<String>> evaluatedExpansions) {
46+
IsolatedOptionsData base, Map<OptionDefinition, ImmutableList<String>> evaluatedExpansions,
47+
boolean allowDuplicatesParsingEquivalently) {
3748
super(base);
3849
this.evaluatedExpansions = ImmutableMap.copyOf(evaluatedExpansions);
50+
this.allowDuplicatesParsingEquivalently = allowDuplicatesParsingEquivalently;
3951
}
4052

4153
private static final ImmutableList<String> EMPTY_EXPANSION = ImmutableList.<String>of();
@@ -50,13 +62,23 @@ public ImmutableList<String> getEvaluatedExpansion(OptionDefinition optionDefini
5062
}
5163

5264
/**
53-
* Constructs an {@link OptionsData} object for a parser that knows about the given {@link
54-
* OptionsBase} classes. In addition to the work done to construct the {@link
55-
* IsolatedOptionsData}, this also computes expansion information. If an option has an expansion,
56-
* try to precalculate its here.
65+
* Returns whether this options data has been created with duplicate options definitions allowed
66+
* as long as those options are parsed (but not necessarily evaluated) equivalently.
67+
*/
68+
public boolean createdWithAllowDuplicatesParsingEquivalently() {
69+
return allowDuplicatesParsingEquivalently;
70+
}
71+
72+
/**
73+
* Constructs an {@link OptionsData} object for a parser that knows about the given
74+
* {@link OptionsBase} classes. In addition to the work done to construct the
75+
* {@link IsolatedOptionsData}, this also computes expansion information. If an option has an
76+
* expansion, try to precalculate its here.
5777
*/
58-
static OptionsData from(Collection<Class<? extends OptionsBase>> classes) {
59-
IsolatedOptionsData isolatedData = IsolatedOptionsData.from(classes);
78+
static OptionsData from(Collection<Class<? extends OptionsBase>> classes,
79+
boolean allowDuplicatesParsingEquivalently) {
80+
IsolatedOptionsData isolatedData = IsolatedOptionsData.from(classes,
81+
allowDuplicatesParsingEquivalently);
6082

6183
// All that's left is to compute expansions.
6284
ImmutableMap.Builder<OptionDefinition, ImmutableList<String>> evaluatedExpansionsBuilder =
@@ -68,6 +90,7 @@ static OptionsData from(Collection<Class<? extends OptionsBase>> classes) {
6890
evaluatedExpansionsBuilder.put(optionDefinition, ImmutableList.copyOf(constExpansion));
6991
}
7092
}
71-
return new OptionsData(isolatedData, evaluatedExpansionsBuilder.build());
93+
return new OptionsData(isolatedData, evaluatedExpansionsBuilder.build(),
94+
allowDuplicatesParsingEquivalently);
7295
}
7396
}

0 commit comments

Comments
 (0)