diff options
Diffstat (limited to 'src')
7 files changed, 883 insertions, 97 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigMatchingProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigMatchingProvider.java index 8cee3fb82e..2d7ccd26b1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigMatchingProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigMatchingProvider.java @@ -17,7 +17,6 @@ package com.google.devtools.build.lib.analysis.config; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; - import java.util.Map; import java.util.Set; @@ -35,17 +34,23 @@ public final class ConfigMatchingProvider implements TransitiveInfoProvider { private final Label label; private final boolean matches; private final Map<String, String> settingsMap; + private final Map<Label, String> flagSettingsMap; /** * @param label the build label corresponding to this matcher * @param settingsMap the condition settings that trigger this matcher - * @param matches whether or not this matcher matches the configuration associated with - * its configured target + * @param flagSettingsMap the label-keyed settings that trigger this matcher + * @param matches whether or not this matcher matches the configuration associated with its + * configured target */ - public ConfigMatchingProvider(Label label, Map<String, String> settingsMap, + public ConfigMatchingProvider( + Label label, + Map<String, String> settingsMap, + Map<Label, String> flagSettingsMap, boolean matches) { this.label = label; this.settingsMap = settingsMap; + this.flagSettingsMap = flagSettingsMap; this.matches = matches; } @@ -71,6 +76,25 @@ public final class ConfigMatchingProvider implements TransitiveInfoProvider { public boolean refines(ConfigMatchingProvider other) { Set<Map.Entry<String, String>> settings = settingsMap.entrySet(); Set<Map.Entry<String, String>> otherSettings = other.settingsMap.entrySet(); - return settings.containsAll(otherSettings) && settings.size() > otherSettings.size(); + Set<Map.Entry<Label, String>> flagSettings = flagSettingsMap.entrySet(); + Set<Map.Entry<Label, String>> otherFlagSettings = other.flagSettingsMap.entrySet(); + + if (!settings.containsAll(otherSettings)) { + // not a superset + return false; + } + + if (!flagSettings.containsAll(otherFlagSettings)) { + // not a superset + return false; + } + + if (!(settings.size() > otherSettings.size() + || flagSettings.size() > otherFlagSettings.size())) { + // not a proper superset + return false; + } + + return true; } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagProvider.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagProvider.java index 0a25669615..1bc6481234 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagProvider.java @@ -47,7 +47,7 @@ public abstract class ConfigFeatureFlagProvider extends SkylarkClassObject new NativeClassObjectConstructor(SKYLARK_NAME) {}; /** Identifier used to retrieve this provider from rules which export it. */ - private static final SkylarkProviderIdentifier SKYLARK_IDENTIFIER = + static final SkylarkProviderIdentifier SKYLARK_IDENTIFIER = SkylarkProviderIdentifier.forKey(SKYLARK_CONSTRUCTOR.getKey()); ConfigFeatureFlagProvider() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java index 053fa7b1c5..0b816c1a0e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.rules.config; import static com.google.devtools.build.lib.packages.Attribute.attr; +import static com.google.devtools.build.lib.packages.BuildType.LABEL_KEYED_STRING_DICT; import static com.google.devtools.build.lib.syntax.Type.STRING; import static com.google.devtools.build.lib.syntax.Type.STRING_DICT; import static com.google.devtools.build.lib.syntax.Type.STRING_LIST; @@ -107,6 +108,8 @@ public class ConfigRuleClasses { * The name of the attribute that declares flag bindings. */ public static final String SETTINGS_ATTRIBUTE = "values"; + /** The name of the attribute that declares user-defined flag bindings. */ + public static final String FLAG_SETTINGS_ATTRIBUTE = "flag_values"; private static final Function<Rule, Set<String>> CONFIG_SETTING_OPTION_REFERENCE = new Function<Rule, Set<String>>() { @@ -157,7 +160,13 @@ public class ConfigRuleClasses { <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ .add( attr(SETTINGS_ATTRIBUTE, STRING_DICT) - .mandatory() + .nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON)) + .add( + attr(FLAG_SETTINGS_ATTRIBUTE, LABEL_KEYED_STRING_DICT) + .undocumented("the feature flag feature has not yet been launched") + .allowedFileTypes() + .mandatoryProviders( + ImmutableList.of(ConfigFeatureFlagProvider.SKYLARK_IDENTIFIER)) .nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON)) .setIsConfigMatcherForConfigSettingOnly() .setOptionReferenceFunctionForConfigSettingOnly(CONFIG_SETTING_OPTION_REFERENCE) diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java index bdc40fb8cc..1da66e0cb1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java +++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java @@ -14,20 +14,26 @@ package com.google.devtools.build.lib.rules.config; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FileProvider; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.LicensesProviderImpl; +import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.RunfilesProvider; +import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.config.BuildConfigurationOptionDetails; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.analysis.config.TransitiveOptionDetails; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; +import com.google.devtools.build.lib.packages.RuleErrorConsumer; import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.Preconditions; @@ -53,26 +59,44 @@ public class ConfigSetting implements RuleConfiguredTargetFactory { // Get the required flag=value settings for this rule. Map<String, String> settings = NonconfigurableAttributeMapper.of(ruleContext.getRule()) .get(ConfigRuleClasses.ConfigSettingRule.SETTINGS_ATTRIBUTE, Type.STRING_DICT); - if (settings.isEmpty()) { - ruleContext.attributeError(ConfigRuleClasses.ConfigSettingRule.SETTINGS_ATTRIBUTE, - "no settings specified"); + Map<Label, String> flagSettings = + NonconfigurableAttributeMapper.of(ruleContext.getRule()) + .get( + ConfigRuleClasses.ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE, + BuildType.LABEL_KEYED_STRING_DICT); + + List<? extends TransitiveInfoCollection> flagValues = + ruleContext.getPrerequisites( + ConfigRuleClasses.ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE, Mode.TARGET); + + ImmutableMap<Label, ConfigFeatureFlagProvider> configProviders = + buildConfigFeatureFlagMap(flagValues); + + if (settings.isEmpty() && flagSettings.isEmpty()) { + ruleContext.ruleError( + String.format( + "Either %s or %s must be specified and non-empty", + ConfigRuleClasses.ConfigSettingRule.SETTINGS_ATTRIBUTE, + ConfigRuleClasses.ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE)); return null; } - ConfigMatchingProvider configMatcher; - try { - configMatcher = - new ConfigMatchingProvider( - ruleContext.getLabel(), - settings, - matchesConfig( - settings, BuildConfigurationOptionDetails.get(ruleContext.getConfiguration()))); - } catch (OptionsParsingException e) { - ruleContext.attributeError(ConfigRuleClasses.ConfigSettingRule.SETTINGS_ATTRIBUTE, - "error while parsing configuration settings: " + e.getMessage()); + boolean flagSettingsMatch = matchesUserConfig(flagSettings, configProviders, ruleContext); + + boolean settingsMatch = + matchesConfig( + settings, + BuildConfigurationOptionDetails.get(ruleContext.getConfiguration()), + ruleContext); + + if (ruleContext.hasErrors()) { return null; } + ConfigMatchingProvider configMatcher = + new ConfigMatchingProvider( + ruleContext.getLabel(), settings, flagSettings, settingsMatch && flagSettingsMatch); + return new RuleConfiguredTargetBuilder(ruleContext) .addProvider(RunfilesProvider.class, RunfilesProvider.EMPTY) .addProvider(FileProvider.class, FileProvider.EMPTY) @@ -82,13 +106,60 @@ public class ConfigSetting implements RuleConfiguredTargetFactory { .build(); } + /** Maps the labels of the given prerequisites to their {@link ConfigFeatureFlagProvider}s. */ + private static ImmutableMap<Label, ConfigFeatureFlagProvider> buildConfigFeatureFlagMap( + Iterable<? extends TransitiveInfoCollection> prerequisites) { + ImmutableMap.Builder<Label, ConfigFeatureFlagProvider> output = new ImmutableMap.Builder<>(); + + for (TransitiveInfoCollection target : prerequisites) { + ConfigFeatureFlagProvider provider = ConfigFeatureFlagProvider.fromTarget(target); + // We know the provider exists because only labels with ConfigFeatureFlagProvider can be added + // to this attribute. + assert provider != null; + output.put(target.getLabel(), provider); + } + + return output.build(); + } + + /** Returns whether the actual user-defined flags are set to the specified values. */ + private static boolean matchesUserConfig( + Map<Label, String> specifiedFlags, + Map<Label, ConfigFeatureFlagProvider> actualFlags, + RuleErrorConsumer errors) { + boolean foundMismatch = false; + for (Map.Entry<Label, String> specifiedFlag : specifiedFlags.entrySet()) { + Label label = specifiedFlag.getKey(); + String specifiedValue = specifiedFlag.getValue(); + ConfigFeatureFlagProvider provider = actualFlags.get(label); + // Both specifiedFlags and actualFlags are built from the same set of keys; therefore, the + // provider will always be present. + assert provider != null; + if (!provider.isValidValue(specifiedValue)) { + errors.attributeError( + ConfigRuleClasses.ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE, + String.format( + "error while parsing user-defined configuration values: " + + "'%s' is not a valid value for '%s'", + specifiedValue, label)); + foundMismatch = true; + continue; + } + if (!provider.getValue().equals(specifiedValue)) { + foundMismatch = true; + } + } + return !foundMismatch; + } + /** * Given a list of [flagName, flagValue] pairs, returns true if flagName == flagValue for every * item in the list under this configuration, false otherwise. */ private boolean matchesConfig( - Map<String, String> expectedSettings, TransitiveOptionDetails options) - throws OptionsParsingException { + Map<String, String> expectedSettings, + TransitiveOptionDetails options, + RuleErrorConsumer errors) { // Rather than returning fast when we find a mismatch, continue looking at the other flags // to check that they're indeed valid flag specifications. boolean foundMismatch = false; @@ -102,7 +173,12 @@ public class ConfigSetting implements RuleConfiguredTargetFactory { Class<? extends OptionsBase> optionClass = options.getOptionClass(optionName); if (optionClass == null) { - throw new OptionsParsingException("unknown option: '" + optionName + "'"); + errors.attributeError( + ConfigRuleClasses.ConfigSettingRule.SETTINGS_ATTRIBUTE, + String.format( + "error while parsing configuration settings: unknown option: '%s'", optionName)); + foundMismatch = true; + continue; } OptionsParser parser = parserCache.get(optionClass); @@ -110,7 +186,17 @@ public class ConfigSetting implements RuleConfiguredTargetFactory { parser = OptionsParser.newOptionsParser(optionClass); parserCache.put(optionClass, parser); } - parser.parse("--" + optionName + "=" + expectedRawValue); + + try { + parser.parse("--" + optionName + "=" + expectedRawValue); + } catch (OptionsParsingException ex) { + errors.attributeError( + ConfigRuleClasses.ConfigSettingRule.SETTINGS_ATTRIBUTE, + "error while parsing configuration settings: " + ex.getMessage()); + foundMismatch = true; + continue; + } + Object expectedParsedValue = parser.getOptions(optionClass).asMap().get(optionName); if (!optionMatches(options, optionName, expectedParsedValue)) { diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java index 9fe164bd65..ad2de2cf59 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java @@ -15,30 +15,10 @@ package com.google.devtools.build.lib.rules.config; import static com.google.common.truth.Truth.assertThat; -import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER; -import static com.google.devtools.build.lib.packages.Attribute.attr; -import static com.google.devtools.build.lib.packages.BuildType.LABEL; -import static com.google.devtools.build.lib.packages.BuildType.LABEL_KEYED_STRING_DICT; -import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.ConfiguredTarget; -import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; -import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; -import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.RuleDefinition; -import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; -import com.google.devtools.build.lib.analysis.RunfilesProvider; -import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.packages.RuleClass; -import com.google.devtools.build.lib.packages.RuleClass.Builder; -import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; import com.google.devtools.build.lib.rules.SkylarkRuleContext; import com.google.devtools.build.lib.skylark.util.SkylarkTestCase; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; @@ -51,55 +31,6 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public final class ConfigFeatureFlagTest extends SkylarkTestCase { - /** Rule introducing a transition to set feature flags for dependencies. */ - public static final class FeatureFlagSetter - implements RuleDefinition, RuleConfiguredTargetFactory { - - @Override - public RuleClass build(Builder builder, RuleDefinitionEnvironment env) { - return builder - .requiresConfigurationFragments(ConfigFeatureFlagConfiguration.class) - .cfg(new ConfigFeatureFlagTransitionFactory("flag_values")) - .add(attr("deps", LABEL_LIST).allowedFileTypes()) - .add( - attr("exports_flag", LABEL) - .allowedRuleClasses("config_feature_flag") - .allowedFileTypes()) - .add( - attr("flag_values", LABEL_KEYED_STRING_DICT) - .allowedRuleClasses("config_feature_flag") - .allowedFileTypes() - .nonconfigurable("used in RuleTransitionFactory") - .value(ImmutableMap.<Label, String>of())) - .build(); - } - - @Override - public Metadata getMetadata() { - return RuleDefinition.Metadata.builder() - .name("feature_flag_setter") - .ancestors(BaseRuleClasses.BaseRule.class) - .factoryClass(FeatureFlagSetter.class) - .build(); - } - - @Override - public ConfiguredTarget create(RuleContext ruleContext) - throws InterruptedException, RuleErrorException { - TransitiveInfoCollection dep = ruleContext.getPrerequisite("exports_flag", Mode.TARGET); - ConfigFeatureFlagProvider exportedProvider = - dep != null ? ConfigFeatureFlagProvider.fromTarget(dep) : null; - RuleConfiguredTargetBuilder builder = - new RuleConfiguredTargetBuilder(ruleContext) - .setFilesToBuild(NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER)) - .addProvider(RunfilesProvider.class, RunfilesProvider.EMPTY); - if (exportedProvider != null) { - builder.addNativeDeclaredProvider(exportedProvider); - } - return builder.build(); - } - } - @Before public void useDynamicConfigurations() throws Exception { useConfiguration("--experimental_dynamic_configs=on"); @@ -108,7 +39,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase { @Override protected ConfiguredRuleClassProvider getRuleClassProvider() { ConfiguredRuleClassProvider.Builder builder = - new ConfiguredRuleClassProvider.Builder().addRuleDefinition(new FeatureFlagSetter()); + new ConfiguredRuleClassProvider.Builder().addRuleDefinition(new FeatureFlagSetterRule()); TestRuleClassProvider.addStandardRules(builder); return builder.build(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java index 7c9bad5f35..33514d38b5 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java @@ -114,6 +114,7 @@ public class ConfigSettingTest extends BuildViewTestCase { protected ConfiguredRuleClassProvider getRuleClassProvider() { ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder(); TestRuleClassProvider.addStandardRules(builder); + builder.addRuleDefinition(new FeatureFlagSetterRule()); builder.addConfigurationOptions(LateBoundTestOptions.class); builder.addConfigurationFragment(new LateBoundTestOptionsLoader()); builder.addConfigurationOptions(InternalTestOptions.class); @@ -224,12 +225,13 @@ public class ConfigSettingTest extends BuildViewTestCase { } /** - * Tests that *some* settings must be specified. + * Tests that *some* settings (values or flag_values) must be specified. */ @Test public void emptySettings() throws Exception { checkError("foo", "empty", - "//foo:empty: no settings specified", + "in config_setting rule //foo:empty: " + + "Either values or flag_values must be specified and non-empty", "config_setting(", " name = 'empty',", " values = {})"); @@ -329,4 +331,637 @@ public class ConfigSettingTest extends BuildViewTestCase { assertThat(target.getRuleClassObject().getOptionReferenceFunction().apply(target)) .containsExactly("copt", "javacopt"); } + + @Test + public void matchesIfFlagValuesAndValuesBothMatch() throws Exception { + useConfiguration("--copt=-Dright"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'match',", + " flag_values = {", + " ':flag': 'right',", + " },", + " values = {", + " 'copt': '-Dright',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right'],", + " default_value = 'right',", + ")"); + assertThat(getConfigMatchingProvider("//test:match").matches()).isTrue(); + } + + @Test + public void matchesIfFlagValuesMatchAndValuesAreEmpty() throws Exception { + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'match',", + " flag_values = {", + " ':flag': 'right',", + " },", + " values = {},", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right'],", + " default_value = 'right',", + ")"); + assertThat(getConfigMatchingProvider("//test:match").matches()).isTrue(); + } + + @Test + public void matchesIfValuesMatchAndFlagValuesAreEmpty() throws Exception { + useConfiguration("--copt=-Dright"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'match',", + " flag_values = {},", + " values = {", + " 'copt': '-Dright',", + " },", + ")"); + assertThat(getConfigMatchingProvider("//test:match").matches()).isTrue(); + } + + @Test + public void doesNotMatchIfNeitherFlagValuesNorValuesMatches() throws Exception { + useConfiguration("--copt=-Dright"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'match',", + " flag_values = {", + " ':flag': 'wrong',", + " },", + " values = {", + " 'copt': '-Dwrong',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")"); + assertThat(getConfigMatchingProvider("//test:match").matches()).isFalse(); + } + + @Test + public void doesNotMatchIfFlagValuesDoNotMatchAndValuesAreEmpty() throws Exception { + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'match',", + " flag_values = {", + " ':flag': 'wrong',", + " },", + " values = {},", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")"); + assertThat(getConfigMatchingProvider("//test:match").matches()).isFalse(); + } + + @Test + public void doesNotMatchIfFlagValuesDoNotMatchButValuesDo() throws Exception { + useConfiguration("--copt=-Dright"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'match',", + " flag_values = {", + " ':flag': 'wrong',", + " },", + " values = {", + " 'copt': '-Dright',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")"); + assertThat(getConfigMatchingProvider("//test:match").matches()).isFalse(); + } + + @Test + public void doesNotMatchIfValuesDoNotMatchAndFlagValuesAreEmpty() throws Exception { + useConfiguration("--copt=-Dright"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'match',", + " flag_values = {},", + " values = {", + " 'copt': '-Dwrong',", + " },", + ")"); + assertThat(getConfigMatchingProvider("//test:match").matches()).isFalse(); + } + + @Test + public void doesNotMatchIfValuesDoNotMatchButFlagValuesDo() throws Exception { + useConfiguration("--copt=-Dright"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'match',", + " flag_values = {", + " ':flag': 'right',", + " },", + " values = {", + " 'copt': '-Dwrong',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")"); + assertThat(getConfigMatchingProvider("//test:match").matches()).isFalse(); + } + + @Test + public void doesNotMatchIfEvenOneFlagValueDoesNotMatch() throws Exception { + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'match',", + " flag_values = {", + " ':flag': 'right',", + " ':flag2': 'bad',", + " },", + " values = {},", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")", + "config_feature_flag(", + " name = 'flag2',", + " allowed_values = ['good', 'bad'],", + " default_value = 'good',", + ")"); + assertThat(getConfigMatchingProvider("//test:match").matches()).isFalse(); + } + + @Test + public void matchesIfNonDefaultIsSpecifiedAndFlagValueIsThatValue() throws Exception { + scratch.file( + "test/BUILD", + "feature_flag_setter(", + " name = 'setter',", + " exports_setting = ':match',", + " flag_values = {':flag': 'actual'},", + ")", + "config_setting(", + " name = 'match',", + " flag_values = {", + " ':flag': 'actual',", + " },", + " values = {},", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['default', 'actual'],", + " default_value = 'default',", + ")"); + assertThat(getConfigMatchingProvider("//test:setter").matches()).isTrue(); + } + + @Test + public void doesNotMatchIfDefaultIsSpecifiedAndFlagValueIsNotDefault() throws Exception { + scratch.file( + "test/BUILD", + "feature_flag_setter(", + " name = 'setter',", + " exports_setting = ':match',", + " flag_values = {':flag': 'actual'},", + ")", + "config_setting(", + " name = 'match',", + " flag_values = {", + " ':flag': 'default',", + " },", + " values = {},", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['default', 'actual'],", + " default_value = 'default',", + ")"); + assertThat(getConfigMatchingProvider("//test:setter").matches()).isFalse(); + } + + @Test + public void doesNotRefineSettingWithSameValuesAndSameFlagValues() throws Exception { + useConfiguration("--copt=-Dright", "--javacopt=-Dgood"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'refined',", + " flag_values = {", + " ':flag': 'right',", + " ':flag2': 'good',", + " },", + " values = {", + " 'copt': '-Dright',", + " 'javacopt': '-Dgood',", + " },", + ")", + "config_setting(", + " name = 'other',", + " flag_values = {", + " ':flag': 'right',", + " ':flag2': 'good',", + " },", + " values = {", + " 'copt': '-Dright',", + " 'javacopt': '-Dgood',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")", + "config_feature_flag(", + " name = 'flag2',", + " allowed_values = ['good', 'bad'],", + " default_value = 'good',", + ")"); + assertThat( + getConfigMatchingProvider("//test:refined") + .refines(getConfigMatchingProvider("//test:other"))) + .isFalse(); + } + + @Test + public void doesNotRefineSettingWithDifferentValuesAndSameFlagValues() throws Exception { + useConfiguration("--copt=-Dright", "--javacopt=-Dgood"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'refined',", + " flag_values = {", + " ':flag': 'right',", + " ':flag2': 'good',", + " },", + " values = {", + " 'javacopt': '-Dgood',", + " },", + ")", + "config_setting(", + " name = 'other',", + " flag_values = {", + " ':flag': 'right',", + " ':flag2': 'good',", + " },", + " values = {", + " 'copt': '-Dright',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")", + "config_feature_flag(", + " name = 'flag2',", + " allowed_values = ['good', 'bad'],", + " default_value = 'good',", + ")"); + assertThat( + getConfigMatchingProvider("//test:refined") + .refines(getConfigMatchingProvider("//test:other"))) + .isFalse(); + } + + @Test + public void doesNotRefineSettingWithSameValuesAndDifferentFlagValues() throws Exception { + useConfiguration("--copt=-Dright", "--javacopt=-Dgood"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'refined',", + " flag_values = {", + " ':flag': 'right',", + " },", + " values = {", + " 'copt': '-Dright',", + " 'javacopt': '-Dgood',", + " },", + ")", + "config_setting(", + " name = 'other',", + " flag_values = {", + " ':flag2': 'good',", + " },", + " values = {", + " 'copt': '-Dright',", + " 'javacopt': '-Dgood',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")", + "config_feature_flag(", + " name = 'flag2',", + " allowed_values = ['good', 'bad'],", + " default_value = 'good',", + ")"); + assertThat( + getConfigMatchingProvider("//test:refined") + .refines(getConfigMatchingProvider("//test:other"))) + .isFalse(); + } + + @Test + public void doesNotRefineSettingWithDifferentValuesAndDifferentFlagValues() throws Exception { + useConfiguration("--copt=-Dright", "--javacopt=-Dgood"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'refined',", + " flag_values = {", + " ':flag': 'right',", + " },", + " values = {", + " 'copt': '-Dright',", + " },", + ")", + "config_setting(", + " name = 'other',", + " flag_values = {", + " ':flag2': 'good',", + " },", + " values = {", + " 'javacopt': '-Dgood',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")", + "config_feature_flag(", + " name = 'flag2',", + " allowed_values = ['good', 'bad'],", + " default_value = 'good',", + ")"); + assertThat( + getConfigMatchingProvider("//test:refined") + .refines(getConfigMatchingProvider("//test:other"))) + .isFalse(); + } + + @Test + public void doesNotRefineSettingWithDifferentValuesAndSubsetFlagValues() throws Exception { + useConfiguration("--copt=-Dright", "--javacopt=-Dgood"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'refined',", + " flag_values = {", + " ':flag': 'right',", + " ':flag2': 'good',", + " },", + " values = {", + " 'copt': '-Dright',", + " },", + ")", + "config_setting(", + " name = 'other',", + " flag_values = {", + " ':flag': 'right',", + " },", + " values = {", + " 'javacopt': '-Dgood',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")", + "config_feature_flag(", + " name = 'flag2',", + " allowed_values = ['good', 'bad'],", + " default_value = 'good',", + ")"); + assertThat( + getConfigMatchingProvider("//test:refined") + .refines(getConfigMatchingProvider("//test:other"))) + .isFalse(); + } + + @Test + public void doesNotRefineSettingWithSubsetValuesAndDifferentFlagValues() throws Exception { + useConfiguration("--copt=-Dright", "--javacopt=-Dgood"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'refined',", + " flag_values = {", + " ':flag': 'right',", + " },", + " values = {", + " 'copt': '-Dright',", + " 'javacopt': '-Dgood',", + " },", + ")", + "config_setting(", + " name = 'other',", + " flag_values = {", + " ':flag2': 'good',", + " },", + " values = {", + " 'copt': '-Dright',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")", + "config_feature_flag(", + " name = 'flag2',", + " allowed_values = ['good', 'bad'],", + " default_value = 'good',", + ")"); + assertThat( + getConfigMatchingProvider("//test:refined") + .refines(getConfigMatchingProvider("//test:other"))) + .isFalse(); + } + + @Test + public void refinesSettingWithSubsetValuesAndSameFlagValues() throws Exception { + useConfiguration("--copt=-Dright", "--javacopt=-Dgood"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'refined',", + " flag_values = {", + " ':flag': 'right',", + " ':flag2': 'good',", + " },", + " values = {", + " 'copt': '-Dright',", + " 'javacopt': '-Dgood',", + " },", + ")", + "config_setting(", + " name = 'other',", + " flag_values = {", + " ':flag': 'right',", + " ':flag2': 'good',", + " },", + " values = {", + " 'copt': '-Dright',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")", + "config_feature_flag(", + " name = 'flag2',", + " allowed_values = ['good', 'bad'],", + " default_value = 'good',", + ")"); + assertThat( + getConfigMatchingProvider("//test:refined") + .refines(getConfigMatchingProvider("//test:other"))) + .isTrue(); + } + + @Test + public void refinesSettingWithSameValuesAndSubsetFlagValues() throws Exception { + useConfiguration("--copt=-Dright", "--javacopt=-Dgood"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'refined',", + " flag_values = {", + " ':flag': 'right',", + " ':flag2': 'good',", + " },", + " values = {", + " 'copt': '-Dright',", + " 'javacopt': '-Dgood',", + " },", + ")", + "config_setting(", + " name = 'other',", + " flag_values = {", + " ':flag': 'right',", + " },", + " values = {", + " 'copt': '-Dright',", + " 'javacopt': '-Dgood',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")", + "config_feature_flag(", + " name = 'flag2',", + " allowed_values = ['good', 'bad'],", + " default_value = 'good',", + ")"); + assertThat( + getConfigMatchingProvider("//test:refined") + .refines(getConfigMatchingProvider("//test:other"))) + .isTrue(); + } + + @Test + public void refinesSettingWithSubsetValuesAndSubsetFlagValues() throws Exception { + useConfiguration("--copt=-Dright", "--javacopt=-Dgood"); + scratch.file( + "test/BUILD", + "config_setting(", + " name = 'refined',", + " flag_values = {", + " ':flag': 'right',", + " ':flag2': 'good',", + " },", + " values = {", + " 'copt': '-Dright',", + " 'javacopt': '-Dgood',", + " },", + ")", + "config_setting(", + " name = 'other',", + " flag_values = {", + " ':flag': 'right',", + " },", + " values = {", + " 'copt': '-Dright',", + " },", + ")", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'wrong'],", + " default_value = 'right',", + ")", + "config_feature_flag(", + " name = 'flag2',", + " allowed_values = ['good', 'bad'],", + " default_value = 'good',", + ")"); + assertThat( + getConfigMatchingProvider("//test:refined") + .refines(getConfigMatchingProvider("//test:other"))) + .isTrue(); + } + + @Test + public void forbidsNonConfigFeatureFlagRulesForFlagValues() throws Exception { + checkError("test", "invalid_flag", + "in flag_values attribute of config_setting rule //test:invalid_flag: " + + "'//test:genrule' does not have mandatory provider 'FeatureFlagInfo'", + "config_setting(", + " name = 'invalid_flag',", + " flag_values = {", + " ':genrule': 'lolz',", + " })", + "genrule(", + " name = 'genrule',", + " outs = ['output'],", + " cmd = 'echo >$@',", + " )"); + } + + @Test + public void requiresValidValueForFlagValues() throws Exception { + checkError("test", "invalid_flag", + "in flag_values attribute of config_setting rule //test:invalid_flag: " + + "error while parsing user-defined configuration values: " + + "'invalid' is not a valid value for '//test:flag'", + "config_setting(", + " name = 'invalid_flag',", + " flag_values = {", + " ':flag': 'invalid',", + " })", + "config_feature_flag(", + " name = 'flag',", + " allowed_values = ['right', 'valid'],", + " default_value = 'valid',", + ")"); + } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagSetterRule.java b/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagSetterRule.java new file mode 100644 index 0000000000..0f40a0d8b0 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagSetterRule.java @@ -0,0 +1,101 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License + +package com.google.devtools.build.lib.rules.config; + +import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER; +import static com.google.devtools.build.lib.packages.Attribute.attr; +import static com.google.devtools.build.lib.packages.BuildType.LABEL; +import static com.google.devtools.build.lib.packages.BuildType.LABEL_KEYED_STRING_DICT; +import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; + +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.BaseRuleClasses; +import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; +import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.RuleDefinition; +import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; +import com.google.devtools.build.lib.analysis.RunfilesProvider; +import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; +import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.packages.RuleClass; +import com.google.devtools.build.lib.packages.RuleClass.Builder; +import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; + +/** Rule introducing a transition to set feature flags for dependencies. */ +public final class FeatureFlagSetterRule implements RuleDefinition, RuleConfiguredTargetFactory { + + @Override + public RuleClass build(Builder builder, RuleDefinitionEnvironment env) { + return builder + .requiresConfigurationFragments(ConfigFeatureFlagConfiguration.class) + .cfg(new ConfigFeatureFlagTransitionFactory("flag_values")) + .add(attr("deps", LABEL_LIST).allowedFileTypes()) + .add( + attr("exports_setting", LABEL) + .allowedRuleClasses("config_setting") + .allowedFileTypes()) + .add( + attr("exports_flag", LABEL) + .allowedRuleClasses("config_feature_flag") + .allowedFileTypes()) + .add( + attr("flag_values", LABEL_KEYED_STRING_DICT) + .allowedRuleClasses("config_feature_flag") + .allowedFileTypes() + .nonconfigurable("used in RuleTransitionFactory") + .value(ImmutableMap.<Label, String>of())) + .build(); + } + + @Override + public Metadata getMetadata() { + return RuleDefinition.Metadata.builder() + .name("feature_flag_setter") + .ancestors(BaseRuleClasses.BaseRule.class) + .factoryClass(FeatureFlagSetterRule.class) + .build(); + } + + @Override + public ConfiguredTarget create(RuleContext ruleContext) + throws InterruptedException, RuleErrorException { + TransitiveInfoCollection exportedFlag = + ruleContext.getPrerequisite("exports_flag", Mode.TARGET); + ConfigFeatureFlagProvider exportedFlagProvider = + exportedFlag != null ? ConfigFeatureFlagProvider.fromTarget(exportedFlag) : null; + + TransitiveInfoCollection exportedSetting = + ruleContext.getPrerequisite("exports_setting", Mode.TARGET); + ConfigMatchingProvider exportedSettingProvider = + exportedSetting != null ? exportedSetting.getProvider(ConfigMatchingProvider.class) : null; + + RuleConfiguredTargetBuilder builder = + new RuleConfiguredTargetBuilder(ruleContext) + .setFilesToBuild(NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER)) + .addProvider(RunfilesProvider.class, RunfilesProvider.EMPTY); + if (exportedFlagProvider != null) { + builder.addNativeDeclaredProvider(exportedFlagProvider); + } + if (exportedSettingProvider != null) { + builder.addProvider(ConfigMatchingProvider.class, exportedSettingProvider); + } + return builder.build(); + } +} |