Skip to content

Commit

Permalink
Fix common .bazelrc behavior for flag expansions
Browse files Browse the repository at this point in the history
When a flag specified with `common` in a `.bazelrc` file expanded to a flag unsupported by the current command, it resulted in an error rather than the flag being ignored.

Fixes #18130 (comment)

Closes #20720.

PiperOrigin-RevId: 597271727
Change-Id: Ieaba4dc00e13495a859e1eedf802759ad7dbf774
  • Loading branch information
fmeum authored and copybara-github committed Jan 10, 2024
1 parent df187f4 commit b303144
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ private OptionsParserImplResult parse(
parsedOption = result.parsedOptionDescription;
}
if (parsedOption.isPresent()) {
handleNewParsedOption(parsedOption.get());
handleNewParsedOption(parsedOption.get(), fallbackData);
}
priority = OptionPriority.nextOptionPriority(priority);
}
Expand Down Expand Up @@ -644,7 +644,7 @@ void setOptionValueAtSpecificPriorityWithoutExpansion(
}

/** Takes care of tracking the parsed option's value in relation to other options. */
private void handleNewParsedOption(ParsedOptionDescription parsedOption)
private void handleNewParsedOption(ParsedOptionDescription parsedOption, OptionsData fallbackData)
throws OptionsParsingException {
OptionDefinition optionDefinition = parsedOption.getOptionDefinition();
ExpansionBundle expansionBundle = setOptionValue(parsedOption);
Expand All @@ -658,7 +658,7 @@ private void handleNewParsedOption(ParsedOptionDescription parsedOption)
optionDefinition.hasImplicitRequirements() ? parsedOption : null,
optionDefinition.isExpansionOption() ? parsedOption : null,
expansionBundle.expansionArgs,
/* fallbackData= */ null);
fallbackData);
if (!optionsParserImplResult.getResidue().isEmpty()) {

// Throw an assertion here, because this indicates an error in the definition of this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2589,6 +2589,40 @@ public void fallbackOptions_optionsParsingDifferently() {
assertThat(e).hasCauseThat().isInstanceOf(DuplicateOptionDeclarationException.class);
}

public static class ExpandingOptions extends OptionsBase {
@Option(
name = "foo",
category = "one",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.NO_OP},
expansion = {"--nobar"},
defaultValue = "null")
public Void foo;
}

public static class ExpandingOptionsFallback extends OptionsBase {
@Option(
name = "bar",
category = "one",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.NO_OP},
defaultValue = "true")
public boolean bar;
}

@Test
public void fallbackOptions_expansionToNegativeBooleanFlag() throws OptionsParsingException {
OpaqueOptionsData fallbackData =
OptionsParser.getFallbackOptionsData(
ImmutableList.of(ExpandingOptions.class, ExpandingOptionsFallback.class));
OptionsParser parser = OptionsParser.builder().optionsClasses(ExpandingOptions.class).build();
parser.parseWithSourceFunction(
PriorityCategory.RC_FILE, o -> ".bazelrc", ImmutableList.of("--foo"), fallbackData);

assertThat(parser.getOptions(ExpandingOptions.class)).isNotNull();
assertThat(parser.getOptions(ExpandingOptionsFallback.class)).isNull();
}

private static OptionInstanceOrigin createInvocationPolicyOrigin() {
return createInvocationPolicyOrigin(/*implicitDependent=*/ null, /*expandedFrom=*/ null);
}
Expand Down

0 comments on commit b303144

Please sign in to comment.