Implements an attribute 'features' that allows overriding package level
features.
Features on the rule level modify features that are enabled at the package
level. Note that this behavior is different from how the current command line /
package level interaction is, but we probably want to change the command line
behavior.
Alternative implementations considered:
a) using package-level features as default value for the rule attribute; this
would make it hard for future transitions; adding a completely new feature
to a package should not require updating all rules that have overrides
b) putting all positive features and all negative features from command-line,
package, and rule attribute into a positive and negative set, and subtract
the negative from the positive set; this is how the command-line features
worked previously, but it makes it impossible to enable a features that
is disabled at the package level just for one rule.
RELNOTES: Add 'features' attribute on the rule level.
--
MOS_MIGRATED_REVID=92448449
diff --git a/src/main/java/com/google/devtools/build/lib/Constants.java b/src/main/java/com/google/devtools/build/lib/Constants.java
index ac4c9d2..ee32c59 100644
--- a/src/main/java/com/google/devtools/build/lib/Constants.java
+++ b/src/main/java/com/google/devtools/build/lib/Constants.java
@@ -44,6 +44,7 @@
"templates/attributes/common/deprecation.html",
"templates/attributes/common/deps.html",
"templates/attributes/common/distribs.html",
+ "templates/attributes/common/features.html",
"templates/attributes/common/licenses.html",
"templates/attributes/common/tags.html",
"templates/attributes/common/testonly.html",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
index 4b7015f..dcec485 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
@@ -196,6 +196,7 @@
.add(attr("generator_function", STRING).undocumented("internal"))
.add(attr("testonly", BOOLEAN).value(testonlyDefault)
.nonconfigurable("policy decision: rules testability should be consistent"))
+ .add(attr("features", STRING_LIST).orderIndependent())
.add(attr(RuleClass.COMPATIBLE_ENVIRONMENT_ATTR, LABEL_LIST)
.allowedRuleClasses(EnvironmentRule.RULE_NAME)
.cfg(Attribute.ConfigurationTransition.HOST)
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 8e30ff4..932bc2b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Multimaps;
@@ -140,8 +141,7 @@
private RuleContext(Builder builder, ListMultimap<String, ConfiguredTarget> targetMap,
ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap,
- Set<ConfigMatchingProvider> configConditions, ImmutableSet<String> features,
- Map<String, Attribute> attributeMap) {
+ Set<ConfigMatchingProvider> configConditions, Map<String, Attribute> attributeMap) {
super(builder.env, builder.rule, builder.configuration, builder.prerequisiteMap.get(null),
builder.visibility);
this.rule = builder.rule;
@@ -150,10 +150,43 @@
this.configConditions = configConditions;
this.attributes =
ConfiguredAttributeMapper.of(builder.rule, configConditions);
- this.features = features;
+ this.features = getEnabledFeatures();
this.attributeMap = attributeMap;
}
+ private ImmutableSet<String> getEnabledFeatures() {
+ Set<String> globallyEnabled = new HashSet<>();
+ Set<String> globallyDisabled = new HashSet<>();
+ parseFeatures(getConfiguration().getDefaultFeatures(), globallyEnabled, globallyDisabled);
+ Set<String> packageEnabled = new HashSet<>();
+ Set<String> packageDisabled = new HashSet<>();
+ parseFeatures(getRule().getPackage().getFeatures(), packageEnabled, packageDisabled);
+ Set<String> packageFeatures =
+ Sets.difference(Sets.union(globallyEnabled, packageEnabled), packageDisabled);
+ Set<String> ruleFeatures = packageFeatures;
+ if (attributes().has("features", Type.STRING_LIST)) {
+ Set<String> ruleEnabled = new HashSet<>();
+ Set<String> ruleDisabled = new HashSet<>();
+ parseFeatures(attributes().get("features", Type.STRING_LIST), ruleEnabled, ruleDisabled);
+ ruleFeatures = Sets.difference(Sets.union(packageFeatures, ruleEnabled), ruleDisabled);
+ }
+ return ImmutableSortedSet.copyOf(Sets.difference(ruleFeatures, globallyDisabled));
+ }
+
+ private void parseFeatures(Iterable<String> features, Set<String> enabled, Set<String> disabled) {
+ for (String feature : features) {
+ if (feature.startsWith("-")) {
+ disabled.add(feature.substring(1));
+ } else if (feature.equals("no_layering_check")) {
+ // TODO(bazel-team): Remove once we do not have BUILD files left that contain
+ // 'no_layering_check'.
+ disabled.add(feature.substring(3));
+ } else {
+ enabled.add(feature);
+ }
+ }
+ }
+
@Override
public Rule getRule() {
return rule;
@@ -1125,26 +1158,7 @@
}
attributeMap.put(attribute.getName(), attribute);
}
- return new RuleContext(this, targetMap, filesetEntryMap, configConditions,
- getEnabledFeatures(), attributeMap);
- }
-
- private ImmutableSet<String> getEnabledFeatures() {
- Set<String> enabled = new HashSet<>();
- Set<String> disabled = new HashSet<>();
- for (String feature : Iterables.concat(getConfiguration().getDefaultFeatures(),
- getRule().getPackage().getFeatures())) {
- if (feature.startsWith("-")) {
- disabled.add(feature.substring(1));
- } else if (feature.equals("no_layering_check")) {
- // TODO(bazel-team): Remove once we do not have BUILD files left that contain
- // 'no_layering_check'.
- disabled.add(feature.substring(3));
- } else {
- enabled.add(feature);
- }
- }
- return Sets.difference(enabled, disabled).immutableCopy();
+ return new RuleContext(this, targetMap, filesetEntryMap, configConditions, attributeMap);
}
Builder setVisibility(NestedSet<PackageSpecification> visibility) {
