remove "support" for space syntax with starlark flags.
Though we had support in the Starlark options parser, we had no actual way for a space-separated flag to get through the regular parser without the value getting dropped off. So remove the support in the Starlark options parser since we weren't ever actually using it anyway.
Update docs.
PiperOrigin-RevId: 336675738
diff --git a/site/docs/skylark/config.md b/site/docs/skylark/config.md
index 3410596..118ae55 100644
--- a/site/docs/skylark/config.md
+++ b/site/docs/skylark/config.md
@@ -238,14 +238,22 @@
```
-#### Settings Build Settings on the command line
+#### Using build settings on the command line
-Build settings are set on the command line like any other flag. Boolean build
-settings understand no-prefixes and both equals and space syntaxes are supported.
-The name of build settings is their full target path:
+Similar to most native flags, you can use the command line to set build settings
+[that are marked as flags](#the-build-setting-rule-parameter). The build
+setting's name is its full target path using `name=value` syntax:
```shell
-$ bazel build //my/target --//example:favorite_flavor="PAMPLEMOUSSE"
+$ bazel build //my/target --//example:string_flag=some-value # allowed
+$ bazel build //my/target --//example:string_flag some-value # not allowed
+```
+
+Special boolean syntax is supported:
+
+```shell
+$ bazel build //my/target --//example:boolean_flag
+$ bazel build //my/target --no//example:boolean_flag
```
There are plans to implement shorthand mapping of flag labels so users don't
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 ec03856..096cac9 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
@@ -40,7 +40,6 @@
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.Collections;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -89,17 +88,14 @@
Maps.newHashMapWithExpectedSize(nativeOptionsParser.getResidue().size());
// sort the old residue into starlark flags and legitimate residue
- Iterator<String> unparsedArgs = nativeOptionsParser.getPreDoubleDashResidue().iterator();
- while (unparsedArgs.hasNext()) {
- String arg = unparsedArgs.next();
-
+ for (String arg : nativeOptionsParser.getPreDoubleDashResidue()) {
// TODO(bazel-team): support single dash options?
if (!arg.startsWith("--")) {
residue.add(arg);
continue;
}
- parseArg(arg, unparsedArgs, unparsedOptions, eventHandler);
+ parseArg(arg, unparsedOptions, eventHandler);
}
List<String> postDoubleDashResidue = nativeOptionsParser.getPostDoubleDashResidue();
@@ -146,7 +142,6 @@
private void parseArg(
String arg,
- Iterator<String> unparsedArgs,
Map<String, Pair<String, Target>> unparsedOptions,
ExtendedEventHandler eventHandler)
throws OptionsParsingException {
@@ -180,12 +175,7 @@
throw new OptionsParsingException(
"Illegal use of 'no' prefix on non-boolean option: " + name, name);
}
- if (unparsedArgs.hasNext()) {
- // --flag value
- unparsedOptions.put(name, new Pair<>(unparsedArgs.next(), buildSettingTarget));
- } else {
- throw new OptionsParsingException("Expected value after " + arg);
- }
+ throw new OptionsParsingException("Expected value after " + arg);
}
}
}
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 ab72af3..994004a 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,19 +45,6 @@
assertThat(result.getResidue()).isEmpty();
}
- // test --flag value
- @Test
- public void testFlagSpaceValueForm() throws Exception {
- writeBasicIntFlag();
-
- OptionsParsingResult result = parseStarlarkOptions("--//test:my_int_setting 666");
-
- assertThat(result.getStarlarkOptions()).hasSize(1);
- assertThat(result.getStarlarkOptions().get("//test:my_int_setting"))
- .isEqualTo(StarlarkInt.of(666));
- assertThat(result.getResidue()).isEmpty();
- }
-
// test --@workspace//flag=value
@Test
public void testFlagNameWithWorkspace() throws Exception {
@@ -88,21 +75,6 @@
assertThat(e.getInvalidArgument()).isEqualTo("//fake_flag");
}
- // test --fake_flag value
- @Test
- public void testBadFlag_spaceForm() throws Exception {
- scratch.file("test/BUILD");
- reporter.removeHandler(failFastHandler);
-
- OptionsParsingException e =
- assertThrows(
- OptionsParsingException.class,
- () -> parseStarlarkOptions("--//fake_flag blahblahblah"));
-
- assertThat(e).hasMessageThat().contains("Error loading option //fake_flag");
- assertThat(e.getInvalidArgument()).isEqualTo("//fake_flag");
- }
-
// test --fake_flag
@Test
public void testBadFlag_boolForm() throws Exception {
