diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 9f991fa0ad734a..c4165ca5874521 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -304,6 +304,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/buildeventstream/transports",
"//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/cmdline",
+ "//src/main/java/com/google/devtools/build/lib/cmdline:LabelValidator",
"//src/main/java/com/google/devtools/build/lib/collect",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/concurrent",
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 598c13d9c678a7..7d6e9378384a90 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
@@ -23,6 +23,8 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
+import com.google.devtools.build.lib.cmdline.LabelValidator;
+import com.google.devtools.build.lib.cmdline.LabelValidator.BadLabelException;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.events.Reporter;
@@ -219,6 +221,11 @@ private Target loadBuildSetting(
/**
* Separates out any Starlark options from the given list
*
+ *
This method doesn't go through the trouble to actually load build setting targets and verify
+ * they are build settings, it just assumes all strings that look like they could be build
+ * settings, aka are formatted like a flag and can parse out to a proper label, are build
+ * settings. Use actual parsing functions above to do full build setting verification.
+ *
* @param list List of strings from which to parse out starlark options
* @return Returns a pair of string lists. The first item contains the list of starlark options
* that were removed; the second contains the remaining string from the original list.
@@ -228,7 +235,25 @@ public static Pair, ImmutableList> removeStarlarkO
ImmutableList.Builder keep = ImmutableList.builder();
ImmutableList.Builder remove = ImmutableList.builder();
for (String name : list) {
- ((name.startsWith("--//") || name.startsWith("--no//")) ? remove : keep).add(name);
+ // Check if the string is a flag and trim off "--" if so.
+ if (!name.startsWith("--")) {
+ keep.add(name);
+ continue;
+ }
+ String potentialStarlarkFlag = name.substring(2);
+ // Check if the string uses the "no" prefix for setting boolean flags to false, trim
+ // off "no" if so.
+ if (name.startsWith("no")) {
+ potentialStarlarkFlag = potentialStarlarkFlag.substring(2);
+ }
+ // 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 {
+ LabelValidator.validateAbsoluteLabel(potentialStarlarkFlag);
+ remove.add(name);
+ } catch (BadLabelException e) {
+ keep.add(name);
+ }
}
return Pair.of(remove.build(), keep.build());
}
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/BUILD b/src/test/java/com/google/devtools/build/lib/skylark/BUILD
index 2ed08a10b29ddf..177d486d96d751 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/skylark/BUILD
@@ -24,6 +24,7 @@ java_test(
"//src/test/java/com/google/devtools/build/lib:test_runner",
],
deps = [
+ "//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib:syntax",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/analysis:actions/parameter_file_write_action",
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/StarlarkOptionsParsingTest.java b/src/test/java/com/google/devtools/build/lib/skylark/StarlarkOptionsParsingTest.java
index 68a0dad13fdb7b..48a58d725e2db5 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/StarlarkOptionsParsingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/StarlarkOptionsParsingTest.java
@@ -17,7 +17,10 @@
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.runtime.StarlarkOptionsParser;
import com.google.devtools.build.lib.skylark.util.StarlarkOptionsTestCase;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.OptionsParsingResult;
import org.junit.Test;
@@ -314,4 +317,23 @@ public void testOptionsAreParsedWithBuildTestsOnly() throws Exception {
assertThat(result.getStarlarkOptions().get("//test:my_int_setting")).isEqualTo(15);
}
+
+ @Test
+ public void testRemoveStarlarkOptionsWorks() throws Exception {
+ Pair, ImmutableList> residueAndStarlarkOptions =
+ StarlarkOptionsParser.removeStarlarkOptions(
+ ImmutableList.of(
+ "--//local/starlark/option",
+ "--@some_repo//external/starlark/option",
+ "--@//main/repo/option",
+ "some-random-residue",
+ "--mangled//external/starlark/option"));
+ assertThat(residueAndStarlarkOptions.getFirst())
+ .containsExactly(
+ "--//local/starlark/option",
+ "--@some_repo//external/starlark/option",
+ "--@//main/repo/option");
+ assertThat(residueAndStarlarkOptions.getSecond())
+ .containsExactly("some-random-residue", "--mangled//external/starlark/option");
+ }
}