Repository: bazelbuild/bazel
Issue: Transitions cannot read Starlark flags as `@repository_name//:flag_name` in 4.2.0

Bazel 4.2.0 introduced a regression in the handling of Starlark flags: In a transition, the value of such a flag cannot be read if the flag is referred to as `@repository_name//:flag_name` instead of `//:flag_name`, which as far as I know is required for that transition to be usable by other workspaces. Reading the flag does not work even if it is set via both `--@repository_name//:flag_name` and `--//:flag-name` on the commandline.

I noticed this while working on [rules_fuzzing](https://github.com/bazelbuild/rules_fuzzing), whose tests no longer pass with Bazel 4.2.0. Usage of `rules_fuzzing` by other workspaces does not seem to be affected.

I [created a reproducer](https://gist.github.com/fmeum/4e2b5d08aa9a7ed8df39e4362f4bfa3b) that prints 

- `regressed`with
  - Bazel 4.2.0
- `not_regressed` with
  - Bazel 4.0.0
  - Bazel 4.1.0
  - Bazel 5.0.0-pre.20210810.4
  - Bazel 4.2.0 with 80c59dea59 reverted

Unfortunately 98d376faeb206f14838156ce4cb305ddbfce08fa cannot be cleanly cherry-picked on top of Bazel 4.2.0 by itself, so reverting 80c59dea59 appears to be the safest way to mitigate this regression.

_Originally posted by @fmeum in https://github.com/bazelbuild/bazel/issues/13558#issuecomment-903488624_

Issue comments:
Investigating now. Please also file an issue at https://github.com/bazelbuild/continuous-integration/issues/new/choose to have rules_fuzzing added to our downstream CI, to prevent this happening in the future.
This will be fixed in 4.2.1.

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java b/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
index 2fcb17302e5d7f..22ee04ab504a49 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
@@ -109,8 +109,6 @@ public void parse(ExtendedEventHandler eventHandler) throws OptionsParsingExcept
     ImmutableMap.Builder<String, Object> parsedOptions = new ImmutableMap.Builder<>();
     for (Map.Entry<String, Pair<String, Target>> option : unparsedOptions.entrySet()) {
       String loadedFlag = option.getKey();
-      // String loadedFlag =
-      // Label.parseAbsoluteUnchecked(option.getKey()).getDefaultCanonicalForm();
       String unparsedValue = option.getValue().first;
       Target buildSettingTarget = option.getValue().second;
       BuildSetting buildSetting =
@@ -157,11 +155,7 @@ private void parseArg(
     if (value != null) {
       // --flag=value or -flag=value form
       Target buildSettingTarget = loadBuildSetting(name, nativeOptionsParser, 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


Test patch:
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 5a95408ba5e221..01fb80d4397376 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
@@ -45,40 +45,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();
   }