diff options
Diffstat (limited to 'src/main/java/com/google/devtools')
4 files changed, 144 insertions, 25 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)) { |