Change config expansion application order, gated by startup flag --expand_configs_in_place.

--config options were expanded in a fix-point expansion, where in practice, the flags that --config values expanded to ended up between the normal bazelrc options and the command line's explicit options. Since the options parser has an order-based priority scheme and it accepts multiple mentions of a single-valued option, this conflicts with users' expectations of being able to override these config expansions by using the order in which they are mentioned.

This change makes it possible to expand the config values defined in your bazelrc (or blazerc) files to occur in-place: --stuff --config=something --laterstuff will interpret the options that --config=something expands to as if they had been mentioned explicitly between --stuff and --laterstuff.

In order to not break users relying on complex flag combinations to configure their builds, this behavior will not yet be turned on by default. Instead, use --expand_configs_in_place as a startup flag to test this feature. --announce_rc may be helpful for debugging any differences between the fixed point and in-place expansions. Once you've debugged your problems, add "startup --expand_configs_in_place" to your blazerc to stick to the new behavior.

RELNOTES: Use --expand_configs_in_place as a startup argument to change the order in which --config expansions are interpreted.
PiperOrigin-RevId: 176371289

3 files changed, 31 insertions, 12 deletions

diff --git a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
index 9864a46945..bb6f24266d 100644
--- a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
+++ b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
@@ -81,7 +81,7 @@ public abstract class OptionValueDescription {
    * and is null.
    */
   @Nullable
-  abstract List<ParsedOptionDescription> getCanonicalInstances();
+  public abstract List<ParsedOptionDescription> getCanonicalInstances();
 
   /**
    * For the given option, returns the correct type of OptionValueDescription, to which unparsed
@@ -137,7 +137,7 @@ public abstract class OptionValueDescription {
     }
 
     @Override
-    ImmutableList<ParsedOptionDescription> getCanonicalInstances() {
+    public ImmutableList<ParsedOptionDescription> getCanonicalInstances() {
       return null;
     }
   }
@@ -247,7 +247,7 @@ public abstract class OptionValueDescription {
     }
 
     @Override
-    ImmutableList<ParsedOptionDescription> getCanonicalInstances() {
+    public ImmutableList<ParsedOptionDescription> getCanonicalInstances() {
       // If the current option is an implicit requirement, we don't need to list this value since
       // the parent implies it. In this case, it is sufficient to not list this value at all.
       if (effectiveOptionInstance.getImplicitDependent() == null) {
@@ -315,7 +315,7 @@ public abstract class OptionValueDescription {
     }
 
     @Override
-    ImmutableList<ParsedOptionDescription> getCanonicalInstances() {
+    public ImmutableList<ParsedOptionDescription> getCanonicalInstances() {
       return parsedOptions
           .asMap()
           .entrySet()
@@ -375,7 +375,7 @@ public abstract class OptionValueDescription {
     }
 
     @Override
-    ImmutableList<ParsedOptionDescription> getCanonicalInstances() {
+    public ImmutableList<ParsedOptionDescription> getCanonicalInstances() {
       // The options this expands to are incorporated in their own right - this option does
       // not have a canonical form.
       return ImmutableList.of();
@@ -464,7 +464,7 @@ public abstract class OptionValueDescription {
     }
 
     @Override
-    ImmutableList<ParsedOptionDescription> getCanonicalInstances() {
+    public ImmutableList<ParsedOptionDescription> getCanonicalInstances() {
       // No wrapper options get listed in the canonical form - the options they are wrapping will
       // be in the right place.
       return ImmutableList.of();
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 4164e7c755..fb7161ca46 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -549,7 +549,7 @@ public class OptionsParser implements OptionsProvider {
    *     or null if the value has not been set.
    * @throws IllegalArgumentException if there is no option by the given name.
    */
-  OptionValueDescription getOptionValueDescription(String name) {
+  public OptionValueDescription getOptionValueDescription(String name) {
     return impl.getOptionValueDescription(name);
   }
 
@@ -619,6 +619,19 @@ public class OptionsParser implements OptionsProvider {
     }
   }
 
+  public void parseOptionsFixedAtSpecificPriority(
+      OptionPriority priority, String source, List<String> args) throws OptionsParsingException {
+    Preconditions.checkNotNull(priority, "Priority not specified for arglist " + args);
+    Preconditions.checkArgument(
+        priority.getPriorityCategory() != OptionPriority.PriorityCategory.DEFAULT,
+        "Priority cannot be default, which was specified for arglist " + args);
+    residue.addAll(impl.parseOptionsFixedAtSpecificPriority(priority, o -> source, args));
+    if (!allowResidue && !residue.isEmpty()) {
+      String errorMsg = "Unrecognized arguments: " + Joiner.on(' ').join(residue);
+      throw new OptionsParsingException(errorMsg);
+    }
+  }
+
   /**
    * @param origin the origin of this option instance, it includes the priority of the value. If
    *     other values have already been or will be parsed at a higher priority, they might override
@@ -652,9 +665,7 @@ public class OptionsParser implements OptionsProvider {
     return ImmutableList.copyOf(residue);
   }
 
-  /**
-   * Returns a list of warnings about problems encountered by previous parse calls.
-   */
+  /** Returns a list of warnings about problems encountered by previous parse calls. */
   public List<String> getWarnings() {
     return impl.getWarnings();
   }
@@ -792,8 +803,7 @@ public class OptionsParser implements OptionsProvider {
    * Option} annotation.
    */
   private static void validateFieldsSets(
-      Class<? extends OptionsBase> optionsClass,
-      LinkedHashSet<Field> fieldsFromMap) {
+      Class<? extends OptionsBase> optionsClass, LinkedHashSet<Field> fieldsFromMap) {
     ImmutableList<OptionDefinition> optionDefsFromClasses =
         OptionsData.getAllOptionDefinitionsForClass(optionsClass);
     Set<Field> fieldsFromClass =
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 d89aad302c..496927b7c3 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -284,6 +284,15 @@ class OptionsParserImpl {
     }
   }
 
+  /** Parses the args at the fixed priority. */
+  List<String> parseOptionsFixedAtSpecificPriority(
+      OptionPriority priority, Function<OptionDefinition, String> sourceFunction, List<String> args)
+      throws OptionsParsingException {
+    ResidueAndPriority residueAndPriority =
+        parse(OptionPriority.getLockedPriority(priority), sourceFunction, null, null, args);
+    return residueAndPriority.residue;
+  }
+
   /**
    * Parses the args, and returns what it doesn't parse. May be called multiple times, and may be
    * called recursively. Calls may contain intersecting sets of options; in that case, the arg seen