From a24b5f389e54d631cfc91ae55b4001e01aa38931 Mon Sep 17 00:00:00 2001 From: schmitt Date: Tue, 12 Mar 2019 13:34:44 -0700 Subject: [PATCH] Match options against parsing result. 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 https://github.com/bazelbuild/bazel/issues/6426 RELNOTES: None. PiperOrigin-RevId: 238085427 --- .../lib/analysis/config/BuildOptions.java | 57 +++++++++ .../lib/analysis/config/BuildOptionsTest.java | 111 +++++++++++++++++- 2 files changed, 166 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java index a2ae5a0628e2df..7775d33c08969f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java @@ -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; @@ -384,6 +386,61 @@ private Map, FragmentOptions> toModifiedFragmen return replacedOptions; } + /** + * Returns true if the passed parsing result's options have the same value as these options. + * + *

If a native parsed option is passed whose fragment has been trimmed in these options it is + * considered to match. + * + *

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 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 fragmentClass = + (Class) 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 starlarkOptions = + labelizeStarlarkOptions(parsingResult.getStarlarkOptions()); + MapDifference 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(); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java index af757e3862367c..bb8b62c5ea8953 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java @@ -345,10 +345,10 @@ 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); @@ -356,6 +356,7 @@ public void parsingResultTransform() throws Exception { 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"); } @@ -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> fragmentClasses = + ImmutableList.>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> fragmentClasses = + ImmutableList.>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> fragmentClasses = + ImmutableList.>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(); + } }