Skip to content

Commit ce78cf5

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 a0cd355 commit ce78cf5

File tree

14 files changed

+554
-204
lines changed

14 files changed

+554
-204
lines changed

site/en/run/bazelrc.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,13 @@ line specifies when these defaults are applied:
105105
106106
- `startup`: startup options, which go before the command, and are described
107107
in `bazel help startup_options`.
108-
- `common`: options that apply to all Bazel commands.
108+
- `common`: options that apply to all Bazel commands. If a command does not
109+
support an option specified in this way, it will fail.
110+
- `all-supported`: options that should be applied to all Bazel commands that
111+
support them. If a command does not support an option specified in this way,
112+
the option will be ignored. Note that this only applies to option names: If
113+
the current command accepts an option with the specified name, but doesn't
114+
support the specified value, it will fail.
109115
- _`command`_: Bazel command, such as `build` or `query` to which the options
110116
apply. These options also apply to all commands that inherit from the
111117
specified command. (For example, `test` inherits from `build`.)

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

Lines changed: 37 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,12 @@ public final class BlazeOptionHandler {
9299
this.commandAnnotation = commandAnnotation;
93100
this.optionsParser = optionsParser;
94101
this.invocationPolicy = invocationPolicy;
102+
this.allOptionsClasses = runtime.getCommandMap().values().stream()
103+
.map(BlazeCommand::getClass)
104+
.flatMap(cmd -> BlazeCommandUtils.getOptions(cmd, runtime.getBlazeModules(),
105+
runtime.getRuleClassProvider()).stream())
106+
.distinct()
107+
.collect(toImmutableList());
95108
}
96109

