From ba8678077024e1b4e5d7419c758a97e8dc9fceea Mon Sep 17 00:00:00 2001 From: John Cater Date: Mon, 23 Aug 2021 11:04:37 -0400 Subject: [PATCH] Revert "fix main repo starlark options parsing. Fixes #13890. This reverts commit 80c59dea59d4dce39d4b5d21665c3d7313197358. --- .../lib/runtime/StarlarkOptionsParser.java | 8 +---- .../starlark/StarlarkOptionsParsingTest.java | 32 +++---------------- 2 files changed, 6 insertions(+), 34 deletions(-) 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 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 = @@ -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 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(); }