bazelbuild · iancha1992 · May 24, 2023
@@ -103,7 +103,14 @@ 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 should be applied to all Bazel commands that support
+    them. If a command does not support an option specified in this way, the
+    option is ignored so long as it is valid for *some* other Bazel command.
+    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.
+-   `always`: options that apply to all Bazel commands. If a command does not
+    support an option specified in this way, 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`.)

@@ -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;
@@ -35,6 +37,7 @@
 import com.google.devtools.common.options.InvocationPolicyEnforcer;
 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 +69,14 @@ public final class BlazeOptionHandler {
           "client_env",
           "client_cwd");
 
+  // All options set on this pseudo command are inherited by all commands, with unrecognized options
+  // resulting in an error.
+  private static final String ALWAYS_PSEUDO_COMMAND = "always";
+
+  // All options set on this pseudo command are inherited by all commands, with unrecognized options
+  // being ignored as long as they are recognized by at least one (other) command.
+  private static final String COMMON_PSEUDO_COMMAND = "common";
+
   // 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 +89,7 @@ public final class BlazeOptionHandler {
   private final Command commandAnnotation;
   private final InvocationPolicy invocationPolicy;
   private final List<String> rcfileNotes = new ArrayList<>();
+  private final ImmutableList<Class<? extends OptionsBase>> allOptionsClasses;
 
   BlazeOptionHandler(
       BlazeRuntime runtime,
@@ -92,6 +104,16 @@ 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 +213,36 @@ 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(COMMON_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 "common"
+          // pseudo command can be parsed unambiguously.
+          ImmutableList<String> ignoredArgs =
+              optionsParser.parseWithSourceFunction(
+                  PriorityCategory.RC_FILE,
+                  o -> rcArgs.getRcFile(),
+                  rcArgs.getArgs(),
+                  OptionsParser.getFallbackOptionsData(allOptionsClasses));
+          if (!ignoredArgs.isEmpty()) {
+            // Append richer information to the note.
+            int index = rcfileNotes.size() - 1;
+            String note = rcfileNotes.get(index);
+            note +=
+                String.format(
+                    "\n  Ignored as unsupported by '%s': %s",
+                    commandAnnotation.name(), Joiner.on(' ').join(ignoredArgs));
+            rcfileNotes.set(index, note);
+          }
+        } else {
+          optionsParser.parse(PriorityCategory.RC_FILE, rcArgs.getRcFile(), rcArgs.getArgs());
+        }
       }
     }
   }
@@ -227,7 +278,8 @@ private void parseArgsAndConfigs(List<String> args, ExtendedEventHandler eventHa
     optionsParser.parseWithSourceFunction(
         PriorityCategory.COMMAND_LINE,
         commandOptionSourceFunction,
-        defaultOverridesAndRcSources.build());
+        defaultOverridesAndRcSources.build(),
+        /* fallbackData= */ null);
 
     // Command-specific options from .blazerc passed in via --default_override and --rc_source.
     ClientOptions rcFileOptions = optionsParser.getOptions(ClientOptions.class);
@@ -241,7 +293,10 @@ 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(),
+        /* fallbackData= */ null);
 
     if (commandAnnotation.builds()) {
       // splits project files from targets in the traditional sense
@@ -372,14 +427,17 @@ void expandConfigOptions(
     ConfigExpander.expandConfigOptions(
         eventHandler,
         commandToRcArgs,
+        commandAnnotation.name(),
         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(ALWAYS_PSEUDO_COMMAND);
+    result.add(COMMON_PSEUDO_COMMAND);
     getCommandNamesToParseHelper(commandAnnotation, result);
     return result;
   }
@@ -470,7 +528,9 @@ 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(ALWAYS_PSEUDO_COMMAND)
+          && !command.equals(COMMON_PSEUDO_COMMAND)) {
         eventHandler.handle(
             Event.warn(
                 "while reading option defaults file '"

@@ -1110,7 +1110,8 @@ 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, /* fallbackData= */ null);
     return parser;
   }
 

@@ -13,12 +13,14 @@
 // limitations under the License.
 package com.google.devtools.build.lib.runtime;
 
+import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
 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;
@@ -29,6 +31,7 @@
 import java.util.List;
 import java.util.Set;
 import java.util.function.Consumer;
+import javax.annotation.Nullable;
 
 /** Encapsulates logic for performing --config option expansion. */
 final class ConfigExpander {
@@ -86,9 +89,11 @@ private static boolean shouldEnablePlatformSpecificConfig(
   static void expandConfigOptions(
       EventHandler eventHandler,
       ListMultimap<String, RcChunkOfArgs> commandToRcArgs,
+      String currentCommand,
       List<String> commandsToParse,
       Consumer<String> rcFileNotesConsumer,
-      OptionsParser optionsParser)
+      OptionsParser optionsParser,
+      @Nullable OpaqueOptionsData fallbackData)
       throws OptionsParsingException {
 
     OptionValueDescription configValueDescription =
@@ -110,8 +115,18 @@ static void expandConfigOptions(
                 commandsToParse,
                 configValueToExpand,
                 rcFileNotesConsumer);
-        optionsParser.parseArgsAsExpansionOfOption(
-            configInstance, String.format("expanded from --%s", configValueToExpand), expansion);
+        var ignoredArgs =
+            optionsParser.parseArgsAsExpansionOfOption(
+                configInstance,
+                String.format("expanded from --config=%s", configValueToExpand),
+                expansion,
+                fallbackData);
+        if (!ignoredArgs.isEmpty()) {
+          rcFileNotesConsumer.accept(
+              String.format(
+                  "Ignored as unsupported by '%s': %s",
+                  currentCommand, Joiner.on(' ').join(ignoredArgs)));
+        }
       }
     }
 
@@ -129,7 +144,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

@@ -46,6 +46,7 @@ java_library(
         ],
     ),
     deps = [
+        "//src/main/java/com/google/devtools/build/lib/util:pair",
         "//third_party:auto_value",
         "//third_party:caffeine",
         "//third_party:guava",