commit f272df3908db3cc0aab884a77d0c8cc338abab40
author Googler <gregce@google.com> Fri Feb 03 15:07:37 2023 -0800
committer Copybara-Service <copybara-worker@google.com> Fri Feb 03 15:09:08 2023 -0800
tree 1992dbe54fb20b383db24ee68b27ec656a5892da
parent 3c5f4cc771963b6c7601e16eff27e7482ff583ed

Remove bazelrc-set Starlark flags from BEP's "original" command line event.

If --//foo is set in bar.bazelrc, it shouldn't appear in the "orignal" command line because it's not part of the explicit "$ bazel build //:mybinary .." call. But it should appear in the canonical command line, which expands bazelrc settings.

This isn't the most elegant fix. I added a TODO in the code to explain why, and how further refactoring of the options parsing logic could improve the code.

I extended the test logic to acknowledge the reality that this code uses both OptionsParser and StarkarkOptionsParser. Calling both together is more reflective of reality. But it breaks a few tests that rely on *just* StarkarkOptionsParser being invoked. I added some controls over this, but ideally we could refactor to not have to fork tests this way.

For https://github.com/bazelbuild/bazel/issues/17414.

PiperOrigin-RevId: 507004295
Change-Id: Ia7ecfc10a19d27fee39254b08a2c45910b78a762

src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java
index b42aee4..fea20cc 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java
@@ -104,7 +104,8 @@
 
     OptionsParsingResult result =
         parseStarlarkOptions(
-            "--@starlark_options_test//test:my_int_setting=666 --@repo2//:flag2=222");
+            "--@starlark_options_test//test:my_int_setting=666 --@repo2//:flag2=222",
+            /* onlyStarlarkParser= */ true);
 
     assertThat(result.getStarlarkOptions()).hasSize(2);
     assertThat(result.getStarlarkOptions().get("//test:my_int_setting"))
@@ -161,7 +162,8 @@
     OptionsParsingException e =
         assertThrows(
             OptionsParsingException.class,
-            () -> parseStarlarkOptions("-//test:my_int_setting=666"));
+            () ->
+                parseStarlarkOptions("-//test:my_int_setting=666", /* onlyStarlarkParser= */ true));
     assertThat(e).hasMessageThat().isEqualTo("Invalid options syntax: -//test:my_int_setting=666");
   }