Skip to content

Commit

Permalink
Transform options by parsing result.
Browse files Browse the repository at this point in the history 
    Allows an existing FragmentOptions to be the base for a new one which is distinguished by a set of flags provided through an option parser (for example from the command line or a file).

    Step 1/N towards the platforms mapping functionality for bazelbuild/bazel#6426

    RELNOTES: None.
    PiperOrigin-RevId: 238022069
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 2383cc4 commit e4c2a8e
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 166 deletions.
57 changes: 0 additions & 57 deletions ...build.bazel/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.MapDifference;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.Ordering;
import com.google.common.collect.Sets;
Expand Down Expand Up @@ -386,61 +384,6 @@ private Map<Class<? extends FragmentOptions>, FragmentOptions> toModifiedFragmen
return replacedOptions;
}

/**
* Returns true if the passed parsing result's options have the same value as these options.
*
* <p>If a native parsed option is passed whose fragment has been trimmed in these options it is
* considered to match.
*
* <p>If no options are present in the parsing result or all options in the parsing result have
* been trimmed the result is considered not to match. This is because otherwise the parsing
* result would match any options in a similar trimmed state, regardless of contents.
*
* @param parsingResult parsing result to be compared to these options
* @return true if all non-trimmed values match
* @throws OptionsParsingException if options cannot be parsed
*/
public boolean matches(OptionsParsingResult parsingResult) throws OptionsParsingException {
Set<OptionDefinition> ignoredDefinitions = new HashSet<>();
for (ParsedOptionDescription parsedOption : parsingResult.asListOfExplicitOptions()) {
OptionDefinition optionDefinition = parsedOption.getOptionDefinition();

// All options obtained from an options parser are guaranteed to have been defined in an
// FragmentOptions class.
@SuppressWarnings("unchecked")
Class<? extends FragmentOptions> fragmentClass =
(Class<? extends FragmentOptions>) optionDefinition.getField().getDeclaringClass();

FragmentOptions originalFragment = fragmentOptionsMap.get(fragmentClass);
if (originalFragment == null) {
// Ignore flags set in trimmed fragments.
ignoredDefinitions.add(optionDefinition);
continue;
}
Object originalValue = originalFragment.asMap().get(optionDefinition.getOptionName());
if (!Objects.equals(originalValue, parsedOption.getConvertedValue())) {
return false;
}
}

Map<Label, Object> starlarkOptions =
labelizeStarlarkOptions(parsingResult.getStarlarkOptions());
MapDifference<Label, Object> starlarkDifference =
Maps.difference(skylarkOptionsMap, starlarkOptions);
if (starlarkDifference.entriesInCommon().size() < starlarkOptions.size()) {
return false;
}

if (ignoredDefinitions.size() == parsingResult.asListOfExplicitOptions().size()
&& starlarkOptions.isEmpty()) {
// Zero options were compared, either because none were passed or because all of them were
// trimmed.
return false;
}

return true;
}

