Skip to content

Commit 1a6fecc

Browse files
fmeumbazel-io
authored andcommitted
Ignore Starlark options on commands with allowResidue = False
Ensures that `bazel version` does not fail when a `common` line in `.bazelrc` contains a Starlark option (similar for `sync` and `shutdown`). There is very little chance for users being confused about these commands accepting and silently ignoring Starlark flags. Fixes bazelbuild#18130 (comment) Closes bazelbuild#19363. PiperOrigin-RevId: 562959337 Change-Id: Ifb4f44d63f1801f6c5a8c2f421138b4014c49a06
1 parent 556396f commit 1a6fecc

File tree

3 files changed

+16
-22
lines changed

3 files changed

+16
-22
lines changed

src/main/java/com/google/devtools/common/options/OptionsParser.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -758,10 +758,8 @@ public ImmutableList<String> parseArgsAsExpansionOfOption(
758758
private void addResidueFromResult(OptionsParserImplResult result) throws OptionsParsingException {
759759
residue.addAll(result.getResidue());
760760
postDoubleDashResidue.addAll(result.postDoubleDashResidue);
761-
if (!allowResidue && (!getSkippedArgs().isEmpty() || !residue.isEmpty())) {
762-
String errorMsg =
763-
"Unrecognized arguments: "
764-
+ Joiner.on(' ').join(Iterables.concat(getSkippedArgs(), residue));
761+
if (!allowResidue && !residue.isEmpty()) {
762+
String errorMsg = "Unrecognized arguments: " + Joiner.on(' ').join(residue);
765763
throw new OptionsParsingException(errorMsg);
766764
}
767765
}

src/test/java/com/google/devtools/build/lib/runtime/BlazeOptionHandlerTest.java

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -437,24 +437,6 @@ public void testParseOptions_residue() {
437437
assertThat(optionHandler.getRcfileNotes()).isEmpty();
438438
}
439439

440-
@Test
441-
public void testParseOptions_disallowResidue_skippedArgsLeadToFailure() throws Exception {
442-
ImmutableList<Class<? extends OptionsBase>> optionsClasses =
443-
ImmutableList.of(TestOptions.class, CommonCommandOptions.class, ClientOptions.class);
444-
445-
BlazeOptionHandlerTestHelper helper =
446-
new BlazeOptionHandlerTestHelper(
447-
optionsClasses,
448-
/* allowResidue= */ false,
449-
/* aliasFlag= */ null,
450-
/* skipStarlarkPrefixes= */ true);
451-
OptionsParser parser = helper.getOptionsParser();
452-
453-
OptionsParsingException e =
454-
assertThrows(OptionsParsingException.class, () -> parser.parse("--//f=1"));
455-
assertThat(e).hasMessageThat().isEqualTo("Unrecognized arguments: --//f=1");
456-
}
457-
458440
@Test
459441
public void testParseOptions_explicitOption() {
460442
optionHandler.parseOptions(

src/test/py/bazel/options_test.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,20 @@ def testCommonPseudoCommand_unsupportedOptionValue(self):
269269
stderr,
270270
)
271271

272+
def testCommonPseudoCommand_allowResidueFalseCommandIgnoresStarlarkOptions(
273+
self,
274+
):
275+
self.ScratchFile("WORKSPACE.bazel")
276+
self.ScratchFile(
277+
".bazelrc",
278+
[
279+
"common --@foo//bar:flag",
280+
],
281+
)
282+
283+
# Check that version doesn't fail.
284+
self.RunBazel(["version"])
285+
272286

273287
if __name__ == "__main__":
274288
unittest.main()

0 commit comments

Comments
 (0)