Action configs are activated like features (instead of being activated by default). Action configs can imply features.

--
MOS_MIGRATED_REVID=122032003

6 files changed, 220 insertions, 46 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index 9c9b19cc12..06f2ef176f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -13,8 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.rules.cpp;
 
-import static com.google.devtools.build.lib.rules.cpp.CcLibraryHelper.SourceCategory;
-
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
@@ -33,6 +31,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.rules.apple.Platform;
+import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper.SourceCategory;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode;
@@ -519,12 +518,14 @@ public final class CcCommon {
    * @param ruleSpecificRequestedFeatures features that will be requested, and thus be always
    * enabled if the toolchain supports them.
    * @param ruleSpecificUnsupportedFeatures features that are not supported in the current context.
+   * @param sourceCategory the source category for this build.
    * @return the feature configuration for the given {@code ruleContext}.
    */
   public static FeatureConfiguration configureFeatures(
       RuleContext ruleContext,
       Set<String> ruleSpecificRequestedFeatures,
       Set<String> ruleSpecificUnsupportedFeatures,
+      SourceCategory sourceCategory,
       CcToolchainProvider toolchain) {
     ImmutableSet.Builder<String> unsupportedFeaturesBuilder = ImmutableSet.builder();
     unsupportedFeaturesBuilder.addAll(ruleSpecificUnsupportedFeatures);
@@ -548,6 +549,8 @@ public final class CcCommon {
     }
     requestedFeatures.addAll(ruleSpecificRequestedFeatures);
 
+    requestedFeatures.addAll(sourceCategory.getActionConfigSet());
+
     FeatureConfiguration configuration =
         toolchain.getFeatures().getFeatureConfiguration(requestedFeatures.build());
     for (String feature : unsupportedFeatures) {
@@ -561,27 +564,44 @@ public final class CcCommon {
     }
     return configuration; 
   }
-  
+ 
   /**
    * Creates a feature configuration for a given rule.
    *
    * @param ruleContext the context of the rule we want the feature configuration for.
    * @param toolchain the toolchain we want the feature configuration for.
+   * @param sourceCategory the category of sources to be used in this build.
    * @return the feature configuration for the given {@code ruleContext}.
    */
   public static FeatureConfiguration configureFeatures(
-      RuleContext ruleContext, CcToolchainProvider toolchain) {
+      RuleContext ruleContext, CcToolchainProvider toolchain, SourceCategory sourceCategory) {
     return configureFeatures(
-        ruleContext, ImmutableSet.<String>of(), ImmutableSet.<String>of(), toolchain);
+        ruleContext,
+        ImmutableSet.<String>of(),
+        ImmutableSet.<String>of(),
+        sourceCategory,
+        toolchain);
   }
 
   /**
    * Creates a feature configuration for a given rule.
    *
+   * @param ruleContext the context of the rule we want the feature configuraiton for.
+   * @param sourceCategory the category of sources to be used in this build.
+   * @return the feature configuration for the given {@code ruleContext}.
+   */
+  public static FeatureConfiguration configureFeatures(
+      RuleContext ruleContext, SourceCategory sourceCategory) {
+    return configureFeatures(ruleContext, CppHelper.getToolchain(ruleContext), sourceCategory);
+  }
+
+  /**
+   * Creates a feature configuration for a given rule.  Assumes strictly cc sources.
+   *
    * @param ruleContext the context of the rule we want the feature configuration for.
    * @return the feature configuration for the given {@code ruleContext}.
    */
   public static FeatureConfiguration configureFeatures(RuleContext ruleContext) {
-    return configureFeatures(ruleContext, CppHelper.getToolchain(ruleContext));
+    return configureFeatures(ruleContext, SourceCategory.CC);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
index 8bbfc5d512..a938294121 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
@@ -18,6 +18,7 @@ import com.google.common.base.Function;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
@@ -72,8 +73,9 @@ import javax.annotation.Nullable;
 public final class CcLibraryHelper {
 
   /**
-   * A group of source file types for builds controlled by CcLibraryHelper.  Determines what
-   * file types CcLibraryHelper considers sources.
+   * A group of source file types and action names for builds controlled by CcLibraryHelper.
+   * Determines what file types CcLibraryHelper considers sources and what action configs are
+   * configured in the CROSSTOOL.
    */
   public static enum SourceCategory {
     CC(
@@ -82,7 +84,15 @@ public final class CcLibraryHelper {
             CppFileTypes.CPP_HEADER,
             CppFileTypes.C_SOURCE,
             CppFileTypes.ASSEMBLER,
-            CppFileTypes.ASSEMBLER_WITH_C_PREPROCESSOR)),
+            CppFileTypes.ASSEMBLER_WITH_C_PREPROCESSOR),
+        ImmutableSet.<String>of(
+            CppCompileAction.C_COMPILE,
+            CppCompileAction.CPP_COMPILE,
+            CppCompileAction.CPP_HEADER_PARSING,
+            CppCompileAction.CPP_HEADER_PREPROCESSING,
+            CppCompileAction.CPP_MODULE_COMPILE,
+            CppCompileAction.ASSEMBLE,
+            CppCompileAction.PREPROCESS_ASSEMBLE)),
     CC_AND_OBJC(
         FileTypeSet.of(
             CppFileTypes.CPP_SOURCE,
@@ -91,20 +101,40 @@ public final class CcLibraryHelper {
             CppFileTypes.OBJCPP_SOURCE,
             CppFileTypes.C_SOURCE,
             CppFileTypes.ASSEMBLER,
-            CppFileTypes.ASSEMBLER_WITH_C_PREPROCESSOR));
+            CppFileTypes.ASSEMBLER_WITH_C_PREPROCESSOR),
+        ImmutableSet.<String>of(
+            CppCompileAction.C_COMPILE,
+            CppCompileAction.CPP_COMPILE,
+            CppCompileAction.OBJC_COMPILE,
+            CppCompileAction.OBJCPP_COMPILE,
+            CppCompileAction.CPP_HEADER_PARSING,
+            CppCompileAction.CPP_HEADER_PREPROCESSING,
+            CppCompileAction.CPP_MODULE_COMPILE,
+            CppCompileAction.ASSEMBLE,
+            CppCompileAction.PREPROCESS_ASSEMBLE));
+            
 
     private final FileTypeSet sourceTypeSet;
+    private final Set<String> actionConfigSet;
 
-    private SourceCategory(FileTypeSet sourceTypeSet) {
+    private SourceCategory(FileTypeSet sourceTypeSet, Set<String> actionConfigSet) {
       this.sourceTypeSet = sourceTypeSet;
+      this.actionConfigSet = actionConfigSet;
     }
 
     /**
-     * Returns the set of file types that are valid for this catagory.
+     * Returns the set of file types that are valid for this category.
      */
     public FileTypeSet getSourceTypes() {
       return sourceTypeSet;
     }
+    
+    /**
+     * Returns the set of enabled actions for this category.
+     */
+    public Set<String> getActionConfigSet() {
+      return actionConfigSet;
+    }
   }
 
   /** Function for extracting module maps from CppCompilationDependencies. */
@@ -251,6 +281,15 @@ public final class CcLibraryHelper {
     this.sourceCatagory = Preconditions.checkNotNull(sourceCatagory);
   }
 
+  public CcLibraryHelper(
+      RuleContext ruleContext, CppSemantics semantics, SourceCategory sourceCategory) {
+    this(
+        ruleContext,
+        semantics,
+        CcCommon.configureFeatures(ruleContext, sourceCategory),
+        sourceCategory);
+  }
+  
   /**
    * Creates a CcLibraryHelper for cpp source files.
    *
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 f04220c62c..1609f9d0b3 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
@@ -1015,6 +1015,7 @@ public class CcToolchainFeatures implements Serializable {
   public static class FeatureConfiguration {
     private final ImmutableSet<String> enabledFeatureNames;
     private final Iterable<Feature> enabledFeatures;
+    private final ImmutableSet<String> enabledActionConfigActionNames;
     
     private final ImmutableMap<String, ActionConfig> actionConfigByActionName;
     
@@ -1037,11 +1038,12 @@ public class CcToolchainFeatures implements Serializable {
         featureBuilder.add(feature.getName());
       }
       this.enabledFeatureNames = featureBuilder.build();
-
+      
       ImmutableSet.Builder<String> actionConfigBuilder = ImmutableSet.builder();
       for (ActionConfig actionConfig : enabledActionConfigs) {
-        actionConfigBuilder.add(actionConfig.getName());
+        actionConfigBuilder.add(actionConfig.getActionName());
       }
+      this.enabledActionConfigActionNames = actionConfigBuilder.build();
     }
     
     /**
@@ -1055,9 +1057,9 @@ public class CcToolchainFeatures implements Serializable {
      * @return whether an action config for the blaze action with the given name is enabled.
      */
     boolean actionIsConfigured(String actionName) {
-      return actionConfigByActionName.containsKey(actionName);
+      return enabledActionConfigActionNames.contains(actionName);
     }
-    
+
     /**
      * @return the command line for the given {@code action}.
      */
@@ -1166,7 +1168,7 @@ public class CcToolchainFeatures implements Serializable {
 
     // Also build a map from action -> action_config, for use in tool lookups
     ImmutableMap.Builder<String, ActionConfig> actionConfigsByActionName = ImmutableMap.builder();
- 
+
     for (CToolchain.Feature toolchainFeature : toolchain.getFeatureList()) {
       Feature feature = new Feature(toolchainFeature);
       selectablesBuilder.add(feature);
@@ -1198,7 +1200,7 @@ public class CcToolchainFeatures implements Serializable {
         ImmutableMultimap.builder();
     ImmutableMultimap.Builder<CrosstoolSelectable, CrosstoolSelectable> requiredBy =
         ImmutableMultimap.builder();
-    
+
     for (CToolchain.Feature toolchainFeature : toolchain.getFeatureList()) {
       String name = toolchainFeature.getName();
       CrosstoolSelectable selectable = selectablesByName.get(name);
@@ -1217,8 +1219,17 @@ public class CcToolchainFeatures implements Serializable {
         implies.put(selectable, implied);
       }
     }
-    
-    
+
+    for (CToolchain.ActionConfig toolchainActionConfig : toolchain.getActionConfigList()) {
+      String name = toolchainActionConfig.getConfigName();
+      CrosstoolSelectable selectable = selectablesByName.get(name);
+      for (String impliedName : toolchainActionConfig.getImpliesList()) {
+        CrosstoolSelectable implied = getActivatableOrFail(impliedName, name);
+        impliedBy.put(implied, selectable);
+        implies.put(selectable, implied);
+      }
+    }
+
     this.implies = implies.build();
     this.requires = requires.build();
     this.impliedBy = impliedBy.build();
@@ -1342,28 +1353,26 @@ public class CcToolchainFeatures implements Serializable {
   private class FeatureSelection {
     
     /**
-     * The features Bazel would like to enable; either because they are supported and generally
-     * useful, or because the user required them (for example through the command line). 
+     * The selectables Bazel would like to enable; either because they are supported and generally
+     * useful, or because the user required them (for example through the command line).
      */
-    private final ImmutableSet<Feature> requestedFeatures;
+    private final ImmutableSet<CrosstoolSelectable> requestedSelectables;
     
     /**
-     * The currently enabled selectable; during feature selection, we first put all features
-     * reachable via an 'implies' edge into the enabled feature set, and than prune that set
-     * from features that have unmet requirements.
+     * The currently enabled selectable; during feature selection, we first put all selectables
+     * reachable via an 'implies' edge into the enabled selectable set, and than prune that set
+     * from selectables that have unmet requirements.
      */
     private final Set<CrosstoolSelectable> enabled = new HashSet<>();
     
-    private FeatureSelection(Collection<String> requestedFeatures) {
-      ImmutableSet.Builder<Feature> builder = ImmutableSet.builder();
-      for (String name : requestedFeatures) {
+    private FeatureSelection(Collection<String> requestedSelectables) {
+      ImmutableSet.Builder<CrosstoolSelectable> builder = ImmutableSet.builder();
+      for (String name : requestedSelectables) {
         if (selectablesByName.containsKey(name)) {
-          if (selectablesByName.get(name) instanceof Feature) {
-            builder.add((Feature) selectablesByName.get(name));
-          }
+          builder.add(selectablesByName.get(name));
         }
       }
-      this.requestedFeatures = builder.build();
+      this.requestedSelectables = builder.build();
     }
 
     /**
@@ -1371,9 +1380,10 @@ public class CcToolchainFeatures implements Serializable {
      * action configs.
      */
     private FeatureConfiguration run() {
-      for (Feature feature : requestedFeatures) {
-        enableAllImpliedBy(feature);
+      for (CrosstoolSelectable selectable : requestedSelectables) {
+        enableAllImpliedBy(selectable);
       }
+
       disableUnsupportedActivatables();
       ImmutableList.Builder<CrosstoolSelectable> enabledActivatablesInOrderBuilder =
           ImmutableList.builder();
@@ -1450,17 +1460,18 @@ public class CcToolchainFeatures implements Serializable {
     }
 
     /**
-     * @return whether all requirements of the feature are met in the set of currently enabled
-     * features.
+     * @return whether all requirements of the selectable are met in the set of currently enabled
+     * selectables.
      */
     private boolean isSatisfied(CrosstoolSelectable selectable) {
-      return (requestedFeatures.contains(selectable) || isImpliedByEnabledActivatable(selectable))
+      return (requestedSelectables.contains(selectable)
+              || isImpliedByEnabledActivatable(selectable))
           && allImplicationsEnabled(selectable)
           && allRequirementsMet(selectable);
     }
     
     /**
-     * @return whether a currently enabled feature implies the given feature.
+     * @return whether a currently enabled selectable implies the given selectable.
      */
     private boolean isImpliedByEnabledActivatable(CrosstoolSelectable selectable) {
       return !Collections.disjoint(impliedBy.get(selectable), enabled);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
index 154ecf6f75..33b1306973 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
@@ -799,7 +799,8 @@ public final class CppLinkAction extends AbstractAction implements ExecutionInfo
               .setNeedWholeArchive(needWholeArchive)
               .setParamFile(paramFile)
               .setAllLTOArtifacts(isLTOIndexing ? null : allLTOArtifacts)
-              .setToolchain(toolchain);
+              .setToolchain(toolchain)
+              .setFeatureConfiguration(featureConfiguration);
 
       if (!isLTOIndexing) {
         linkCommandLineBuilder
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
index 79c24e6bef..6aa888369c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
@@ -1021,6 +1021,7 @@ public final class LinkCommandLine extends CommandLine {
     @Nullable private Artifact paramFile;
     @Nullable private Artifact interfaceSoBuilder;
     @Nullable private CcToolchainProvider toolchain;
+    private FeatureConfiguration featureConfiguration;
 
     // This interface is needed to support tests that don't create a
     // ruleContext, in which case the configuration and action owner
@@ -1045,13 +1046,16 @@ public final class LinkCommandLine extends CommandLine {
             Iterables.concat(DEFAULT_LINKSTAMP_OPTIONS, linkstampCompileOptions));
       }
       CcToolchainFeatures.Variables variables = null;
-      FeatureConfiguration featureConfiguration = null;
       // The ruleContext can be null for some tests.
       if (ruleContext != null) {
-        if (toolchain != null) {
-          featureConfiguration = CcCommon.configureFeatures(ruleContext, toolchain);
-        } else {
-          featureConfiguration = CcCommon.configureFeatures(ruleContext);
+        if (featureConfiguration == null) {
+          if (toolchain != null) {
+            featureConfiguration =
+                CcCommon.configureFeatures(
+                    ruleContext, toolchain, CcLibraryHelper.SourceCategory.CC);
+          } else {
+            featureConfiguration = CcCommon.configureFeatures(ruleContext);
+          }
         }
         CcToolchainFeatures.Variables.Builder buildVariables =
             new CcToolchainFeatures.Variables.Builder();
@@ -1095,6 +1099,14 @@ public final class LinkCommandLine extends CommandLine {
     }
 
     /**
+     * Sets the feature configuration for this link action.
+     */
+    public Builder setFeatureConfiguration(FeatureConfiguration featureConfiguration) {
+      this.featureConfiguration = featureConfiguration;
+      return this;
+    }
+    
+    /**
      * Sets the type of the link. It is an error to try to set this to {@link
      * LinkTargetType#INTERFACE_DYNAMIC_LIBRARY}. Note that all the static target types (see {@link
      * LinkTargetType#isStaticLibraryLink}) are equivalent, and there is no check that the output
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
index e35acf5ddc..8707f46565 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
@@ -585,7 +585,56 @@ public class CcToolchainFeaturesTest {
     assertThat(features.getFeatureConfiguration("b").getCommandLine(CppCompileAction.CPP_COMPILE,
         createVariables("v", "1"))).containsExactly("-f", "1");
   }
-  
+
+  @Test
+  public void testActivateActionConfigFromFeature() throws Exception {
+    CcToolchainFeatures toolchainFeatures =
+        buildFeatures(
+            "action_config {",
+            "  config_name: 'action-a'",
+            "  action_name: 'action-a'",
+            "  tool {",
+            "    tool_path: 'toolchain/feature-a'",
+            "    with_feature: { feature: 'feature-a' }",
+            "  }",
+            "}",
+            "feature {",
+            "   name: 'activates-action-a'",
+            "   implies: 'action-a'",
+            "}");
+
+    FeatureConfiguration featureConfiguration =
+        toolchainFeatures.getFeatureConfiguration("activates-action-a");
+
+    assertThat(featureConfiguration.actionIsConfigured("action-a")).isTrue();
+  }
+
+  @Test
+  public void testFeatureCanRequireActionConfig() throws Exception {
+    CcToolchainFeatures toolchainFeatures =
+        buildFeatures(
+            "action_config {",
+            "  config_name: 'action-a'",
+            "  action_name: 'action-a'",
+            "  tool {",
+            "    tool_path: 'toolchain/feature-a'",
+            "    with_feature: { feature: 'feature-a' }",
+            "  }",
+            "}",
+            "feature {",
+            "   name: 'requires-action-a'",
+            "   requires: { feature: 'action-a' }",
+            "}");
+
+    FeatureConfiguration featureConfigurationWithoutAction =
+        toolchainFeatures.getFeatureConfiguration("requires-action-a");
+    assertThat(featureConfigurationWithoutAction.isEnabled("requires-action-a")).isFalse();
+
+    FeatureConfiguration featureConfigurationWithAction =
+        toolchainFeatures.getFeatureConfiguration("action-a", "requires-action-a");
+    assertThat(featureConfigurationWithAction.isEnabled("requires-action-a")).isTrue();
+  }
+
   @Test
   public void testSimpleActionTool() throws Exception {
     FeatureConfiguration configuration =
@@ -718,6 +767,48 @@ public class CcToolchainFeaturesTest {
   }
 
   @Test
+  public void testActivateActionConfigDirectly() throws Exception {
+    CcToolchainFeatures toolchainFeatures =
+        buildFeatures(
+            "action_config {",
+            "  config_name: 'action-a'",
+            "  action_name: 'action-a'",
+            "  tool {",
+            "    tool_path: 'toolchain/feature-a'",
+            "    with_feature: { feature: 'feature-a' }",
+            "  }",
+            "}");
+
+    FeatureConfiguration featureConfiguration =
+        toolchainFeatures.getFeatureConfiguration("action-a");
+
+    assertThat(featureConfiguration.actionIsConfigured("action-a")).isTrue();
+  }
+
+  @Test
+  public void testActionConfigCanActivateFeature() throws Exception {
+    CcToolchainFeatures toolchainFeatures =
+        buildFeatures(
+            "action_config {",
+            "  config_name: 'action-a'",
+            "  action_name: 'action-a'",
+            "  tool {",
+            "    tool_path: 'toolchain/feature-a'",
+            "    with_feature: { feature: 'feature-a' }",
+            "  }",
+            "  implies: 'activated-feature'",
+            "}",
+            "feature {",
+            "   name: 'activated-feature'",
+            "}");
+
+    FeatureConfiguration featureConfiguration =
+        toolchainFeatures.getFeatureConfiguration("action-a");
+
+    assertThat(featureConfiguration.isEnabled("activated-feature")).isTrue();
+  }
+
+  @Test
   public void testInvalidActionConfigurationDuplicateActionConfigs() throws Exception {
     try {
       buildFeatures(