/** Creates a builder object for BuildOptions */
public static Builder builder() {
return new Builder();
Expand Down
111 changes: 2 additions & 109 deletions ...d.bazel/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,18 +345,17 @@ public void testMultiValueOptionImmutability() throws Exception {

@Test
public void parsingResultTransform() throws Exception {
BuildOptions original = BuildOptions.of(BUILD_CONFIG_OPTIONS, "--cpu=foo", "--stamp");
BuildOptions original = BuildOptions.of(BUILD_CONFIG_OPTIONS, "--cpu=foo");

OptionsParser parser = OptionsParser.newOptionsParser(BUILD_CONFIG_OPTIONS);
parser.parse("--cpu=bar", "--nostamp");
parser.parse("--cpu=bar");
parser.setStarlarkOptions(ImmutableMap.of("//custom:flag", "hello"));

BuildOptions modified = original.applyParsingResult(parser);

assertThat(original.get(BuildConfiguration.Options.class).cpu)
.isNotEqualTo(modified.get(BuildConfiguration.Options.class).cpu);
assertThat(modified.get(BuildConfiguration.Options.class).cpu).isEqualTo("bar");
assertThat(modified.get(Options.class).stampBinaries).isFalse();
assertThat(modified.getStarlarkOptions().get(Label.parseAbsoluteUnchecked("//custom:flag")))
.isEqualTo("hello");
}
Expand Down Expand Up @@ -388,110 +387,4 @@ public void parsingResultTransformIllegalStarlarkLabel() throws Exception {

assertThrows(IllegalArgumentException.class, () -> original.applyParsingResult(parser));
}

@Test
public void parsingResultMatch() throws Exception {
BuildOptions original = BuildOptions.of(BUILD_CONFIG_OPTIONS, "--cpu=foo", "--stamp");

OptionsParser matchingParser = OptionsParser.newOptionsParser(BUILD_CONFIG_OPTIONS);
matchingParser.parse("--cpu=foo", "--stamp");

OptionsParser notMatchingParser = OptionsParser.newOptionsParser(BUILD_CONFIG_OPTIONS);
notMatchingParser.parse("--cpu=foo", "--nostamp");

assertThat(original.matches(matchingParser)).isTrue();
assertThat(original.matches(notMatchingParser)).isFalse();
}

@Test
public void parsingResultMatchStarlark() throws Exception {
BuildOptions original =
BuildOptions.builder()
.addStarlarkOption(Label.parseAbsoluteUnchecked("//custom:flag"), "hello")
.build();

OptionsParser matchingParser = OptionsParser.newOptionsParser(BUILD_CONFIG_OPTIONS);
matchingParser.setStarlarkOptions(ImmutableMap.of("//custom:flag", "hello"));

OptionsParser notMatchingParser = OptionsParser.newOptionsParser(BUILD_CONFIG_OPTIONS);
notMatchingParser.setStarlarkOptions(ImmutableMap.of("//custom:flag", "foo"));

assertThat(original.matches(matchingParser)).isTrue();
assertThat(original.matches(notMatchingParser)).isFalse();
}

@Test
public void parsingResultMatchMissingFragment() throws Exception {
BuildOptions original = BuildOptions.of(BUILD_CONFIG_OPTIONS, "--cpu=foo");

ImmutableList<Class<? extends FragmentOptions>> fragmentClasses =
ImmutableList.<Class<? extends FragmentOptions>>builder()
.add(BuildConfiguration.Options.class)
.add(CppOptions.class)
.build();

OptionsParser parser = OptionsParser.newOptionsParser(fragmentClasses);
parser.parse("--cpu=foo", "--cxxopt=bar");

assertThat(original.matches(parser)).isTrue();
}

@Test
public void parsingResultMatchEmptyNativeMatch() throws Exception {
BuildOptions original = BuildOptions.of(BUILD_CONFIG_OPTIONS, "--cpu=foo");

ImmutableList<Class<? extends FragmentOptions>> fragmentClasses =
ImmutableList.<Class<? extends FragmentOptions>>builder()
.add(BuildConfiguration.Options.class)
.add(CppOptions.class)
.build();

OptionsParser parser = OptionsParser.newOptionsParser(fragmentClasses);
parser.parse("--cxxopt=bar");

assertThat(original.matches(parser)).isFalse();
}

@Test
public void parsingResultMatchEmptyNativeMatchWithStarlark() throws Exception {
BuildOptions original =
BuildOptions.builder()
.addStarlarkOption(Label.parseAbsoluteUnchecked("//custom:flag"), "hello")
.build();

ImmutableList<Class<? extends FragmentOptions>> fragmentClasses =
ImmutableList.<Class<? extends FragmentOptions>>builder()
.add(BuildConfiguration.Options.class)
.add(CppOptions.class)
.build();

OptionsParser parser = OptionsParser.newOptionsParser(fragmentClasses);
parser.parse("--cxxopt=bar");
parser.setStarlarkOptions(ImmutableMap.of("//custom:flag", "hello"));

assertThat(original.matches(parser)).isTrue();
}

@Test
public void parsingResultMatchStarlarkOptionMissing() throws Exception {
BuildOptions original =
BuildOptions.builder()
.addStarlarkOption(Label.parseAbsoluteUnchecked("//custom:flag1"), "hello")
.build();

OptionsParser parser = OptionsParser.newOptionsParser(BUILD_CONFIG_OPTIONS);
parser.setStarlarkOptions(ImmutableMap.of("//custom:flag2", "foo"));

assertThat(original.matches(parser)).isFalse();
}

@Test
public void parsingResultMatchNullOption() throws Exception {
BuildOptions original = BuildOptions.of(BUILD_CONFIG_OPTIONS);

OptionsParser parser = OptionsParser.newOptionsParser(BUILD_CONFIG_OPTIONS);
parser.parse("--platform_suffix=foo"); // Note: platform_suffix is null by default.

assertThat(original.matches(parser)).isFalse();
}
}

0 comments on commit e4c2a8e

Please sign in to comment.