From 3719fca71fe3a37eac1843cc44d4dd57c97f1f4e Mon Sep 17 00:00:00 2001
From: Luca Di Grazia <luca.digrazia94@gmail.com>
Date: Sun, 4 Sep 2022 16:44:22 +0200
Subject: [PATCH]     Respect Starlark options with values in
 `removeStarlarkOptions()`

    Related: https://github.com/bazelbuild/bazel/issues/11301
    (Partially fixed in https://github.com/bazelbuild/bazel/commit/2ec58f60ae1732f4c1d09330c4583050ac8d1f46)

    Motivation:

    `StarlarkOptionsParser.removeStarlarkOptions()` doesn't take the case
    into account where the specified Starlark option has a value, e.g.
    `--//my_rules/custom_flags:foo=bar`.

    Modifications:

    - Do not pass a Starlark flag value when validating the flag name.

    Result:

    `bazel info` does not fail anymore when `.bazelrc` contains a statement
    like the following:

        build --//my_rules/custom_flags:foo=bar

    Closes #12648.

    PiperOrigin-RevId: 348676049
---
 .../lib/runtime/StarlarkOptionsParser.java    | 10 +-----
 .../starlark/StarlarkOptionsParsingTest.java  | 32 +++----------------
 2 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
index 21104ee7264..13af8bd59e2 100644
--- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
+++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
@@ -151,11 +151,7 @@ public void parse(ExtendedEventHandler eventHandler) throws OptionsParsingExcept
     // Map of flag label as a string to its loaded target and set value after parsing.
     HashMap<String, Pair<Target, Object>> buildSettingWithTargetAndValue = new HashMap<>();
     for (Map.Entry<String, Pair<String, Target>> option : unparsedOptions.entries()) {
-      // These are already in umambiguous canonical form - this just turns main repo
-      // label from @//myflag -> //myflag since that's how users are used to seeing this label.
       String loadedFlag = option.getKey();
-      // String loadedFlag =
-      // Label.parseAbsoluteUnchecked(option.getKey()).getDefaultCanonicalForm();
       String unparsedValue = option.getValue().first;
       Target buildSettingTarget = option.getValue().second;
       BuildSetting buildSetting =
@@ -242,11 +238,7 @@ private void parseArg(
     if (value != null) {
       // --flag=value or -flag=value form
       Target buildSettingTarget = loadBuildSetting(name, eventHandler);
-      // Use the unambiguous canonical form to ensure we don't have
-      // duplicate options getting into the starlark options map.
-      unparsedOptions.put(
-          buildSettingTarget.getLabel().getDefaultCanonicalForm(),
-          new Pair<>(value, buildSettingTarget));
+      unparsedOptions.put(name, new Pair<>(value, buildSettingTarget));
     } else {
       boolean booleanValue = true;
       // check --noflag form
diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java
index 4b3a05a30f0..3bd9fb6ac01 100644
--- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java
+++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java
@@ -78,40 +78,18 @@ public void testFlagEqualsValueForm() throws Exception {
     assertThat(result.getResidue()).isEmpty();
   }
 
-  // test --@main_workspace//flag=value parses out to //flag=value
-  // test --@other_workspace//flag=value parses out to @other_workspace//flag=value
+  // test --@workspace//flag=value
   @Test
   public void testFlagNameWithWorkspace() throws Exception {
     writeBasicIntFlag();
-    scratch.file("test/repo2/WORKSPACE");
-    scratch.file(
-        "test/repo2/defs.bzl",
-        "def _impl(ctx):",
-        "  pass",
-        "my_flag = rule(",
-        "  implementation = _impl,",
-        "  build_setting = config.int(flag = True),",
-        ")");
-    scratch.file(
-        "test/repo2/BUILD",
-        "load(':defs.bzl', 'my_flag')",
-        "my_flag(name = 'flag2', build_setting_default=2)");
-
-    rewriteWorkspace(
-        "workspace(name = 'starlark_options_test')",
-        "local_repository(",
-        "  name = 'repo2',",
-        "  path = 'test/repo2',",
-        ")");
+    rewriteWorkspace("workspace(name = 'starlark_options_test')");
 
     OptionsParsingResult result =
-        parseStarlarkOptions(
-            "--@starlark_options_test//test:my_int_setting=666 --@repo2//:flag2=222");
+        parseStarlarkOptions("--@starlark_options_test//test:my_int_setting=666");
 
-    assertThat(result.getStarlarkOptions()).hasSize(2);
-    assertThat(result.getStarlarkOptions().get("//test:my_int_setting"))
+    assertThat(result.getStarlarkOptions()).hasSize(1);
+    assertThat(result.getStarlarkOptions().get("@starlark_options_test//test:my_int_setting"))
         .isEqualTo(StarlarkInt.of(666));
-    assertThat(result.getStarlarkOptions().get("@repo2//:flag2")).isEqualTo(StarlarkInt.of(222));
     assertThat(result.getResidue()).isEmpty();
   }