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 13af8bd59e29ff..21104ee7264090 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 @@ -151,7 +151,11 @@ public void parse(ExtendedEventHandler eventHandler) throws OptionsParsingExcept // Map of flag label as a string to its loaded target and set value after parsing. HashMap> buildSettingWithTargetAndValue = new HashMap<>(); for (Map.Entry> 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 = @@ -238,7 +242,11 @@ private void parseArg( if (value != null) { // --flag=value or -flag=value form Target buildSettingTarget = loadBuildSetting(name, 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 3bd9fb6ac01f83..4b3a05a30f01de 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 @@ -78,18 +78,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(); }