Skip to content

Commit

Permalink
[7.1.0] Fix common .bazelrc behavior for flag expansions (#20844)
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.

Commit
b303144

PiperOrigin-RevId: 597271727
Change-Id: Ieaba4dc00e13495a859e1eedf802759ad7dbf774

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
  • Loading branch information
bazel-io and fmeum authored Jan 11, 2024
1 parent 16210f5 commit 7fb091b
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 @@ -545,7 +545,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 @@ -645,7 +645,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 @@ -659,7 +659,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 @@ -2480,6 +2480,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 7fb091b

Please sign in to comment.