PROJECT.scl: support "my_scl_config": ["--config=foo"]

### Notes
 - Only supports `--configs` defined in global rc files, as determined by `GlobalRcUtils`
 - We should only support `--config`s that only set `BuildOptions` flags. I left that part as a TODO for this change.
 - This is not meant to be permanent. We ideally want the reverse: `--scl_config` is the source of truth and `--config`s can set `--scl_config`. The current approach supports practical migration since bazerlcs are already sources of truth, and we don't want to further *fork* sources of truth.

### Implementation details

I originally tried creating a dedicated `OptionsParser`, parsing `--config=foo`, then reading the results. I eventually realized this fights Bazel's option parsing model, which expects `--config=foo` parses once, at the beginning of a build, with access to core runtime objects.

`OptionsParser` doesn't even handle `--config`. `BlazeOptionHandler` and `ConfigExpander` process it and feed results into `OptionsParser`. That logic has strange expectations that are hard to replicate.

IMO the current approach is much cleaner. I made a point to hide all implementation details from any code not in .lib.runtime`. There's no need to export that complexity.

PiperOrigin-RevId: 709145745
Change-Id: I1cc946bc83b273491a148855b6ec427cf6b3c236

Author: gregestren

src/main/java/com/google/devtools/build/lib/BUILD

@@ -281,6 +281,18 @@ java_library(
     ],
 )
 
+java_library(
+    name = "runtime/config_flag_definitions",
+    srcs = [
+        "runtime/ConfigFlagDefinitions.java",
+    ],
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
+        "//src/main/java/com/google/devtools/common/options",
+        "//third_party:guava",
+    ],
+)
+
 java_library(
     name = "runtime",
     srcs = glob(
@@ -295,6 +307,7 @@ java_library(
             "runtime/BlazeCommandResult.java",
             "runtime/CommandDispatcher.java",
             "runtime/CommandLinePathFactory.java",
+            "runtime/ConfigFlagDefinitions.java",
             "runtime/GcThrashingDetector.java",
             "runtime/KeepGoingOption.java",
             "runtime/LoadingPhaseThreadsOption.java",
@@ -313,6 +326,7 @@ java_library(
         ":runtime/blaze_command_result",
         ":runtime/command_dispatcher",
         ":runtime/command_line_path_factory",
+        ":runtime/config_flag_definitions",
         ":runtime/memory_pressure",
         ":runtime/test_summary_options",
         ":starlark_options_parser",

src/main/java/com/google/devtools/build/lib/analysis/BUILD

@@ -615,6 +615,7 @@ java_library(
         ":config/core_options",
         ":config/invalid_configuration_exception",
         ":project_resolution_exception",
+        "//src/main/java/com/google/devtools/build/lib:runtime/config_flag_definitions",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/skyframe:project_files_lookup_value",

src/main/java/com/google/devtools/build/lib/analysis/Project.java

@@ -32,6 +32,7 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.runtime.ConfigFlagDefinitions;
 import com.google.devtools.build.lib.server.FailureDetails.BuildConfiguration.Code;
 import com.google.devtools.build.lib.skyframe.ProjectFilesLookupValue;
 import com.google.devtools.build.lib.skyframe.ProjectValue;
@@ -277,6 +278,7 @@ public static FlagSetValue modifyBuildOptionsWithFlagSets(
       Label projectFile,
       BuildOptions targetOptions,
       ImmutableMap<String, String> userOptions,
+      ConfigFlagDefinitions configFlagDefinitions,
       boolean enforceCanonicalConfigs,
       ExtendedEventHandler eventHandler,
       SkyframeExecutor skyframeExecutor)
@@ -288,6 +290,7 @@ public static FlagSetValue modifyBuildOptionsWithFlagSets(
             targetOptions.get(CoreOptions.class).sclConfig,
             targetOptions,
             userOptions,
+            configFlagDefinitions,
             enforceCanonicalConfigs);
 
     EvaluationResult<SkyValue> result =

src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java

@@ -280,6 +280,7 @@ static ProjectEvaluationResult evaluateProjectFile(
           BuildTool.applySclConfigs(
               buildOptions,
               userOptions,
+              env.getConfigFlagDefinitions(),
               projectFile,
               request.getBuildOptions().enforceProjectConfigs,
               env.getSkyframeExecutor(),

src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java

@@ -75,6 +75,7 @@
 import com.google.devtools.build.lib.runtime.BlazeRuntime;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.runtime.CommandLineEvent.CanonicalCommandLineEvent;
+import com.google.devtools.build.lib.runtime.ConfigFlagDefinitions;
 import com.google.devtools.build.lib.server.FailureDetails.ActionQuery;
 import com.google.devtools.build.lib.server.FailureDetails.BuildConfiguration.Code;
 import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
@@ -1016,6 +1017,7 @@ public static PathFragmentPrefixTrie getWorkingSetMatcherForSkyfocus(
   public static ImmutableSet<String> applySclConfigs(
       BuildOptions buildOptionsBeforeFlagSets,
       ImmutableMap<String, String> userOptions,
+      ConfigFlagDefinitions configFlagDefinitions,
       Label projectFile,
       boolean enforceCanonicalConfigs,
       SkyframeExecutor skyframeExecutor,
@@ -1027,6 +1029,7 @@ public static ImmutableSet<String> applySclConfigs(
             projectFile,
             buildOptionsBeforeFlagSets,
             userOptions,
+            configFlagDefinitions,
             enforceCanonicalConfigs,
             eventHandler,
             skyframeExecutor);

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

@@ -49,6 +49,7 @@
 import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.profiler.SilentCloseable;
+import com.google.devtools.build.lib.runtime.BlazeOptionHandler.DetailedParseResults;
 import com.google.devtools.build.lib.runtime.InstrumentationOutputFactory.DestinationRelativeTo;
 import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
 import com.google.devtools.build.lib.server.FailureDetails;
@@ -339,11 +340,12 @@ private BlazeCommandResult execExclusively(
     BlazeOptionHandler optionHandler =
         new BlazeOptionHandler(
             runtime, workspace, command, commandAnnotation, optionsParser, invocationPolicy);
-    DetailedExitCode earlyExitCode =
-        optionHandler.parseOptions(
+    DetailedParseResults parseResults =
+        optionHandler.parseOptionsAndGetConfigDefinitions(
             args,
             storedEventHandler,
             /* invocationPolicyFlagListBuilder= */ ImmutableList.builder());
+    DetailedExitCode earlyExitCode = parseResults.detailedExitCode();
     OptionsParsingResult options = optionHandler.getOptionsResult();
 
     // The initCommand call also records the start time for the timestamp granularity monitor.
@@ -359,7 +361,8 @@ private BlazeCommandResult execExclusively(
             commandExtensions,
             this::setShutdownReason,
             commandExtensionReporter,
-            attemptNumber);
+            attemptNumber,
+            parseResults.configFlagDefinitions());
 
     if (!attemptedCommandIds.isEmpty()) {
       if (attemptedCommandIds.contains(env.getCommandId())) {

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

@@ -20,6 +20,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
@@ -29,6 +30,7 @@
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.runtime.ConfigFlagDefinitions.ConfigDefinition;
 import com.google.devtools.build.lib.runtime.StarlarkOptionsParser.BuildSettingLoader;
 import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
 import com.google.devtools.build.lib.server.FailureDetails;
@@ -245,7 +247,13 @@ void parseRcOptions(
     }
   }
 
-  private void parseArgsAndConfigs(List<String> args, ExtendedEventHandler eventHandler)
+  /**
+   * Returns a map from rc file definitions to the options they define and which rc files defined.
+   * them. For example, "build:asan" keys a {@code --config=asan} definition and "build" keys
+   * options that apply to all build commands.
+   */
+  private ListMultimap<String, RcChunkOfArgs> parseArgsAndConfigs(
+      List<String> args, ExtendedEventHandler eventHandler)
       throws OptionsParsingException, InterruptedException, AbruptExitException {
     Path workspaceDirectory = workspace.getWorkspace();
     // TODO(ulfjack): The working directory is passed by the client as part of CommonCommandOptions,
@@ -308,6 +316,7 @@ private void parseArgsAndConfigs(List<String> args, ExtendedEventHandler eventHa
     }
 
     expandConfigOptions(eventHandler, commandToRcArgs);
+    return commandToRcArgs;
   }
 
   /**
@@ -423,6 +432,10 @@ DetailedExitCode parseStarlarkOptions(CommandEnvironment env) {
     return DetailedExitCode.success();
   }
 
+  /** Detailed parsing results: exit code and {@code --config=foo} definitions. */
+  static record DetailedParseResults(
+      DetailedExitCode detailedExitCode, ConfigFlagDefinitions configFlagDefinitions) {}
+
   /**
    * Parses the options, taking care not to generate any output to outErr, return, or throw an
    * exception.
@@ -433,29 +446,51 @@ DetailedExitCode parseOptions(
       List<String> args,
       ExtendedEventHandler eventHandler,
       ImmutableList.Builder<OptionAndRawValue> invocationPolicyFlagListBuilder) {
-    DetailedExitCode result =
-        parseOptionsInternal(args, eventHandler, invocationPolicyFlagListBuilder);
-    if (!result.isSuccess()) {
+    DetailedParseResults result =
+        parseOptionsInternal(
+            args, eventHandler, invocationPolicyFlagListBuilder, /* getConfigDefinitions= */ false);
+    if (!result.detailedExitCode.isSuccess()) {
       optionsParser.setError();
     }
-    return result;
+    return result.detailedExitCode;
   }
 
-  private DetailedExitCode parseOptionsInternal(
+  /**
+   * {@link #parseOptions} variation that also returns {@code --config=foo} definitions. Callers can
+   * use this to determine which flags {@code --config=foo} sets.
+   */
+  DetailedParseResults parseOptionsAndGetConfigDefinitions(
       List<String> args,
       ExtendedEventHandler eventHandler,
       ImmutableList.Builder<OptionAndRawValue> invocationPolicyFlagListBuilder) {
+    DetailedParseResults result =
+        parseOptionsInternal(
+            args, eventHandler, invocationPolicyFlagListBuilder, /* getConfigDefinitions= */ true);
+    if (!result.detailedExitCode.isSuccess()) {
+      optionsParser.setError();
+    }
+    return result;
+  }
+
+  private DetailedParseResults parseOptionsInternal(
+      List<String> args,
+      ExtendedEventHandler eventHandler,
+      ImmutableList.Builder<OptionAndRawValue> invocationPolicyFlagListBuilder,
+      boolean getConfigDefinitions) {
     // The initialization code here was carefully written to parse the options early before we call
     // into the BlazeModule APIs, which means we must not generate any output to outErr, return, or
     // throw an exception. All the events happening here are instead stored in a temporary event
     // handler, and later replayed.
     DetailedExitCode earlyExitCode = checkCwdInWorkspace(eventHandler);
     if (!earlyExitCode.isSuccess()) {
-      return earlyExitCode;
+      return new DetailedParseResults(
+          earlyExitCode, new ConfigFlagDefinitions(ImmutableListMultimap.of()));
     }
 
+    ListMultimap<String, RcChunkOfArgs> rcDefinitions = ImmutableListMultimap.of();
+    DetailedExitCode exitCode;
     try {
-      parseArgsAndConfigs(args, eventHandler);
+      rcDefinitions = parseArgsAndConfigs(args, eventHandler);
       // Allow the command to edit the options.
       command.editOptions(optionsParser);
       // Merge the invocation policy that is user-supplied, from the command line, and any
@@ -485,22 +520,49 @@ private DetailedExitCode parseOptionsInternal(
       for (String warning : commonOptions.deprecationWarnings) {
         eventHandler.handle(Event.warn(warning));
       }
+      exitCode = DetailedExitCode.success();
     } catch (OptionsParsingException e) {
       String logMessage = "Error parsing options";
       logger.atInfo().withCause(e).log("%s", logMessage);
-      return processOptionsParsingException(
-          eventHandler, e, logMessage, Code.OPTIONS_PARSE_FAILURE);
+      exitCode =
+          processOptionsParsingException(eventHandler, e, logMessage, Code.OPTIONS_PARSE_FAILURE);
     } catch (InterruptedException e) {
-      return DetailedExitCode.of(
-          FailureDetail.newBuilder()
-              .setInterrupted(
-                  FailureDetails.Interrupted.newBuilder()
-                      .setCode(FailureDetails.Interrupted.Code.INTERRUPTED))
-              .build());
+      exitCode =
+          DetailedExitCode.of(
+              FailureDetail.newBuilder()
+                  .setInterrupted(
+                      FailureDetails.Interrupted.newBuilder()
+                          .setCode(FailureDetails.Interrupted.Code.INTERRUPTED))
+                  .build());
     } catch (AbruptExitException e) {
-      return e.getDetailedExitCode();
+      exitCode = e.getDetailedExitCode();
     }
-    return DetailedExitCode.success();
+
+    if (!getConfigDefinitions) {
+      return new DetailedParseResults(
+          exitCode, new ConfigFlagDefinitions(ImmutableListMultimap.of()));
+    }
+    // Transforms all rc definitions into valid --config definitions for this command. For example,
+    // "build:asan" is valid for a build command but not "test:asan". Rc definitions like "build"
+    // aren't included because those aren't --config definitions.
+    var validConfigDefs = new ImmutableListMultimap.Builder<String, ConfigDefinition>();
+    List<String> matchingRcCommands = getCommandNamesToParse(commandAnnotation);
+    for (var entry : rcDefinitions.entries()) {
+      String rcKey = entry.getKey();
+      int firstColon = rcKey.indexOf(":");
+      if (firstColon == -1) {
+        continue;
+      }
+      String cmd = rcKey.substring(0, firstColon);
+      String configName = rcKey.substring(firstColon + 1);
+      if (matchingRcCommands.contains(cmd)) {
+        validConfigDefs.put(
+            configName,
+            new ConfigFlagDefinitions.ConfigDefinition(
+                ImmutableList.copyOf(entry.getValue().getArgs()), entry.getValue().getRcFile()));
+      }
+    }
+    return new DetailedParseResults(exitCode, new ConfigFlagDefinitions(validConfigDefs.build()));
   }
 
   /**

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

@@ -233,7 +233,8 @@ public CommandEnvironment initCommand(
       List<Any> commandExtensions,
       Consumer<String> shutdownReasonConsumer,
       CommandExtensionReporter commandExtensionReporter,
-      int attemptNumber) {
+      int attemptNumber,
+      ConfigFlagDefinitions configFlagDefinitions) {
     quiescingExecutors.resetParameters(options);
     CommandEnvironment env =
         new CommandEnvironment(
@@ -253,7 +254,8 @@ public CommandEnvironment initCommand(
             commandExtensions,
             shutdownReasonConsumer,
             commandExtensionReporter,
-            attemptNumber);
+            attemptNumber,
+            configFlagDefinitions);
     skyframeExecutor.setClientEnv(env.getClientEnv());
     BuildRequestOptions buildRequestOptions = options.getOptions(BuildRequestOptions.class);
     if (buildRequestOptions != null && !buildRequestOptions.useActionCache) {

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

@@ -100,6 +100,7 @@ public class CommandEnvironment {
   private final BlazeRuntime runtime;
   private final BlazeWorkspace workspace;
   private final BlazeDirectories directories;
+  private final ConfigFlagDefinitions configFlagDefinitions;
 
   private final UUID commandId; // Unique identifier for the command being run
   private final String buildRequestId; // Unique identifier for the build being run
@@ -223,7 +224,8 @@ public void exit(AbruptExitException exception) {
       List<Any> commandExtensions,
       Consumer<String> shutdownReasonConsumer,
       CommandExtensionReporter commandExtensionReporter,
-      int attemptNumber) {
+      int attemptNumber,
+      ConfigFlagDefinitions configFlagDefinitions) {
     checkArgument(attemptNumber >= 1);
 
     this.runtime = runtime;
@@ -243,6 +245,8 @@ public void exit(AbruptExitException exception) {
     this.blazeModuleEnvironment = new BlazeModuleEnvironment();
     this.timestampGranularityMonitor = new TimestampGranularityMonitor(runtime.getClock());
     this.attemptNumber = attemptNumber;
+    this.configFlagDefinitions = configFlagDefinitions;
+
     // Record the command's starting time again, for use by
     // TimestampGranularityMonitor.waitForTimestampGranularity().
     // This should be done as close as possible to the start of
@@ -462,6 +466,11 @@ public OptionsParsingResult getOptions() {
     return options;
   }
 
+  /** {@code --config} definitions for this invocation. */
+  public ConfigFlagDefinitions getConfigFlagDefinitions() {
+    return configFlagDefinitions;
+  }
+
   public InvocationPolicy getInvocationPolicy() {
     return invocationPolicy;
   }