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

4 files changed, 46 insertions, 23 deletions

diff --git a/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/features.html b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/features.html
new file mode 100644
index 0000000000..ba4bbb46d7
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/features.html
@@ -0,0 +1,7 @@
+List of <i>features</i>. Default is the empty list.<br/>
+<i>Features</i> on a rule modify the <i>features</i> currently enabled on
+the <a href="#package">package</a> level via the <i>features</i> attribute.<br/>
+For example, if the features ['a', 'b'] are enabled on the package level,
+and a rule <i>features</i> attribute contains ['-a', 'c'], the features
+enabled for the rule will be 'b' and 'c'.
+</p>
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 ac4c9d2708..ee32c59f31 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 @@ public class Constants {
       "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 4b7015f648..dcec485bd7 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 @@ public class BaseRuleClasses {
         .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 8e30ff47e2..932bc2b672 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.ImmutableList;
 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 @@ public final class RuleContext extends TargetContext
 
   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 @@ public final class RuleContext extends TargetContext
     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 @@ public final class RuleContext extends TargetContext
         }
         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) {