diff --git a/site/en/run/bazelrc.md b/site/en/run/bazelrc.md index 0e0518aae15e63..4c247ec10d8175 100644 --- a/site/en/run/bazelrc.md +++ b/site/en/run/bazelrc.md @@ -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`.) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java index 35ef818191765e..113569aa4db35a 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java @@ -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; @@ -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; @@ -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. @@ -78,6 +84,7 @@ public final class BlazeOptionHandler { private final Command commandAnnotation; private final InvocationPolicy invocationPolicy; private final List rcfileNotes = new ArrayList<>(); + private final List> allOptionsClasses; BlazeOptionHandler( BlazeRuntime runtime, @@ -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()); } /** @@ -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()); + } } } } @@ -227,7 +255,8 @@ private void parseArgsAndConfigs(List 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); @@ -241,7 +270,7 @@ private void parseArgsAndConfigs(List 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 @@ -374,12 +403,14 @@ void expandConfigOptions( commandToRcArgs, getCommandNamesToParse(commandAnnotation), rcfileNotes::add, - optionsParser); + optionsParser, + OptionsParser.getFallbackOptionsData(allOptionsClasses)); } private static List getCommandNamesToParse(Command commandAnnotation) { List result = new ArrayList<>(); result.add("common"); + result.add(ALL_SUPPORTED_PSEUDO_COMMAND); getCommandNamesToParseHelper(commandAnnotation, result); return result; } @@ -470,7 +501,8 @@ static ListMultimap 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 '" diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java index d79a509bf990b7..076b5e10d66790 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java @@ -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; } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ConfigExpander.java b/src/main/java/com/google/devtools/build/lib/runtime/ConfigExpander.java index 998908a5102698..508319bad9f6d4 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/ConfigExpander.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/ConfigExpander.java @@ -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; @@ -88,7 +89,8 @@ static void expandConfigOptions( ListMultimap commandToRcArgs, List commandsToParse, Consumer rcFileNotesConsumer, - OptionsParser optionsParser) + OptionsParser optionsParser, + OpaqueOptionsData fallbackData) throws OptionsParsingException { OptionValueDescription configValueDescription = @@ -113,7 +115,8 @@ static void expandConfigOptions( optionsParser.parseArgsAsExpansionOfOption( configInstance, String.format("expanded from --config=%s", configValueToExpand), - expansion); + expansion, + fallbackData); } } @@ -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 diff --git a/src/main/java/com/google/devtools/common/options/BUILD b/src/main/java/com/google/devtools/common/options/BUILD index 50d66920e93e15..b5120de838dc0c 100644 --- a/src/main/java/com/google/devtools/common/options/BUILD +++ b/src/main/java/com/google/devtools/common/options/BUILD @@ -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", diff --git a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java index 1c7c8ecd172e8a..cf08cf274639b9 100644 --- a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java +++ b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java @@ -83,22 +83,22 @@ public static ImmutableList 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, 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 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 oldNameToField; @@ -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> getOptionsClasses() { return optionsClasses.keySet(); @@ -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> getAllOptionDefinitions() { return nameToField.entrySet(); @@ -177,9 +177,15 @@ public boolean getUsesOnlyCoreTypes(Class optionsClass) { * both single-character abbreviations and full names. */ private static void checkForCollisions( - Map aFieldMap, A optionName, String description) + Map 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); } @@ -212,22 +218,30 @@ private static void checkAndUpdateBooleanAliases( Map nameToFieldMap, Map oldNameToFieldMap, Map 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. + * + *

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> classes) { + static IsolatedOptionsData from(Collection> classes, + boolean allowDuplicatesParsingEquivalently) { // Mind which fields have to preserve order. Map, Constructor> constructorBuilder = new LinkedHashMap<>(); Map nameToFieldBuilder = new LinkedHashMap<>(); @@ -256,15 +270,18 @@ static IsolatedOptionsData from(Collection> 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); @@ -273,23 +290,27 @@ static IsolatedOptionsData from(Collection> 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) { diff --git a/src/main/java/com/google/devtools/common/options/OptionsData.java b/src/main/java/com/google/devtools/common/options/OptionsData.java index 2a3e485b7bf5dd..2cbf251b0a20c0 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsData.java +++ b/src/main/java/com/google/devtools/common/options/OptionsData.java @@ -28,14 +28,26 @@ @Immutable final class OptionsData extends IsolatedOptionsData { - /** Mapping from each option to the (unparsed) options it expands to, if any. */ + /** + * Mapping from each option to the (unparsed) options it expands to, if any. + */ private final ImmutableMap> evaluatedExpansions; - /** Construct {@link OptionsData} by extending an {@link IsolatedOptionsData} with new info. */ + /** + * Whether this options data has been created with duplicate options definitions allowed as long + * as those options are parsed (but not necessarily evaluated) equivalently. + */ + private final boolean allowDuplicatesParsingEquivalently; + + /** + * Construct {@link OptionsData} by extending an {@link IsolatedOptionsData} with new info. + */ private OptionsData( - IsolatedOptionsData base, Map> evaluatedExpansions) { + IsolatedOptionsData base, Map> evaluatedExpansions, + boolean allowDuplicatesParsingEquivalently) { super(base); this.evaluatedExpansions = ImmutableMap.copyOf(evaluatedExpansions); + this.allowDuplicatesParsingEquivalently = allowDuplicatesParsingEquivalently; } private static final ImmutableList EMPTY_EXPANSION = ImmutableList.of(); @@ -50,13 +62,23 @@ public ImmutableList getEvaluatedExpansion(OptionDefinition optionDefini } /** - * Constructs an {@link OptionsData} object for a parser that knows about the given {@link - * OptionsBase} classes. In addition to the work done to construct the {@link - * IsolatedOptionsData}, this also computes expansion information. If an option has an expansion, - * try to precalculate its here. + * Returns whether this options data has been created with duplicate options definitions allowed + * as long as those options are parsed (but not necessarily evaluated) equivalently. + */ + public boolean createdWithAllowDuplicatesParsingEquivalently() { + return allowDuplicatesParsingEquivalently; + } + + /** + * Constructs an {@link OptionsData} object for a parser that knows about the given + * {@link OptionsBase} classes. In addition to the work done to construct the + * {@link IsolatedOptionsData}, this also computes expansion information. If an option has an + * expansion, try to precalculate its here. */ - static OptionsData from(Collection> classes) { - IsolatedOptionsData isolatedData = IsolatedOptionsData.from(classes); + static OptionsData from(Collection> classes, + boolean allowDuplicatesParsingEquivalently) { + IsolatedOptionsData isolatedData = IsolatedOptionsData.from(classes, + allowDuplicatesParsingEquivalently); // All that's left is to compute expansions. ImmutableMap.Builder> evaluatedExpansionsBuilder = @@ -68,6 +90,7 @@ static OptionsData from(Collection> classes) { evaluatedExpansionsBuilder.put(optionDefinition, ImmutableList.copyOf(constExpansion)); } } - return new OptionsData(isolatedData, evaluatedExpansionsBuilder.build()); + return new OptionsData(isolatedData, evaluatedExpansionsBuilder.build(), + allowDuplicatesParsingEquivalently); } } diff --git a/src/main/java/com/google/devtools/common/options/OptionsParser.java b/src/main/java/com/google/devtools/common/options/OptionsParser.java index fdea7cc5d4bd08..a3db0b2abe380e 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParser.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java @@ -30,6 +30,7 @@ import com.google.common.collect.ListMultimap; import com.google.common.collect.MoreCollectors; import com.google.common.escape.Escaper; +import com.google.devtools.build.lib.util.Pair; import com.google.devtools.common.options.OptionsParserImpl.OptionsParserImplResult; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.ArrayList; @@ -103,8 +104,8 @@ public ConstructionException(String message, Throwable cause) { * cache is very unlikely to grow to a significant amount of memory, because there's only a fixed * set of options classes on the classpath. */ - private static final Map>, OptionsData> optionsData = - new HashMap<>(); + private static final Map>, Boolean>, OptionsData> + optionsData = new HashMap<>(); /** Skipped prefixes for starlark options. */ public static final ImmutableList STARLARK_SKIPPED_PREFIXES = @@ -120,30 +121,40 @@ public ConstructionException(String message, Throwable cause) { */ public static OpaqueOptionsData getOptionsData( List> optionsClasses) { - return getOptionsDataInternal(optionsClasses); + return getOptionsDataInternal(optionsClasses, false); } - /** Returns the {@link OptionsData} associated with the given list of options classes. */ - static synchronized OptionsData getOptionsDataInternal( + public static OpaqueOptionsData getFallbackOptionsData( List> optionsClasses) { + return getOptionsDataInternal(optionsClasses, true); + } + + /** + * Returns the {@link OptionsData} associated with the given list of options classes. + */ + static synchronized OptionsData getOptionsDataInternal( + List> optionsClasses, + boolean allowDuplicatesParsingEquivalently) { ImmutableList> immutableOptionsClasses = ImmutableList.copyOf(optionsClasses); - OptionsData result = optionsData.get(immutableOptionsClasses); + Pair>, Boolean> cacheKey = Pair.of( + immutableOptionsClasses, allowDuplicatesParsingEquivalently); + OptionsData result = optionsData.get(cacheKey); if (result == null) { try { - result = OptionsData.from(immutableOptionsClasses); + result = OptionsData.from(immutableOptionsClasses, allowDuplicatesParsingEquivalently); } catch (Exception e) { Throwables.throwIfInstanceOf(e, ConstructionException.class); throw new ConstructionException(e.getMessage(), e); } - optionsData.put(immutableOptionsClasses, result); + optionsData.put(cacheKey, result); } return result; } /** Returns the {@link OptionsData} associated with the given options class. */ static OptionsData getOptionsDataInternal(Class optionsClass) { - return getOptionsDataInternal(ImmutableList.of(optionsClass)); + return getOptionsDataInternal(ImmutableList.of(optionsClass), false); } /** A helper class to create new instances of {@link OptionsParser}. */ @@ -158,6 +169,7 @@ private Builder(OptionsParserImpl.Builder implBuilder) { /** Directly sets the {@link OptionsData} used by this parser. */ @CanIgnoreReturnValue public Builder optionsData(OptionsData optionsData) { + Preconditions.checkArgument(!optionsData.createdWithAllowDuplicatesParsingEquivalently()); this.implBuilder.optionsData(optionsData); return this; } @@ -173,7 +185,7 @@ public Builder optionsData(OpaqueOptionsData optionsData) { @SafeVarargs public final Builder optionsClasses(Class... optionsClasses) { return this.optionsData( - (OpaqueOptionsData) getOptionsDataInternal(ImmutableList.copyOf(optionsClasses))); + (OpaqueOptionsData) getOptionsDataInternal(ImmutableList.copyOf(optionsClasses), false)); } /** @@ -181,7 +193,7 @@ public final Builder optionsClasses(Class... optionsClass */ public Builder optionsClasses(Iterable> optionsClasses) { return this.optionsData( - (OpaqueOptionsData) getOptionsDataInternal(ImmutableList.copyOf(optionsClasses))); + (OpaqueOptionsData) getOptionsDataInternal(ImmutableList.copyOf(optionsClasses), false)); } /** @@ -699,7 +711,7 @@ public void parse(List args) throws OptionsParsingException { */ public void parse(OptionPriority.PriorityCategory priority, String source, List args) throws OptionsParsingException { - parseWithSourceFunction(priority, o -> source, args); + parseWithSourceFunction(priority, o -> source, args, null); } /** @@ -715,17 +727,22 @@ public void parse(OptionPriority.PriorityCategory priority, String source, List< * each option will be given an index to track its position. If parse() has already been * called at this priority, the indexing will continue where it left off, to keep ordering. * @param sourceFunction a function that maps option names to the source of the option. + * @param fallbackData if provided, the full collection of options that should be parsed and + * ignored without raising an error if they are not recognized by the options classes + * registered with this parser. * @param args the arg list to parse. Each element might be an option, a value linked to an * option, or residue. */ public void parseWithSourceFunction( OptionPriority.PriorityCategory priority, Function sourceFunction, - List args) + List args, + @Nullable OpaqueOptionsData fallbackData) throws OptionsParsingException { Preconditions.checkNotNull(priority); Preconditions.checkArgument(priority != OptionPriority.PriorityCategory.DEFAULT); - OptionsParserImplResult optionsParserImplResult = impl.parse(priority, sourceFunction, args); + OptionsParserImplResult optionsParserImplResult = impl.parse(priority, sourceFunction, args, + (OptionsData) fallbackData); addResidueFromResult(optionsParserImplResult); aliases.putAll(optionsParserImplResult.aliases); } @@ -739,9 +756,13 @@ public void parseWithSourceFunction( * @param source a description of where the expansion arguments came from. * @param args the arguments to parse as the expansion. Order matters, as the value of a flag may * be in the following argument. + * @param fallbackData if provided, the full collection of options that should be parsed and + * ignored without raising an error if they are not recognized by the options classes + * registered with this parser. */ public void parseArgsAsExpansionOfOption( - ParsedOptionDescription optionToExpand, String source, List args) + ParsedOptionDescription optionToExpand, String source, List args, + @Nullable OpaqueOptionsData fallbackData) throws OptionsParsingException { Preconditions.checkNotNull( optionToExpand, "Option for expansion not specified for arglist %s", args); @@ -750,8 +771,8 @@ public void parseArgsAsExpansionOfOption( != OptionPriority.PriorityCategory.DEFAULT, "Priority cannot be default, which was specified for arglist %s", args); - OptionsParserImplResult optionsParserImplResult = - impl.parseArgsAsExpansionOfOption(optionToExpand, o -> source, args); + OptionsParserImplResult optionsParserImplResult = impl.parseArgsAsExpansionOfOption( + optionToExpand, o -> source, args, (OptionsData) fallbackData); addResidueFromResult(optionsParserImplResult); } diff --git a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java index d6a7bee4693db7..5a08f7ed1ff2d0 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java @@ -39,7 +39,9 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; +import java.util.function.BiFunction; import java.util.function.Function; import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -406,15 +408,15 @@ ImmutableList getExpansionValueDescriptions( Iterator optionsIterator = options.iterator(); while (optionsIterator.hasNext()) { String unparsedFlagExpression = optionsIterator.next(); - ParsedOptionDescription parsedOption = - identifyOptionAndPossibleArgument( - unparsedFlagExpression, - optionsIterator, - nextOptionPriority, - o -> source, - implicitDependent, - expandedFrom); - builder.add(parsedOption); + identifyOptionAndPossibleArgument( + unparsedFlagExpression, + optionsIterator, + nextOptionPriority, + o -> source, + implicitDependent, + expandedFrom, + /* fallbackData */ null) + .ifPresent(builder::add); nextOptionPriority = OptionPriority.nextOptionPriority(nextOptionPriority); } return builder.build(); @@ -447,10 +449,12 @@ List getSkippedArgs() { OptionsParserImplResult parse( PriorityCategory priorityCat, Function sourceFunction, - List args) + List args, + OptionsData fallbackData) throws OptionsParsingException { - OptionsParserImplResult optionsParserImplResult = - parse(nextPriorityPerPriorityCategory.get(priorityCat), sourceFunction, null, null, args); + OptionsParserImplResult optionsParserImplResult = parse( + nextPriorityPerPriorityCategory.get(priorityCat), sourceFunction, null, null, args, + fallbackData); nextPriorityPerPriorityCategory.put(priorityCat, optionsParserImplResult.nextPriority); return optionsParserImplResult; } @@ -468,7 +472,8 @@ private OptionsParserImplResult parse( Function sourceFunction, ParsedOptionDescription implicitDependent, ParsedOptionDescription expandedFrom, - List args) + List args, + OptionsData fallbackData) throws OptionsParsingException { List unparsedArgs = new ArrayList<>(); List unparsedPostDoubleDashArgs = new ArrayList<>(); @@ -501,14 +506,14 @@ private OptionsParserImplResult parse( break; } - ParsedOptionDescription parsedOption; + Optional parsedOption; if (containsSkippedPrefix(arg)) { // Parse the skipped arg into a synthetic allowMultiple option to preserve its order // relative to skipped args coming from expansions. Simply adding it to the residue would // end up placing expanded skipped args after all explicitly given skipped args, which isn't // correct. parsedOption = - ParsedOptionDescription.newParsedOptionDescription( + Optional.of(ParsedOptionDescription.newParsedOptionDescription( skippedArgsDefinition, arg, arg, @@ -517,13 +522,14 @@ private OptionsParserImplResult parse( sourceFunction.apply(skippedArgsDefinition), implicitDependent, expandedFrom), - conversionContext); + conversionContext)); } else { - parsedOption = - identifyOptionAndPossibleArgument( - arg, argsIterator, priority, sourceFunction, implicitDependent, expandedFrom); + parsedOption = identifyOptionAndPossibleArgument(arg, argsIterator, priority, + sourceFunction, implicitDependent, expandedFrom, fallbackData); + } + if (parsedOption.isPresent()) { + handleNewParsedOption(parsedOption.get()); } - handleNewParsedOption(parsedOption); priority = OptionPriority.nextOptionPriority(priority); } @@ -569,14 +575,16 @@ public List getResidue() { OptionsParserImplResult parseArgsAsExpansionOfOption( ParsedOptionDescription optionToExpand, Function sourceFunction, - List args) + List args, + OptionsData fallbackData) throws OptionsParsingException { return parse( OptionPriority.getChildPriority(optionToExpand.getPriority()), sourceFunction, null, optionToExpand, - args); + args, + fallbackData); } /** @@ -630,7 +638,8 @@ private void handleNewParsedOption(ParsedOptionDescription parsedOption) o -> expansionBundle.sourceOfExpansionArgs, optionDefinition.hasImplicitRequirements() ? parsedOption : null, optionDefinition.isExpansionOption() ? parsedOption : null, - expansionBundle.expansionArgs); + expansionBundle.expansionArgs, + null); if (!optionsParserImplResult.getResidue().isEmpty()) { // Throw an assertion here, because this indicates an error in the definition of this @@ -693,28 +702,38 @@ private ExpansionBundle setOptionValue(ParsedOptionDescription parsedOption) return expansionBundle; } - private ParsedOptionDescription identifyOptionAndPossibleArgument( + /** + * Keep the properties of {@link OptionsData} used below in sync with {@link + * #equivalentForParsing}. + * + *

If an option is not found in the current {@link OptionsData}, but is found in the specified + * fallback data, an empty {@link Optional} is returned rather than throwing an exception. + */ + private Optional identifyOptionAndPossibleArgument( String arg, Iterator nextArgs, OptionPriority priority, Function sourceFunction, ParsedOptionDescription implicitDependent, - ParsedOptionDescription expandedFrom) + ParsedOptionDescription expandedFrom, + @Nullable OptionsData fallbackData) throws OptionsParsingException { // Store the way this option was parsed on the command line. StringBuilder commandLineForm = new StringBuilder(); commandLineForm.append(arg); String unconvertedValue = null; - OptionDefinition optionDefinition; + OptionLookupResult lookupResult; boolean booleanValue = true; if (arg.length() == 2) { // -l (may be nullary or unary) - optionDefinition = optionsData.getFieldForAbbrev(arg.charAt(1)); + lookupResult = getWithFallback(OptionsData::getFieldForAbbrev, arg.charAt(1), + fallbackData); booleanValue = true; } else if (arg.length() == 3 && arg.charAt(2) == '-') { // -l- (boolean) - optionDefinition = optionsData.getFieldForAbbrev(arg.charAt(1)); + lookupResult = getWithFallback(OptionsData::getFieldForAbbrev, arg.charAt(1), + fallbackData); booleanValue = false; } else if (arg.startsWith("--")) { // --long_option @@ -727,16 +746,17 @@ private ParsedOptionDescription identifyOptionAndPossibleArgument( throw new OptionsParsingException("Invalid options syntax: " + arg, arg); } unconvertedValue = equalsAt == -1 ? null : arg.substring(equalsAt + 1); - optionDefinition = optionsData.getOptionDefinitionFromName(name); + lookupResult = getWithFallback(OptionsData::getOptionDefinitionFromName, name, fallbackData); // Look for a "no"-prefixed option name: "no". - if (optionDefinition == null && name.startsWith("no")) { + if (lookupResult == null && name.startsWith("no")) { name = name.substring(2); - optionDefinition = optionsData.getOptionDefinitionFromName(name); + lookupResult = getWithFallback(OptionsData::getOptionDefinitionFromName, name, + fallbackData); booleanValue = false; - if (optionDefinition != null) { + if (lookupResult != null) { // TODO(bazel-team): Add tests for these cases. - if (!optionDefinition.usesBooleanValueSyntax()) { + if (!lookupResult.definition.usesBooleanValueSyntax()) { throw new OptionsParsingException( "Illegal use of 'no' prefix on non-boolean option: " + arg, arg); } @@ -751,16 +771,16 @@ private ParsedOptionDescription identifyOptionAndPossibleArgument( throw new OptionsParsingException("Invalid options syntax: " + arg, arg); } - if (optionDefinition == null || shouldIgnoreOption(optionDefinition)) { + if (lookupResult == null || shouldIgnoreOption(lookupResult.definition)) { // Do not recognize internal options, which are treated as if they did not exist. throw new OptionsParsingException("Unrecognized option: " + arg, arg); } if (unconvertedValue == null) { // Special-case boolean to supply value based on presence of "no" prefix. - if (optionDefinition.usesBooleanValueSyntax()) { + if (lookupResult.definition.usesBooleanValueSyntax()) { unconvertedValue = booleanValue ? "1" : "0"; - } else if (optionDefinition.getType().equals(Void.class)) { + } else if (lookupResult.definition.getType().equals(Void.class)) { // This is expected, Void type options have no args. } else if (nextArgs.hasNext()) { // "--flag value" form @@ -771,22 +791,69 @@ private ParsedOptionDescription identifyOptionAndPossibleArgument( } } - return ParsedOptionDescription.newParsedOptionDescription( - optionDefinition, + if (lookupResult.fromFallback) { + // The option was not found on the current command, but is a valid option for some other + // command. Ignore it. + return Optional.empty(); + } + + return Optional.of(ParsedOptionDescription.newParsedOptionDescription( + lookupResult.definition, commandLineForm.toString(), unconvertedValue, - new OptionInstanceOrigin( - priority, sourceFunction.apply(optionDefinition), implicitDependent, expandedFrom), - conversionContext); + new OptionInstanceOrigin(priority, sourceFunction.apply(lookupResult.definition), + implicitDependent, expandedFrom), + conversionContext)); + } + + /** + * Keep in sync with the properties of OptionsData used in + * {@link #identifyOptionAndPossibleArgument}. + */ + public static boolean equivalentForParsing(OptionDefinition definition, + OptionDefinition otherDefinition) { + if (definition.equals(otherDefinition)) { + return true; + } + return (definition.usesBooleanValueSyntax() == otherDefinition.usesBooleanValueSyntax()) + && (definition.getType().equals(Void.class) == otherDefinition.getType().equals(Void.class)) + && (ImmutableList.copyOf(definition.getOptionMetadataTags()) + .contains(OptionMetadataTag.INTERNAL) == ImmutableList.copyOf( + otherDefinition.getOptionMetadataTags()).contains(OptionMetadataTag.INTERNAL)); + } + + private static final class OptionLookupResult { + + final OptionDefinition definition; + final boolean fromFallback; + + private OptionLookupResult(OptionDefinition definition, boolean fromFallback) { + this.definition = definition; + this.fromFallback = fromFallback; + } + } + + private OptionLookupResult getWithFallback( + BiFunction getter, T param, OptionsData fallbackData) { + OptionDefinition optionDefinition; + if ((optionDefinition = getter.apply(optionsData, param)) != null) { + return new OptionLookupResult(optionDefinition, false); + } + if (fallbackData != null && (optionDefinition = getter.apply(fallbackData, param)) != null) { + return new OptionLookupResult(optionDefinition, true); + } + return null; } private boolean shouldIgnoreOption(OptionDefinition optionDefinition) { return ignoreInternalOptions && ImmutableList.copyOf(optionDefinition.getOptionMetadataTags()) - .contains(OptionMetadataTag.INTERNAL); + .contains(OptionMetadataTag.INTERNAL); } - /** Gets the result of parsing the options. */ + /** + * Gets the result of parsing the options. + */ O getParsedOptions(Class optionsClass) { // Create the instance: O optionsInstance; diff --git a/src/test/java/com/google/devtools/common/options/OptionsDataTest.java b/src/test/java/com/google/devtools/common/options/OptionsDataTest.java index ed922c0e6e87c1..776263e7a61019 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsDataTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsDataTest.java @@ -33,15 +33,14 @@ public class OptionsDataTest { private static IsolatedOptionsData construct(Class optionsClass) throws OptionsParser.ConstructionException { - return IsolatedOptionsData.from(ImmutableList.>of(optionsClass)); + return IsolatedOptionsData.from(ImmutableList.of(optionsClass), false); } private static IsolatedOptionsData construct( Class optionsClass1, Class optionsClass2) throws OptionsParser.ConstructionException { - return IsolatedOptionsData.from( - ImmutableList.>of(optionsClass1, optionsClass2)); + return IsolatedOptionsData.from(ImmutableList.of(optionsClass1, optionsClass2), false); } private static IsolatedOptionsData construct( @@ -49,9 +48,8 @@ private static IsolatedOptionsData construct( Class optionsClass2, Class optionsClass3) throws OptionsParser.ConstructionException { - return IsolatedOptionsData.from( - ImmutableList.>of( - optionsClass1, optionsClass2, optionsClass3)); + return IsolatedOptionsData.from(ImmutableList.of(optionsClass1, optionsClass2, optionsClass3), + false); } /** Dummy options class. */ diff --git a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java index b6b63d9a6ece15..f23afb2ea4c115 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter; import com.google.devtools.common.options.OptionPriority.PriorityCategory; +import com.google.devtools.common.options.OptionsParser.ConstructionException; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -209,20 +210,74 @@ public static class ExampleInternalOptions extends OptionsBase { public boolean privateBoolean; @Option( - name = "internal_string", - metadataTags = {OptionMetadataTag.INTERNAL}, - documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, - effectTags = {OptionEffectTag.NO_OP}, - defaultValue = "super secret" + name = "internal_string", + metadataTags = {OptionMetadataTag.INTERNAL}, + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.NO_OP}, + defaultValue = "super secret" ) public String privateString; } + public static class ExampleEquivalentWithFoo extends OptionsBase { + + @Option( + name = "foo", + category = "one", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.NO_OP}, + defaultValue = "differentDefault" + ) + public String foo; + + @Option( + name = "bar", + category = "one", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.NO_OP}, + defaultValue = "differentDefault" + ) + public String bar; + + @Option( + name = "not_foo", + category = "one", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.NO_OP}, + defaultValue = "differentDefault" + ) + public String notFoo; + + @Option( + name = "not_bar", + category = "one", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.NO_OP}, + defaultValue = "differentDefault" + ) + public String notBar; + } + + public static class ExampleIncompatibleWithFoo extends OptionsBase { + + @Option( + name = "foo", + category = "one", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.NO_OP}, + defaultValue = "true" + ) + public boolean foo; + } + + public static class StringConverter extends Converter.Contextless { + @Override public String convert(String input) { return input; } + @Override public String getTypeDescription() { return "a string"; @@ -275,7 +330,8 @@ public void parseWithSourceFunctionThrowsExceptionIfResidueIsNotAllowed() { parser.parseWithSourceFunction( PriorityCategory.COMMAND_LINE, sourceFunction, - ImmutableList.of("residue", "not", "allowed", "in", "parseWithSource"))); + ImmutableList.of("residue", "not", "allowed", "in", "parseWithSource"), + null)); assertThat(e) .hasMessageThat() .isEqualTo("Unrecognized arguments: residue not allowed in parseWithSource"); @@ -295,7 +351,8 @@ public void parseWithSourceFunctionDoesntThrowExceptionIfResidueIsAllowed() thro parser.parseWithSourceFunction( PriorityCategory.COMMAND_LINE, sourceFunction, - ImmutableList.of("residue", "is", "allowed", "in", "parseWithSource")); + ImmutableList.of("residue", "is", "allowed", "in", "parseWithSource"), + null); assertThat(parser.getResidue()) .containsExactly("residue", "is", "allowed", "in", "parseWithSource"); } @@ -320,7 +377,8 @@ public void parseArgsAsExpansionOfOptionThrowsExceptionIfResidueIsNotAllowed() t parser.parseArgsAsExpansionOfOption( optionToExpand, "source", - ImmutableList.of("--underlying=direct_value", "residue", "in", "expansion"))); + ImmutableList.of("--underlying=direct_value", "residue", "in", "expansion"), + null)); assertThat(parser.getResidue()).isNotEmpty(); assertThat(e).hasMessageThat().isEqualTo("Unrecognized arguments: residue in expansion"); } @@ -2398,6 +2456,31 @@ public void negativeExternalTargetPatternsInOptions_failsDistinctively() { .contains("Flags corresponding to Starlark-defined build settings always start with '--'"); } + @Test + public void fallbackOptions_optionsParsingEquivalently() + throws OptionsParsingException { + OpaqueOptionsData fallbackData = OptionsParser.getFallbackOptionsData( + ImmutableList.of(ExampleFoo.class, ExampleEquivalentWithFoo.class)); + OptionsParser parser = OptionsParser.builder().optionsClasses(ExampleFoo.class).build(); + parser.parseWithSourceFunction(PriorityCategory.RC_FILE, o -> ".bazelrc", + ImmutableList.of("--foo=bar", "--not_foo=baz", "--bar", "1", "--not_bar", "baz"), + fallbackData); + + assertThat(parser.getOptions(ExampleFoo.class)).isNotNull(); + assertThat(parser.getOptions(ExampleFoo.class).foo).isEqualTo("bar"); + assertThat(parser.getOptions(ExampleFoo.class).bar).isEqualTo(1); + + assertThat(parser.getOptions(ExampleEquivalentWithFoo.class)).isNull(); + } + + @Test + public void fallbackOptions_optionsParsingDifferently() { + Exception e = assertThrows(ConstructionException.class, + () -> OptionsParser.getFallbackOptionsData( + ImmutableList.of(ExampleFoo.class, ExampleIncompatibleWithFoo.class))); + assertThat(e).hasCauseThat().isInstanceOf(DuplicateOptionDeclarationException.class); + } + private static OptionInstanceOrigin createInvocationPolicyOrigin() { return createInvocationPolicyOrigin(/*implicitDependent=*/ null, /*expandedFrom=*/ null); } diff --git a/src/test/py/bazel/BUILD b/src/test/py/bazel/BUILD index 9ab83f5e719010..4b7043e8fea777 100644 --- a/src/test/py/bazel/BUILD +++ b/src/test/py/bazel/BUILD @@ -329,8 +329,8 @@ py_test( ) py_test( - name = "starlark_options_test", - srcs = ["starlark_options_test.py"], + name = "options_test", + srcs = ["options_test.py"], deps = [":test_base"], ) diff --git a/src/test/py/bazel/options_test.py b/src/test/py/bazel/options_test.py new file mode 100644 index 00000000000000..004d1d790a2411 --- /dev/null +++ b/src/test/py/bazel/options_test.py @@ -0,0 +1,174 @@ +# Copyright 2022 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest + +from src.test.py.bazel import test_base + + +class OptionsTest(test_base.TestBase): + + def testCanOverrideStarlarkFlagInBazelrcConfigStanza(self): + self.ScratchFile("WORKSPACE.bazel") + self.ScratchFile("bazelrc", [ + "build:red --//f:color=red", + ]) + self.ScratchFile("f/BUILD.bazel", [ + 'load(":f.bzl", "color", "r")', + "color(", + ' name = "color",', + ' build_setting_default = "white",', + ")", + 'r(name = "r")', + ]) + self.ScratchFile("f/f.bzl", [ + 'ColorValue = provider("color")', + "def _color_impl(ctx):", + " return [ColorValue(color = ctx.build_setting_value)]", + "color = rule(", + " implementation = _color_impl,", + "build_setting = config.string(flag = True),", + ")", + "def _r_impl(ctx):", + " print(ctx.attr._color[ColorValue].color)", + " return [DefaultInfo()]", + "r = rule(", + " implementation = _r_impl,", + ' attrs = {"_color": attr.label(default = "//f:color")},', + ")", + ]) + + _, _, stderr = self.RunBazel([ + "--bazelrc=bazelrc", + "build", + "--nobuild", + "//f:r", + "--config=red", + "--//f:color=green", + ]) + self.assertTrue( + any("/f/f.bzl:9:10: green" in line for line in stderr), + "\n".join(stderr), + ) + + _, _, stderr = self.RunBazel([ + "--bazelrc=bazelrc", + "build", + "--nobuild", + "//f:r", + "--//f:color=green", + "--config=red", + ]) + self.assertTrue( + any("/f/f.bzl:9:10: red" in line for line in stderr), + "\n".join(stderr), + ) + + def testAllSupportedPseudoCommand(self): + self.ScratchFile("WORKSPACE.bazel") + self.ScratchFile(".bazelrc", [ + "all-supported --copt=-Dfoo", + "all-supported --copt -Dbar", + "all-supported:my-config --copt=-Dbaz", + "all-supported:my-config --copt -Dquz", + ]) + self.ScratchFile("pkg/BUILD.bazel", [ + "cc_binary(name='main',srcs=['main.cc'])", + ]) + self.ScratchFile("pkg/main.cc", [ + "#include ", + "int main() {", + "#ifdef foo", + " printf(\"foo\\n\");", + "#endif", + "#ifdef bar", + " printf(\"bar\\n\");", + "#endif", + "#ifdef baz", + " printf(\"baz\\n\");", + "#endif", + "#ifdef quz", + " printf(\"quz\\n\");", + "#endif", + " return 0;", + "}", + ]) + + # Check that run honors the all-supported flags. + _, stdout, _ = self.RunBazel([ + "run", + "//pkg:main", + ]) + self.assertEquals( + ["foo", "bar"], + stdout, + ) + + _, stdout, _ = self.RunBazel([ + "run", + "--config=my-config", + "//pkg:main", + ]) + self.assertEquals( + ["foo", "bar", "baz", "quz"], + stdout, + ) + + # Check that query ignores the unsupported all-supported flags. + _, stdout, _ = self.RunBazel([ + "query", + "//pkg:main", + ]) + + _, stdout, _ = self.RunBazel([ + "query", + "--config=my-config", + "//pkg:main", + ]) + + def testAllSupportedPseudoCommand_unsupportedOptionValue(self): + self.ScratchFile("WORKSPACE.bazel") + self.ScratchFile(".bazelrc", [ + "all-supported --output=starlark", + ]) + self.ScratchFile("pkg/BUILD.bazel", [ + "cc_binary(name='main',srcs=['main.cc'])", + ]) + + # Check that cquery honors the all-supported flag. + _, stdout, _ = self.RunBazel([ + "cquery", + "--starlark:expr=target.label.name", + "//pkg:main", + ]) + self.assertEquals( + ["main"], + stdout, + ) + + # Check that query fails as it supports the --output flag, but not its + # value. + exit_code, stdout, stderr = self.RunBazel([ + "query", + "//pkg:main", + ], allow_failure = True) + self.AssertExitCode(exit_code, 2, stderr) + self.assertTrue( + any("ERROR: Invalid output format 'starlark'." in line for line in stderr), + stderr, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/src/test/py/bazel/starlark_options_test.py b/src/test/py/bazel/starlark_options_test.py deleted file mode 100644 index 4a22c16d3a2248..00000000000000 --- a/src/test/py/bazel/starlark_options_test.py +++ /dev/null @@ -1,80 +0,0 @@ -# Copyright 2022 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import unittest - -from src.test.py.bazel import test_base - - -class StarlarkOptionsTest(test_base.TestBase): - - def testCanOverrideStarlarkFlagInBazelrcConfigStanza(self): - self.ScratchFile("WORKSPACE.bazel") - self.ScratchFile("bazelrc", [ - "build:red --//f:color=red", - ]) - self.ScratchFile("f/BUILD.bazel", [ - 'load(":f.bzl", "color", "r")', - "color(", - ' name = "color",', - ' build_setting_default = "white",', - ")", - 'r(name = "r")', - ]) - self.ScratchFile("f/f.bzl", [ - 'ColorValue = provider("color")', - "def _color_impl(ctx):", - " return [ColorValue(color = ctx.build_setting_value)]", - "color = rule(", - " implementation = _color_impl,", - "build_setting = config.string(flag = True),", - ")", - "def _r_impl(ctx):", - " print(ctx.attr._color[ColorValue].color)", - " return [DefaultInfo()]", - "r = rule(", - " implementation = _r_impl,", - ' attrs = {"_color": attr.label(default = "//f:color")},', - ")", - ]) - - _, _, stderr = self.RunBazel([ - "--bazelrc=bazelrc", - "build", - "--nobuild", - "//f:r", - "--config=red", - "--//f:color=green", - ]) - self.assertTrue( - any("/f/f.bzl:9:10: green" in line for line in stderr), - "\n".join(stderr), - ) - - _, _, stderr = self.RunBazel([ - "--bazelrc=bazelrc", - "build", - "--nobuild", - "//f:r", - "--//f:color=green", - "--config=red", - ]) - self.assertTrue( - any("/f/f.bzl:9:10: red" in line for line in stderr), - "\n".join(stderr), - ) - - -if __name__ == "__main__": - unittest.main()