Skip to content

Commit

Permalink
Support all-supported pseudo command in rc files
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fmeum committed May 2, 2023
1 parent a0cd355 commit 85daf39
Show file tree
Hide file tree
Showing 14 changed files with 554 additions and 204 deletions.
8 changes: 7 additions & 1 deletion site/en/run/bazelrc.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,13 @@ line specifies when these defaults are applied:
- `startup`: startup options, which go before the command, and are described
in `bazel help startup_options`.
- `common`: options that apply to all Bazel commands.
- `common`: options that apply to all Bazel commands. If a command does not
support an option specified in this way, it will fail.
- `all-supported`: options that should be applied to all Bazel commands that
support them. If a command does not support an option specified in this way,
the option will be ignored. Note that this only applies to option names: If
the current command accepts an option with the specified name, but doesn't
support the specified value, it will fail.
- _`command`_: Bazel command, such as `build` or `query` to which the options
apply. These options also apply to all commands that inherit from the
specified command. (For example, `test` inherits from `build`.)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.runtime;

import static com.google.common.collect.ImmutableList.toImmutableList;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.collect.ArrayListMultimap;
Expand All @@ -33,8 +35,10 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.common.options.Converters;
import com.google.devtools.common.options.InvocationPolicyEnforcer;
import com.google.devtools.common.options.OpaqueOptionsData;
import com.google.devtools.common.options.OptionDefinition;
import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.OptionsParsingResult;
Expand Down Expand Up @@ -66,6 +70,8 @@ public final class BlazeOptionHandler {
"client_env",
"client_cwd");

private static final String ALL_SUPPORTED_PSEUDO_COMMAND = "all-supported";

