Allow conditioning flags on the _absence_ of a feature.

This allows a flag_set to emit one flag when a feature is enabled, and a
different flag when that feature is disabled.

And while I was in there, I noticed and fixed a couple other issues:

1. env_set didn't actually implement with_feature, despite having the field in
   its proto.
2. action_config implemented with_feature as an optional field, instead of
   repeated field.

RELNOTES: None
PiperOrigin-RevId: 176510960

2 files changed, 53 insertions, 24 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
index 679d2c2b38..87a04c9f7f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
@@ -479,12 +479,19 @@ public class CcToolchainFeatures implements Serializable {
   }
 
   private static boolean isWithFeaturesSatisfied(
-      Set<CToolchain.FeatureSet> withFeatureSets, Set<String> enabledFeatureNames) {
+      Collection<CToolchain.WithFeatureSet> withFeatureSets, Set<String> enabledFeatureNames) {
     if (withFeatureSets.isEmpty()) {
       return true;
     }
-    for (CToolchain.FeatureSet featureSet : withFeatureSets) {
-      if (enabledFeatureNames.containsAll(featureSet.getFeatureList())) {
+    for (CToolchain.WithFeatureSet featureSet : withFeatureSets) {
+      boolean negativeMatch =
+          featureSet
+              .getNotFeatureList()
+              .stream()
+              .anyMatch(notFeature -> enabledFeatureNames.contains(notFeature));
+      boolean positiveMatch = enabledFeatureNames.containsAll(featureSet.getFeatureList());
+
+      if (!negativeMatch && positiveMatch) {
         return true;
       }
     }
@@ -498,7 +505,7 @@ public class CcToolchainFeatures implements Serializable {
   private static class FlagSet implements Serializable {
     private final ImmutableSet<String> actions;
     private final ImmutableSet<String> expandIfAllAvailable;
-    private final ImmutableSet<CToolchain.FeatureSet> withFeatureSets;
+    private final ImmutableSet<CToolchain.WithFeatureSet> withFeatureSets;
     private final ImmutableList<FlagGroup> flagGroups;
     
     private FlagSet(CToolchain.FlagSet flagSet) throws InvalidConfigurationException {
@@ -550,7 +557,8 @@ public class CcToolchainFeatures implements Serializable {
   private static class EnvSet implements Serializable {
     private final ImmutableSet<String> actions;
     private final ImmutableList<EnvEntry> envEntries;
-    
+    private final ImmutableSet<CToolchain.WithFeatureSet> withFeatureSets;
+
     private EnvSet(CToolchain.EnvSet envSet) throws InvalidConfigurationException {
       this.actions = ImmutableSet.copyOf(envSet.getActionList());
       ImmutableList.Builder<EnvEntry> builder = ImmutableList.builder();
@@ -558,17 +566,24 @@ public class CcToolchainFeatures implements Serializable {
         builder.add(new EnvEntry(envEntry));
       }
       this.envEntries = builder.build();
+      this.withFeatureSets = ImmutableSet.copyOf(envSet.getWithFeatureList());
     }
 
     /**
      * Adds the environment key/value pairs that apply to the given {@code action} to
      * {@code envBuilder}.
      */
-    private void expandEnvironment(String action, Variables variables,
+    private void expandEnvironment(
+        String action,
+        Variables variables,
+        Set<String> enabledFeatureNames,
         ImmutableMap.Builder<String, String> envBuilder) {
       if (!actions.contains(action)) {
         return;
       }
+      if (!isWithFeaturesSatisfied(withFeatureSets, enabledFeatureNames)) {
+        return;
+      }
       for (EnvEntry envEntry : envEntries) {
         envEntry.addEnvEntry(variables, envBuilder);
       }
@@ -618,13 +633,14 @@ public class CcToolchainFeatures implements Serializable {
       return name;
     }
 
-    /**
-     * Adds environment variables for the given action to the provided builder.
-     */
+    /** Adds environment variables for the given action to the provided builder. */
     private void expandEnvironment(
-        String action, Variables variables, ImmutableMap.Builder<String, String> envBuilder) {
+        String action,
+        Variables variables,
+        Set<String> enabledFeatureNames,
+        ImmutableMap.Builder<String, String> envBuilder) {
       for (EnvSet envSet : envSets) {
-        envSet.expandEnvironment(action, variables, envBuilder);
+        envSet.expandEnvironment(action, variables, enabledFeatureNames, envBuilder);
       }
     }
 
@@ -747,8 +763,7 @@ public class CcToolchainFeatures implements Serializable {
           Iterables.tryFind(
               tools,
               input -> {
-                Collection<String> featureNamesForTool = input.getWithFeature().getFeatureList();
-                return enabledFeatureNames.containsAll(featureNamesForTool);
+                return isWithFeaturesSatisfied(input.getWithFeatureList(), enabledFeatureNames);
               });
       if (tool.isPresent()) {
         return new Tool(tool.get());
@@ -1756,7 +1771,7 @@ public class CcToolchainFeatures implements Serializable {
     ImmutableMap<String, String> getEnvironmentVariables(String action, Variables variables) {
       ImmutableMap.Builder<String, String> envBuilder = ImmutableMap.builder();
       for (Feature feature : enabledFeatures) {
-        feature.expandEnvironment(action, variables, envBuilder);
+        feature.expandEnvironment(action, variables, enabledFeatureNames, envBuilder);
       }
       return envBuilder.build();
     }
diff --git a/src/main/protobuf/crosstool_config.proto b/src/main/protobuf/crosstool_config.proto
index 3ef9ad01c1..d0ae71a584 100644
--- a/src/main/protobuf/crosstool_config.proto
+++ b/src/main/protobuf/crosstool_config.proto
@@ -127,11 +127,19 @@ message CToolchain {
   }
 
   // A set of features; used to support logical 'and' when specifying feature
-  // requirements in FlagSet and Feature.
+  // requirements in Feature.
   message FeatureSet {
     repeated string feature = 1;
   }
 
+  // A set of positive and negative features. This stanza will
+  // evaluate to true when every 'feature' is enabled, and every
+  // 'not_feature' is not enabled.
+  message WithFeatureSet {
+    repeated string feature = 1;
+    repeated string not_feature = 2;
+  }
+
   // A set of flags that are expanded in the command line for specific actions.
   message FlagSet {
     // The actions this flag set applies to; each flag set must specify at
@@ -142,12 +150,13 @@ message CToolchain {
     repeated FlagGroup flag_group = 2;
 
     // A list of feature sets defining when this flag set gets applied.  The
-    // flag set will be applied when any of the feature sets fully apply, that
-    // is, when all features of the feature set are enabled.
+    // flag set will be applied when any one of the feature sets evaluate to
+    // true. (That is, when when every 'feature' is enabled, and every
+    // 'not_feature' is not enabled.)
     //
     // If 'with_feature' is omitted, the flag set will be applied
     // unconditionally for every action specified.
-    repeated FeatureSet with_feature = 3;
+    repeated WithFeatureSet with_feature = 3;
 
     // A list of build variables that this feature set needs, but which are
     // allowed to not be set. If any of the build variables listed is not
@@ -171,12 +180,13 @@ message CToolchain {
     repeated EnvEntry env_entry = 2;
 
     // A list of feature sets defining when this env set gets applied.  The
-    // env set will be applied when any of the feature sets fully apply, that
-    // is, when all features of the feature set are enabled.
+    // env set will be applied when any one of the feature sets evaluate to
+    // true. (That is, when when every 'feature' is enabled, and every
+    // 'not_feature' is not enabled.)
     //
     // If 'with_feature' is omitted, the env set will be applied
     // unconditionally for every action specified.
-    repeated FeatureSet with_feature = 3;
+    repeated WithFeatureSet with_feature = 3;
   }
 
   // Contains all flag specifications for one feature.
@@ -235,10 +245,14 @@ message CToolchain {
     // Path to the tool, relative to the location of the crosstool.
     required string tool_path = 1;
 
-    // A feature set defining when this tool is applicable.  If this attribute
-    // is left out, the Tool will be assumed to apply for any feature
+    // A list of feature sets defining when this tool is applicable.  The tool
+    // will used when any one of the feature sets evaluate to true. (That is,
+    // when when every 'feature' is enabled, and every 'not_feature' is not
+    // enabled.)
+    //
+    // If 'with_feature' is omitted, the tool will apply for any feature
     // configuration.
-    optional FeatureSet with_feature = 2;
+    repeated WithFeatureSet with_feature = 2;
 
     // Requirements on the execution environment for the execution of this tool,
     // to be passed as out-of-band "hints" to the execution backend.