Generalize ConfigSetting's special options handling.
Currently ConfigSetting is treated specially; TransitiveTargetFunction
identifies it by name, then runs a special function on it. This change
makes ConfigSetting use a new options reference function in RuleClass,
which TransitiveTargetFunction will evaluate over the rule and use with
the options-to-fragments map to figure out which fragments are needed.
Although this is still a hack and not really a new feature on RuleClass,
this avoids a dependency on ConfigSettingRule from TransitiveTargetFunction,
which is necessary for ConfigSettingRule to be moved to rules/config.
RELNOTES: None.
PiperOrigin-RevId: 152156905
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigRuleClasses.java
index 7df805c..d79e3c9 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigRuleClasses.java
@@ -17,19 +17,17 @@
import static com.google.devtools.build.lib.packages.Attribute.attr;
import static com.google.devtools.build.lib.syntax.Type.STRING_DICT;
+import com.google.common.base.Function;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
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.packages.AttributeMap;
import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.syntax.Type;
-
-import java.util.List;
-import java.util.Map;
+import java.util.Set;
/**
* Definitions for rule classes that specify or manipulate configuration settings.
@@ -108,6 +106,16 @@
*/
public static final String SETTINGS_ATTRIBUTE = "values";
+ private static final Function<Rule, Set<String>> CONFIG_SETTING_OPTION_REFERENCE =
+ new Function<Rule, Set<String>>() {
+ @Override
+ public Set<String> apply(Rule rule) {
+ return NonconfigurableAttributeMapper.of(rule)
+ .get(SETTINGS_ATTRIBUTE, Type.STRING_DICT)
+ .keySet();
+ }
+ };
+
@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
return builder
@@ -145,9 +153,12 @@
<p>This attribute cannot be empty.
</p>
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
- .add(attr(SETTINGS_ATTRIBUTE, STRING_DICT).mandatory()
- .nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON))
+ .add(
+ attr(SETTINGS_ATTRIBUTE, STRING_DICT)
+ .mandatory()
+ .nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON))
.setIsConfigMatcherForConfigSettingOnly()
+ .setOptionReferenceFunctionForConfigSettingOnly(CONFIG_SETTING_OPTION_REFERENCE)
.build();
}
@@ -160,28 +171,7 @@
.factoryClass(ConfigSetting.class)
.build();
}
-
- /**
- * config_setting can't use {@link RuleClass.Builder#requiresConfigurationFragments} because
- * config_setting's dependencies come from option names as strings. This special override
- * computes that properly.
- */
- public static List<Class<? extends BuildConfiguration.Fragment>> requiresConfigurationFragments(
- Rule rule, Map<String, Class<? extends BuildConfiguration.Fragment>> optionsToFragmentMap) {
- ImmutableList.Builder<Class<? extends BuildConfiguration.Fragment>> builder =
- ImmutableList.builder();
- AttributeMap attributes = NonconfigurableAttributeMapper.of(rule);
- for (String optionName : attributes.get(SETTINGS_ATTRIBUTE, Type.STRING_DICT).keySet()) {
- Class<? extends BuildConfiguration.Fragment> value = optionsToFragmentMap.get(optionName);
- // Null values come from BuildConfiguration.Options, which is implicitly included.
- if (value != null) {
- builder.add(value);
- }
- }
- return builder.build();
- }
}
-
/*<!-- #BLAZE_RULE (NAME = config_setting, TYPE = OTHER, FAMILY = General)[GENERIC_RULE] -->
<p>
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 1324cce..d8c06f5 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -106,6 +106,8 @@
public class RuleClass {
static final Function<? super Rule, Map<String, Label>> NO_EXTERNAL_BINDINGS =
Functions.<Map<String, Label>>constant(ImmutableMap.<String, Label>of());
+ static final Function<? super Rule, Set<String>> NO_OPTION_REFERENCE =
+ Functions.<Set<String>>constant(ImmutableSet.<String>of());
public static final PathFragment THIRD_PARTY_PREFIX = PathFragment.create("third_party");
@@ -496,6 +498,8 @@
private BaseFunction configuredTargetFunction = null;
private Function<? super Rule, Map<String, Label>> externalBindingsFunction =
NO_EXTERNAL_BINDINGS;
+ private Function<? super Rule, ? extends Set<String>> optionReferenceFunction =
+ NO_OPTION_REFERENCE;
@Nullable private Environment ruleDefinitionEnvironment = null;
@Nullable private String ruleDefinitionEnvironmentHashCode = null;
private ConfigurationFragmentPolicy.Builder configurationFragmentPolicy =
@@ -609,6 +613,7 @@
advertisedProviders.build(),
configuredTargetFunction,
externalBindingsFunction,
+ optionReferenceFunction,
ruleDefinitionEnvironment,
ruleDefinitionEnvironmentHashCode,
configurationFragmentPolicy.build(),
@@ -982,6 +987,18 @@
}
/**
+ * Causes rules of this type to implicitly reference the configuration fragments associated with
+ * the options its attributes reference.
+ *
+ * <p>This is only intended for use by {@code config_setting} - other rules should not use this!
+ */
+ public Builder setOptionReferenceFunctionForConfigSettingOnly(
+ Function<? super Rule, ? extends Set<String>> optionReferenceFunction) {
+ this.optionReferenceFunction = Preconditions.checkNotNull(optionReferenceFunction);
+ return this;
+ }
+
+ /**
* Returns an Attribute.Builder object which contains a replica of the
* same attribute in the parent rule if exists.
*
@@ -1078,6 +1095,11 @@
private final Function<? super Rule, Map<String, Label>> externalBindingsFunction;
/**
+ * Returns the options referenced by this rule's attributes.
+ */
+ private final Function<? super Rule, ? extends Set<String>> optionReferenceFunction;
+
+ /**
* The Skylark rule definition environment of this RuleClass.
* Null for non Skylark executable RuleClasses.
*/
@@ -1138,6 +1160,7 @@
AdvertisedProviderSet advertisedProviders,
@Nullable BaseFunction configuredTargetFunction,
Function<? super Rule, Map<String, Label>> externalBindingsFunction,
+ Function<? super Rule, ? extends Set<String>> optionReferenceFunction,
@Nullable Environment ruleDefinitionEnvironment,
String ruleDefinitionEnvironmentHashCode,
ConfigurationFragmentPolicy configurationFragmentPolicy,
@@ -1161,6 +1184,7 @@
this.advertisedProviders = advertisedProviders;
this.configuredTargetFunction = configuredTargetFunction;
this.externalBindingsFunction = externalBindingsFunction;
+ this.optionReferenceFunction = optionReferenceFunction;
this.ruleDefinitionEnvironment = ruleDefinitionEnvironment;
this.ruleDefinitionEnvironmentHashCode = ruleDefinitionEnvironmentHashCode;
validateNoClashInPublicNames(attributes);
@@ -1924,6 +1948,13 @@
}
/**
+ * Returns a function that computes the options referenced by a rule.
+ */
+ public Function<? super Rule, ? extends Set<String>> getOptionReferenceFunction() {
+ return optionReferenceFunction;
+ }
+
+ /**
* Returns this RuleClass's rule definition environment. Is null for native rules' RuleClass
* objects and deserialized Skylark rules. Deserialized rules do provide a hash code encapsulating
* their behavior, available at {@link #getRuleDefinitionEnvironmentHashCode}.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
index b620259..23d22b4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
@@ -14,10 +14,10 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
-import com.google.devtools.build.lib.analysis.config.ConfigRuleClasses.ConfigSettingRule;
import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.cmdline.Label;
@@ -210,10 +210,7 @@
// config_setting rules have values like {"some_flag": "some_value"} that need the
// corresponding fragments in their configurations to properly resolve:
- if (rule.getRuleClass().equals(ConfigSettingRule.RULE_NAME)) {
- addFragmentsIfNew(builder,
- ConfigSettingRule.requiresConfigurationFragments(rule, optionsToFragmentMap));
- }
+ addFragmentsIfNew(builder, getFragmentsFromRequiredOptions(rule));
// Fragments to unconditionally include:
addFragmentIfNew(builder,
@@ -223,6 +220,22 @@
return builder.build(errorLoadingTarget);
}
+ private Set<Class<? extends Fragment>> getFragmentsFromRequiredOptions(Rule rule) {
+ Set<String> requiredOptions =
+ rule.getRuleClassObject().getOptionReferenceFunction().apply(rule);
+ ImmutableSet.Builder<Class<? extends BuildConfiguration.Fragment>> optionsFragments =
+ new ImmutableSet.Builder<>();
+ for (String requiredOption : requiredOptions) {
+ Class<? extends BuildConfiguration.Fragment> fragment =
+ optionsToFragmentMap.get(requiredOption);
+ // Null values come from BuildConfiguration.Options, which is implicitly included.
+ if (fragment != null) {
+ optionsFragments.add(fragment);
+ }
+ }
+ return optionsFragments.build();
+ }
+
private void addFragmentIfNew(TransitiveTargetValueBuilder builder,
Class<? extends Fragment> fragment) {
// This only checks that the deps don't already use this fragment, not the parent rule itself.
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/ConfigSettingTest.java
index 44a15de..c277b31 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/config/ConfigSettingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/config/ConfigSettingTest.java
@@ -21,7 +21,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
-import com.google.devtools.build.lib.analysis.config.ConfigRuleClasses.ConfigSettingRule;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Rule;
@@ -328,8 +327,8 @@
"unused", PythonConfiguration.class,
"javacopt", Jvm.class
);
- assertThat(
- ConfigSettingRule.requiresConfigurationFragments((Rule) getTarget("//test:match"), map))
- .containsExactly(CppConfiguration.class, Jvm.class);
+ Rule target = (Rule) getTarget("//test:match");
+ assertThat(target.getRuleClassObject().getOptionReferenceFunction().apply(target))
+ .containsExactly("copt", "javacopt");
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
index 38a8e0a..dab6130 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
@@ -896,6 +896,7 @@
advertisedProviders,
configuredTargetFunction,
externalBindingsFunction,
+ /*optionReferenceFunction=*/ RuleClass.NO_OPTION_REFERENCE,
ruleDefinitionEnvironment,
ruleDefinitionEnvironmentHashCode,
new ConfigurationFragmentPolicy.Builder()