97110
/**
@@ -191,7 +204,22 @@ void parseRcOptions(
191204
"%s:\n %s'%s' options: %s",
192205
source, inherited, commandToParse, Joiner.on(' ').join(rcArgs.getArgs())));
193206
}
194-
optionsParser.parse(PriorityCategory.RC_FILE, rcArgs.getRcFile(), rcArgs.getArgs());
207+
if (commandToParse.equals(ALL_SUPPORTED_PSEUDO_COMMAND)) {
208+
// Pass in options data for all commands supported by the runtime so that options that
209+
// apply to some but not the current command can be ignored.
210+
//
211+
// Important note: The consistency checks performed by
212+
// OptionsParser#getFallbackOptionsData ensure that there aren't any two options across
213+
// all commands that have the same name but parse differently (e.g. because one accepts
214+
// a value and the other doesn't). This means that the options available on a command
215+
// limit the options available on other commands even without command inheritance. This
216+
// restriction is necessary to ensure that the options specified on the "all-supported"
217+
// pseudo command can be parsed unambiguously.
218+
optionsParser.parseWithSourceFunction(PriorityCategory.RC_FILE, o -> rcArgs.getRcFile(),
219+
rcArgs.getArgs(), OptionsParser.getFallbackOptionsData(allOptionsClasses));
220+
} else {
221+
optionsParser.parse(PriorityCategory.RC_FILE, rcArgs.getRcFile(), rcArgs.getArgs());
222+
}
195223
}
196224
}
197225
}
@@ -227,7 +255,8 @@ private void parseArgsAndConfigs(List<String> args, ExtendedEventHandler eventHa
227255
optionsParser.parseWithSourceFunction(
228256
PriorityCategory.COMMAND_LINE,
229257
commandOptionSourceFunction,
230-
defaultOverridesAndRcSources.build());
258+
defaultOverridesAndRcSources.build(),
259+
null);
231260

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

242271
// Parses the remaining command-line options.
243272
optionsParser.parseWithSourceFunction(
244-
PriorityCategory.COMMAND_LINE, commandOptionSourceFunction, remainingCmdLine.build());
273+
PriorityCategory.COMMAND_LINE, commandOptionSourceFunction, remainingCmdLine.build(), null);
245274

246275
if (commandAnnotation.builds()) {
247276
// splits project files from targets in the traditional sense
@@ -374,12 +403,14 @@ void expandConfigOptions(
374403
commandToRcArgs,
375404
getCommandNamesToParse(commandAnnotation),
376405
rcfileNotes::add,
377-
optionsParser);
406+
optionsParser,
407+
OptionsParser.getFallbackOptionsData(allOptionsClasses));
378408
}
379409

380410
private static List<String> getCommandNamesToParse(Command commandAnnotation) {
381411
List<String> result = new ArrayList<>();
382412
result.add("common");
413+
result.add(ALL_SUPPORTED_PSEUDO_COMMAND);
383414
getCommandNamesToParseHelper(commandAnnotation, result);
384415
return result;
385416
}
@@ -470,7 +501,8 @@ static ListMultimap<String, RcChunkOfArgs> structureRcOptionsAndConfigs(
470501
if (index > 0) {
471502
command = command.substring(0, index);
472503
}
473-
if (!validCommands.contains(command) && !command.equals("common")) {
504+
if (!validCommands.contains(command) && !command.equals("common") && !command.equals(
505+
ALL_SUPPORTED_PSEUDO_COMMAND)) {
474506
eventHandler.handle(
475507
Event.warn(
476508
"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: 49 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,30 @@ 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.
239+
*
240+
* <p>If {@code allowDuplicatesParsingEquivalently} is true, then options that collide in name but
241+
* parse equivalently (e.g. both of them accept a value or both of them do not), are allowed.
229242
*/
230-
static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes) {
243+
static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes,
244+
boolean allowDuplicatesParsingEquivalently) {
231245
// Mind which fields have to preserve order.
232246
Map<Class<? extends OptionsBase>, Constructor<?>> constructorBuilder = new LinkedHashMap<>();
233247
Map<String, OptionDefinition> nameToFieldBuilder = new LinkedHashMap<>();
@@ -256,15 +270,18 @@ static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes
256270
for (OptionDefinition optionDefinition : optionDefinitions) {
257271
try {
258272
String optionName = optionDefinition.getOptionName();
259-
checkForCollisions(nameToFieldBuilder, optionName, "option name collision");
273+
checkForCollisions(nameToFieldBuilder, optionName, optionDefinition,
274+
"option name collision", allowDuplicatesParsingEquivalently);
260275
checkForCollisions(
261276
oldNameToFieldBuilder,
262277
optionName,
263-
"option name collision with another option's old name");
278+
optionDefinition,
279+
"option name collision with another option's old name",
280+
allowDuplicatesParsingEquivalently);
264281
checkForBooleanAliasCollisions(booleanAliasMap, optionName, "option");
265282
if (optionDefinition.usesBooleanValueSyntax()) {
266-
checkAndUpdateBooleanAliases(
267-
nameToFieldBuilder, oldNameToFieldBuilder, booleanAliasMap, optionName);
283+
checkAndUpdateBooleanAliases(nameToFieldBuilder, oldNameToFieldBuilder, booleanAliasMap,
284+
optionName, optionDefinition, allowDuplicatesParsingEquivalently);
268285
}
269286
nameToFieldBuilder.put(optionName, optionDefinition);
270287

@@ -273,23 +290,27 @@ static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes
273290
checkForCollisions(
274291
nameToFieldBuilder,
275292
oldName,
276-
"old option name collision with another option's canonical name");
293+
optionDefinition,
294+
"old option name collision with another option's canonical name",
295+
allowDuplicatesParsingEquivalently);
277296
checkForCollisions(
278297
oldNameToFieldBuilder,
279298
oldName,
280-
"old option name collision with another old option name");
299+
optionDefinition,
300+
"old option name collision with another old option name",
301+
allowDuplicatesParsingEquivalently);
281302
checkForBooleanAliasCollisions(booleanAliasMap, oldName, "old option name");
282303
// If boolean, repeat the alias dance for the old name.
283304
if (optionDefinition.usesBooleanValueSyntax()) {
284-
checkAndUpdateBooleanAliases(
285-
nameToFieldBuilder, oldNameToFieldBuilder, booleanAliasMap, oldName);
305+
checkAndUpdateBooleanAliases(nameToFieldBuilder, oldNameToFieldBuilder,
306+
booleanAliasMap, oldName, optionDefinition, allowDuplicatesParsingEquivalently);
286307
}
287308
// Now that we've checked for conflicts, confidently store the old name.
288309
oldNameToFieldBuilder.put(oldName, optionDefinition);
289310
}
290311
if (optionDefinition.getAbbreviation() != '\0') {
291-
checkForCollisions(
292-
abbrevToFieldBuilder, optionDefinition.getAbbreviation(), "option abbreviation");
312+
checkForCollisions(abbrevToFieldBuilder, optionDefinition.getAbbreviation(),
313+
optionDefinition, "option abbreviation", allowDuplicatesParsingEquivalently);
293314
abbrevToFieldBuilder.put(optionDefinition.getAbbreviation(), optionDefinition);
294315
}
295316
} catch (DuplicateOptionDeclarationException e) {

0 commit comments

Comments
 (0)