// Marks an event to indicate a parsing error.
static final String BAD_OPTION_TAG = "invalidOption";
// Separates the invalid tag from the full error message for easier parsing.
Expand All @@ -78,6 +84,7 @@ public final class BlazeOptionHandler {
private final Command commandAnnotation;
private final InvocationPolicy invocationPolicy;
private final List<String> rcfileNotes = new ArrayList<>();
private final List<Class<? extends OptionsBase>> allOptionsClasses;

BlazeOptionHandler(
BlazeRuntime runtime,
Expand All @@ -92,6 +99,12 @@ public final class BlazeOptionHandler {
this.commandAnnotation = commandAnnotation;
this.optionsParser = optionsParser;
this.invocationPolicy = invocationPolicy;
this.allOptionsClasses = runtime.getCommandMap().values().stream()
.map(BlazeCommand::getClass)
.flatMap(cmd -> BlazeCommandUtils.getOptions(cmd, runtime.getBlazeModules(),
runtime.getRuleClassProvider()).stream())
.distinct()
.collect(toImmutableList());
}

/**
Expand Down Expand Up @@ -191,7 +204,22 @@ void parseRcOptions(
"%s:\n %s'%s' options: %s",
source, inherited, commandToParse, Joiner.on(' ').join(rcArgs.getArgs())));
}
optionsParser.parse(PriorityCategory.RC_FILE, rcArgs.getRcFile(), rcArgs.getArgs());
if (commandToParse.equals(ALL_SUPPORTED_PSEUDO_COMMAND)) {
// Pass in options data for all commands supported by the runtime so that options that
// apply to some but not the current command can be ignored.
//
// Important note: The consistency checks performed by
// OptionsParser#getFallbackOptionsData ensure that there aren't any two options across
// all commands that have the same name but parse differently (e.g. because one accepts
// a value and the other doesn't). This means that the options available on a command
// limit the options available on other commands even without command inheritance. This
// restriction is necessary to ensure that the options specified on the "all-supported"
// pseudo command can be parsed unambiguously.
optionsParser.parseWithSourceFunction(PriorityCategory.RC_FILE, o -> rcArgs.getRcFile(),
rcArgs.getArgs(), OptionsParser.getFallbackOptionsData(allOptionsClasses));
} else {
optionsParser.parse(PriorityCategory.RC_FILE, rcArgs.getRcFile(), rcArgs.getArgs());
}
}
}
}
Expand Down Expand Up @@ -227,7 +255,8 @@ private void parseArgsAndConfigs(List<String> args, ExtendedEventHandler eventHa
optionsParser.parseWithSourceFunction(
PriorityCategory.COMMAND_LINE,
commandOptionSourceFunction,
defaultOverridesAndRcSources.build());
defaultOverridesAndRcSources.build(),
null);

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

// Parses the remaining command-line options.
optionsParser.parseWithSourceFunction(
PriorityCategory.COMMAND_LINE, commandOptionSourceFunction, remainingCmdLine.build());
PriorityCategory.COMMAND_LINE, commandOptionSourceFunction, remainingCmdLine.build(), null);

if (commandAnnotation.builds()) {
// splits project files from targets in the traditional sense
Expand Down Expand Up @@ -374,12 +403,14 @@ void expandConfigOptions(
commandToRcArgs,
getCommandNamesToParse(commandAnnotation),
rcfileNotes::add,
optionsParser);
optionsParser,
OptionsParser.getFallbackOptionsData(allOptionsClasses));
}

private static List<String> getCommandNamesToParse(Command commandAnnotation) {
List<String> result = new ArrayList<>();
result.add("common");
result.add(ALL_SUPPORTED_PSEUDO_COMMAND);
getCommandNamesToParseHelper(commandAnnotation, result);
return result;
}
Expand Down Expand Up @@ -470,7 +501,8 @@ static ListMultimap<String, RcChunkOfArgs> structureRcOptionsAndConfigs(
if (index > 0) {
command = command.substring(0, index);
}
if (!validCommands.contains(command) && !command.equals("common")) {
if (!validCommands.contains(command) && !command.equals("common") && !command.equals(
ALL_SUPPORTED_PSEUDO_COMMAND)) {
eventHandler.handle(
Event.warn(
"while reading option defaults file '"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1113,7 +1113,7 @@ private static OptionsParsingResult parseStartupOptions(

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.common.options.OpaqueOptionsData;
import com.google.devtools.common.options.OptionValueDescription;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
Expand Down Expand Up @@ -88,7 +89,8 @@ static void expandConfigOptions(
ListMultimap<String, RcChunkOfArgs> commandToRcArgs,
List<String> commandsToParse,
Consumer<String> rcFileNotesConsumer,
OptionsParser optionsParser)
OptionsParser optionsParser,
OpaqueOptionsData fallbackData)
throws OptionsParsingException {

OptionValueDescription configValueDescription =
Expand All @@ -113,7 +115,8 @@ static void expandConfigOptions(
optionsParser.parseArgsAsExpansionOfOption(
configInstance,
String.format("expanded from --config=%s", configValueToExpand),
expansion);
expansion,
fallbackData);
}
}

Expand All @@ -131,7 +134,8 @@ static void expandConfigOptions(
optionsParser.parseArgsAsExpansionOfOption(
Iterables.getOnlyElement(enablePlatformSpecificConfigDescription.getCanonicalInstances()),
String.format("enabled by --enable_platform_specific_config"),
expansion);
expansion,
fallbackData);
}

// At this point, we've expanded everything, identify duplicates, if any, to warn about
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/common/options/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ java_library(
],
),
deps = [
"//src/main/java/com/google/devtools/build/lib/util:pair",
"//third_party:auto_value",
"//third_party:caffeine",
"//third_party:guava",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,22 +83,22 @@ public static ImmutableList<OptionDefinition> getAllOptionDefinitionsForClass(

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

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

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

Expand Down Expand Up @@ -136,7 +136,7 @@ protected IsolatedOptionsData(IsolatedOptionsData other) {

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

/**
* Returns all {@link OptionDefinition} objects loaded, mapped by their canonical names. Entries
* appear ordered first by their options class (the order in which they were passed to {@link
* #from(Collection)}, and then in alphabetic order within each options class.
* appear ordered first by their options class (the order in which they were passed to
* {@link #from(Collection, boolean)}, and then in alphabetic order within each options class.
*/
public Iterable<Map.Entry<String, OptionDefinition>> getAllOptionDefinitions() {
return nameToField.entrySet();
Expand All @@ -177,9 +177,15 @@ public boolean getUsesOnlyCoreTypes(Class<? extends OptionsBase> optionsClass) {
* both single-character abbreviations and full names.
*/
private static <A> void checkForCollisions(
Map<A, OptionDefinition> aFieldMap, A optionName, String description)
Map<A, OptionDefinition> aFieldMap, A optionName, OptionDefinition definition,
String description, boolean allowDuplicatesParsingEquivalently)
throws DuplicateOptionDeclarationException {
if (aFieldMap.containsKey(optionName)) {
OptionDefinition otherDefinition = aFieldMap.get(optionName);
if (allowDuplicatesParsingEquivalently && OptionsParserImpl.equivalentForParsing(
otherDefinition, definition)) {
return;
}
throw new DuplicateOptionDeclarationException(
"Duplicate option name, due to " + description + ": --" + optionName);
}
Expand Down Expand Up @@ -212,22 +218,30 @@ private static void checkAndUpdateBooleanAliases(
Map<String, OptionDefinition> nameToFieldMap,
Map<String, OptionDefinition> oldNameToFieldMap,
Map<String, String> booleanAliasMap,
String optionName)
String optionName,
OptionDefinition optionDefinition,
boolean allowDuplicatesParsingEquivalently)
throws DuplicateOptionDeclarationException {
// Check that the negating alias does not conflict with existing flags.
checkForCollisions(nameToFieldMap, "no" + optionName, "boolean option alias");
checkForCollisions(oldNameToFieldMap, "no" + optionName, "boolean option alias");
checkForCollisions(nameToFieldMap, "no" + optionName, optionDefinition, "boolean option alias",
allowDuplicatesParsingEquivalently);
checkForCollisions(oldNameToFieldMap, "no" + optionName, optionDefinition,
"boolean option alias", allowDuplicatesParsingEquivalently);

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

/**
* Constructs an {@link IsolatedOptionsData} object for a parser that knows about the given {@link
* OptionsBase} classes. No inter-option analysis is done. Performs basic validity checks on each
* option in isolation.
* Constructs an {@link IsolatedOptionsData} object for a parser that knows about the given
* {@link OptionsBase} classes. No inter-option analysis is done. Performs basic validity checks
* on each option in isolation.
*
* <p>If {@code allowDuplicatesParsingEquivalently} is true, then options that collide in name but
* parse equivalently (e.g. both of them accept a value or both of them do not), are allowed.
*/
static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes) {
static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes,
boolean allowDuplicatesParsingEquivalently) {
// Mind which fields have to preserve order.
Map<Class<? extends OptionsBase>, Constructor<?>> constructorBuilder = new LinkedHashMap<>();
Map<String, OptionDefinition> nameToFieldBuilder = new LinkedHashMap<>();
Expand Down Expand Up @@ -256,15 +270,18 @@ static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes
for (OptionDefinition optionDefinition : optionDefinitions) {
try {
String optionName = optionDefinition.getOptionName();
checkForCollisions(nameToFieldBuilder, optionName, "option name collision");
checkForCollisions(nameToFieldBuilder, optionName, optionDefinition,
"option name collision", allowDuplicatesParsingEquivalently);
checkForCollisions(
oldNameToFieldBuilder,
optionName,
"option name collision with another option's old name");
optionDefinition,
"option name collision with another option's old name",
allowDuplicatesParsingEquivalently);
checkForBooleanAliasCollisions(booleanAliasMap, optionName, "option");
if (optionDefinition.usesBooleanValueSyntax()) {
checkAndUpdateBooleanAliases(
nameToFieldBuilder, oldNameToFieldBuilder, booleanAliasMap, optionName);
checkAndUpdateBooleanAliases(nameToFieldBuilder, oldNameToFieldBuilder, booleanAliasMap,
optionName, optionDefinition, allowDuplicatesParsingEquivalently);
}
nameToFieldBuilder.put(optionName, optionDefinition);

Expand All @@ -273,23 +290,27 @@ static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes
checkForCollisions(
nameToFieldBuilder,
oldName,
"old option name collision with another option's canonical name");
optionDefinition,
"old option name collision with another option's canonical name",
allowDuplicatesParsingEquivalently);
checkForCollisions(
oldNameToFieldBuilder,
oldName,
"old option name collision with another old option name");
optionDefinition,
"old option name collision with another old option name",
allowDuplicatesParsingEquivalently);
checkForBooleanAliasCollisions(booleanAliasMap, oldName, "old option name");
// If boolean, repeat the alias dance for the old name.
if (optionDefinition.usesBooleanValueSyntax()) {
checkAndUpdateBooleanAliases(
nameToFieldBuilder, oldNameToFieldBuilder, booleanAliasMap, oldName);
checkAndUpdateBooleanAliases(nameToFieldBuilder, oldNameToFieldBuilder,
booleanAliasMap, oldName, optionDefinition, allowDuplicatesParsingEquivalently);
}
// Now that we've checked for conflicts, confidently store the old name.
oldNameToFieldBuilder.put(oldName, optionDefinition);
}
if (optionDefinition.getAbbreviation() != '\0') {
checkForCollisions(
abbrevToFieldBuilder, optionDefinition.getAbbreviation(), "option abbreviation");
checkForCollisions(abbrevToFieldBuilder, optionDefinition.getAbbreviation(),
optionDefinition, "option abbreviation", allowDuplicatesParsingEquivalently);
abbrevToFieldBuilder.put(optionDefinition.getAbbreviation(), optionDefinition);
}
} catch (DuplicateOptionDeclarationException e) {
Expand Down
Loading

0 comments on commit 85daf39

Please sign in to comment.