From fc4908387b41f7e50b875329e2e045c8d055d467 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Tue, 22 Dec 2020 12:29:55 -0800 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 --- .../build/lib/runtime/StarlarkOptionsParser.java | 5 +++++ .../lib/starlark/StarlarkOptionsParsingTest.java | 16 +++++++++++++--- 2 files changed, 18 insertions(+), 3 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 096cac98281c0d..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 @@ -235,6 +235,11 @@ public static Pair, ImmutableList> removeStarlarkO if (name.startsWith("no")) { potentialStarlarkFlag = potentialStarlarkFlag.substring(2); } + // Check if the string contains a value, trim off the value if so. + int equalsIdx = potentialStarlarkFlag.indexOf('='); + if (equalsIdx > 0) { + potentialStarlarkFlag = potentialStarlarkFlag.substring(0, equalsIdx); + } // Check if we can properly parse the (potentially trimmed) string as a label. If so, count // as starlark flag, else count as regular residue. try { 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..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 @@ -306,16 +306,26 @@ public void testRemoveStarlarkOptionsWorks() throws Exception { StarlarkOptionsParser.removeStarlarkOptions( ImmutableList.of( "--//local/starlark/option", + "--//local/starlark/option=with_value", "--@some_repo//external/starlark/option", + "--@some_repo//external/starlark/option=with_value", "--@//main/repo/option", + "--@//main/repo/option=with_value", "some-random-residue", - "--mangled//external/starlark/option")); + "--mangled//external/starlark/option", + "--mangled//external/starlark/option=with_value")); assertThat(residueAndStarlarkOptions.getFirst()) .containsExactly( "--//local/starlark/option", + "--//local/starlark/option=with_value", "--@some_repo//external/starlark/option", - "--@//main/repo/option"); + "--@some_repo//external/starlark/option=with_value", + "--@//main/repo/option", + "--@//main/repo/option=with_value"); assertThat(residueAndStarlarkOptions.getSecond()) - .containsExactly("some-random-residue", "--mangled//external/starlark/option"); + .containsExactly( + "some-random-residue", + "--mangled//external/starlark/option", + "--mangled//external/starlark/option=with_value"); } }