Improve the warning for malformed recursive configs.

Unlike in fixed-point (legacy) expansion of configs, with --expand_configs_in_place we do not use the options parser to parse each config-definition default override - we first find the full expansion and then expand it in place of the original --config=value instance. For this reason though, we don't support space-separation of recursive configs and their values.

The old warning for this was confusing though, and did not provide much guidance. This should be better, now the warning specifies which config is malformed, in what file, and that it expects the "=" character.

RELNOTES: None.
PiperOrigin-RevId: 188509060

2 files changed, 55 insertions, 1 deletions

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 c7c72ec77a..812854a84e 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
@@ -541,7 +541,13 @@ public abstract class BlazeOptionHandler {
               // we will only accept --config=value, and will not accept value on a following line.
               int charOfConfigValue = arg.indexOf('=');
               if (charOfConfigValue < 0) {
-                throw new OptionsParsingException("Config flag was provided without a value.");
+                throw new OptionsParsingException(
+                    String.format(
+                        "In file %s, the definition of config %s expands to another config "
+                            + "that either has no value or is not in the form --config=value. For "
+                            + "recursive config definitions, please do not provide the value in a "
+                            + "separate token, such as in the form '--config value'.",
+                        rcArgs.rcFile, configToExpand));
               }
               String newConfigValue = arg.substring(charOfConfigValue + 1);
               LinkedHashSet<String> extendedConfigAncestorSet =
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BlazeOptionHandlerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/BlazeOptionHandlerTest.java
index 2efebed15d..567499ca7d 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/BlazeOptionHandlerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/BlazeOptionHandlerTest.java
@@ -945,6 +945,54 @@ public class BlazeOptionHandlerTest {
         .inOrder();
   }
 
+  private void testParseOptions_recursiveConfigWithDifferentTokens() {
+    optionHandler.parseOptions(
+        ImmutableList.of(
+            "c0",
+            "--default_override=0:c0=--test_multiple_string=rc",
+            "--default_override=0:c0:other=--test_multiple_string=other",
+            "--default_override=0:c0:conf=--test_multiple_string=config1",
+            "--default_override=0:c0:conf=--config",
+            "--default_override=0:c0:conf=other",
+            "--rc_source=/somewhere/.blazerc",
+            "--config=conf"),
+        eventHandler);
+  }
+
+  @Test
+  public void testParseOptions_recursiveConfigWithDifferentTokens_fixedPoint() {
+    makeFixedPointExpandingConfigOptionHandler();
+    testParseOptions_recursiveConfigWithDifferentTokens();
+    assertThat(eventHandler.getEvents()).isEmpty();
+    assertThat(parser.getResidue()).isEmpty();
+    assertThat(optionHandler.getRcfileNotes())
+        .containsExactly(
+            "Reading rc options for 'c0' from /somewhere/.blazerc:\n"
+                + "  'c0' options: --test_multiple_string=rc",
+            "Found applicable config definition c0:conf in file /somewhere/.blazerc: "
+                + "--test_multiple_string=config1 --config other",
+            "Found applicable config definition c0:other in file /somewhere/.blazerc: "
+                + "--test_multiple_string=other");
+
+    // The 2nd config, --config other, is expanded after the config that added it.
+    TestOptions options = parser.getOptions(TestOptions.class);
+    assertThat(options).isNotNull();
+    assertThat(options.testMultipleString).containsExactly("rc", "config1", "other").inOrder();
+  }
+
+  @Test
+  public void testParseOptions_recursiveConfigWithDifferentTokens_inPlace() {
+    makeInPlaceExpandingConfigOptionHandler();
+    testParseOptions_recursiveConfigWithDifferentTokens();
+    assertThat(eventHandler.getEvents())
+        .containsExactly(
+            Event.error(
+                "In file /somewhere/.blazerc, the definition of config conf expands to another "
+                    + "config that either has no value or is not in the form --config=value. For "
+                    + "recursive config definitions, please do not provide the value in a "
+                    + "separate token, such as in the form '--config value'."));
+  }
+
   private void parseComplexConfigOrderCommandLine() {
     optionHandler.parseOptions(
         ImmutableList.of(