Implements pch in experimental_objc_library as a feature. Provides a mechanism to add build variables through the CcLibraryHelper API.

--
MOS_MIGRATED_REVID=120710713

3 files changed, 61 insertions, 13 deletions

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 3730cd1136..8bbfc5d512 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
@@ -37,6 +37,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
+import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
@@ -105,7 +106,7 @@ public final class CcLibraryHelper {
       return sourceTypeSet;
     }
   }
-  
+
   /** Function for extracting module maps from CppCompilationDependencies. */
   public static final Function<TransitiveInfoCollection, CppModuleMap> CPP_DEPS_TO_MODULES =
     new Function<TransitiveInfoCollection, CppModuleMap>() {
@@ -226,7 +227,8 @@ public final class CcLibraryHelper {
   private boolean checkDepsGenerateCpp = true;
   private boolean emitCompileProviders;
   private SourceCategory sourceCatagory;
-
+  private List<VariablesExtension> variablesExtensions = new ArrayList<>();
+  
   private final FeatureConfiguration featureConfiguration;
 
   /**
@@ -619,6 +621,15 @@ public final class CcLibraryHelper {
   }
 
   /**
+   * Adds a variableExtension to template the crosstool.
+   */
+  public CcLibraryHelper addVariableExtension(VariablesExtension variableExtension) {
+    Preconditions.checkNotNull(variableExtension);
+    this.variablesExtensions.add(variableExtension);
+    return this;
+  }
+  
+  /**
    * Overrides the path for the generated dynamic library - this should only be called if the
    * dynamic library is an implicit or explicit output of the rule, i.e., if it is accessible by
    * name from other rules in the same package. Set to {@code null} to use the default computation.
@@ -897,7 +908,8 @@ public final class CcLibraryHelper {
         .setNoCopts(nocopts)
         .setDynamicLibrary(dynamicLibrary)
         .addLinkopts(linkopts)
-        .setFeatureConfiguration(featureConfiguration);
+        .setFeatureConfiguration(featureConfiguration)
+        .addVariablesExtension(variablesExtensions);
   }
 
   /**
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 b6d9e10c94..f04220c62c 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
@@ -782,14 +782,14 @@ public class CcToolchainFeatures implements Serializable {
     /**
      * Builder for {@code Variables}.
      */
-    static class Builder {
+    public static class Builder {
       private final ImmutableMap.Builder<String, String> variables = ImmutableMap.builder();
       private final ImmutableMap.Builder<String, Sequence> expandables = ImmutableMap.builder();
       
       /**
        * Add a variable that expands {@code name} to {@code value}.
        */
-      Builder addVariable(String name, String value) {
+      public Builder addVariable(String name, String value) {
         variables.put(name, value);
         return this;
       }
@@ -797,7 +797,7 @@ public class CcToolchainFeatures implements Serializable {
       /**
        * Add all variables in a map.
        */
-      Builder addAllVariables(Map<String, String> variableMap) {
+      public Builder addAllVariables(Map<String, String> variableMap) {
         variables.putAll(variableMap);
         return this;
       }
@@ -805,7 +805,7 @@ public class CcToolchainFeatures implements Serializable {
       /**
        * Add a nested sequence that expands {@code name} recursively.
        */
-      Builder addSequence(String name, Sequence sequence) {
+      public Builder addSequence(String name, Sequence sequence) {
         expandables.put(name, sequence);
         return this;
       }
@@ -814,7 +814,7 @@ public class CcToolchainFeatures implements Serializable {
        * Add a variable that expands a flag group containing a reference to {@code name} for each
        * entry in {@code values}.
        */
-      Builder addSequenceVariable(String name, Collection<String> values) {
+      public Builder addSequenceVariable(String name, Collection<String> values) {
         ValueSequence.Builder sequenceBuilder = new ValueSequence.Builder();
         for (String value : values) {
           sequenceBuilder.addValue(value);
@@ -831,6 +831,14 @@ public class CcToolchainFeatures implements Serializable {
     }
     
     /**
+     * A group of extra {@code Variable} instances, packaged as logic for adding to a
+     * {@code Builder}
+     */
+    public interface VariablesExtension {
+      void addVariables(Builder builder);
+    }
+    
+    /**
      * Interface for a set of variables visible during an expansion of a variable.
      */
     interface View {
@@ -1271,14 +1279,14 @@ public class CcToolchainFeatures implements Serializable {
   /**
    * Given a list of {@code requestedFeatures}, returns all features that are enabled by the
    * toolchain configuration.
-   * 
+   *
    * <p>A requested feature will not be enabled if the toolchain does not support it (which may
    * depend on other requested features).
-   * 
+   *
    * <p>Additional features will be enabled if the toolchain supports them and they are implied by
    * requested features.
    */
-  FeatureConfiguration getFeatureConfiguration(Collection<String> requestedFeatures) {
+  public FeatureConfiguration getFeatureConfiguration(Collection<String> requestedFeatures) {
     return configurationCache.getUnchecked(requestedFeatures);
   }
       
@@ -1289,8 +1297,15 @@ public class CcToolchainFeatures implements Serializable {
   }
   
   /**
-   * Convenience method taking a variadic string argument list for testing.   
-   */
+   * Given a list of {@code requestedFeatures}, returns all features that are enabled by the
+   * toolchain configuration.
+   *
+   * <p>A requested feature will not be enabled if the toolchain does not support it (which may
+   * depend on other requested features).
+   *
+   * <p>Additional features will be enabled if the toolchain supports them and they are implied by
+   * requested features.
+   */ 
   public FeatureConfiguration getFeatureConfiguration(String... requestedFeatures) {
     return getFeatureConfiguration(Arrays.asList(requestedFeatures));
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
index 34e7b3a1ab..80cbb21442 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.rules.cpp.CcCompilationOutputs.Builder;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
+import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
@@ -79,6 +80,7 @@ public final class CppModel {
   private boolean createDynamicLibrary = true;
   private Artifact soImplArtifact;
   private FeatureConfiguration featureConfiguration;
+  private List<VariablesExtension> variablesExtensions = new ArrayList<>();
 
   public CppModel(RuleContext ruleContext, CppSemantics semantics) {
     this.ruleContext = Preconditions.checkNotNull(ruleContext);
@@ -188,6 +190,21 @@ public final class CppModel {
   }
 
   /**
+   * Adds the given variablesExensions for templating the crosstool.
+   *
+   * <p>In general, we prefer the build variables (especially those that derive strictly from
+   * the configuration) be learned by inspecting the CcToolchain, as passed to the rule in the
+   * CcToolchainProvider.  However, for build variables that must be injected into the rule
+   * implementation (ex. build variables learned from the BUILD file), should be added using the
+   * VariablesExtension abstraction.  This allows the injection to construct non-trivial build
+   * variables (lists, ect.).
+   */
+  public CppModel addVariablesExtension(Collection<VariablesExtension> variablesExtensions) {
+    this.variablesExtensions.addAll(variablesExtensions);
+    return this;
+  }
+  
+  /**
    * Sets the link type used for the link actions. Note that only static links are supported at this
    * time.
    */
@@ -393,6 +410,10 @@ public final class CppModel {
 
     buildVariables.addAllVariables(CppHelper.getToolchain(ruleContext).getBuildVariables());
     
+    for (VariablesExtension extension : variablesExtensions) {
+      extension.addVariables(buildVariables);
+    }
+    
     CcToolchainFeatures.Variables variables = buildVariables.build();
     builder.setVariables(variables);
   }