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 096cac98281c0d..6c94eb794a80bf 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,6 +109,8 @@ public void parse(ExtendedEventHandler eventHandler) throws OptionsParsingExcept ImmutableMap.Builder parsedOptions = new ImmutableMap.Builder<>(); for (Map.Entry> 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 = @@ -155,7 +157,11 @@ private void parseArg( if (value != null) { // --flag=value or -flag=value form Target buildSettingTarget = loadBuildSetting(name, nativeOptionsParser, eventHandler); - unparsedOptions.put(name, new Pair<>(value, buildSettingTarget)); + // 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)); } else { boolean booleanValue = true; // check --noflag form 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 994004adacad3d..481d55a14435f6 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,18 +45,40 @@ public void testFlagEqualsValueForm() throws Exception { assertThat(result.getResidue()).isEmpty(); } - // test --@workspace//flag=value + // test --@main_workspace//flag=value parses out to //flag=value + // test --@other_workspace//flag=value parses out to @other_workspace//flag=value @Test public void testFlagNameWithWorkspace() throws Exception { writeBasicIntFlag(); - rewriteWorkspace("workspace(name = 'starlark_options_test')"); + 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',", + ")"); OptionsParsingResult result = - parseStarlarkOptions("--@starlark_options_test//test:my_int_setting=666"); + parseStarlarkOptions( + "--@starlark_options_test//test:my_int_setting=666 --@repo2//:flag2=222"); - assertThat(result.getStarlarkOptions()).hasSize(1); - assertThat(result.getStarlarkOptions().get("@starlark_options_test//test:my_int_setting")) + assertThat(result.getStarlarkOptions()).hasSize(2); + assertThat(result.getStarlarkOptions().get("//test:my_int_setting")) .isEqualTo(StarlarkInt.of(666)); + assertThat(result.getStarlarkOptions().get("@repo2//:flag2")).isEqualTo(StarlarkInt.of(222)); assertThat(result.getResidue()).isEmpty(); }