Skip to content

Commit

Permalink
Match options against parsing result.
Browse files Browse the repository at this point in the history
Checks whether an existing build options matches a set of flags provided through an option parser (for example from the command line or a file).

Step 2/N towards the platforms mapping functionality for #6426

RELNOTES: None.
PiperOrigin-RevId: 238085427
  • Loading branch information
aragos authored and copybara-github committed Mar 12, 2019
1 parent 30976d8 commit a24b5f3
Show file tree
Hide file tree
Showing 2 changed files with 166 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
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 @@ -384,6 +386,61 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -345,17 +345,18 @@ public void testMultiValueOptionImmutability() throws Exception {

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

OptionsParser parser = OptionsParser.newOptionsParser(BUILD_CONFIG_OPTIONS);
parser.parse("--cpu=bar");
parser.parse("--cpu=bar", "--nostamp");
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 @@ -387,4 +388,110 @@ 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 a24b5f3

Please sign in to comment.