Extend crosstool configuration to allow features to specify (expandable) environment variables to pass to actions

--
MOS_MIGRATED_REVID=111608329

5 files changed, 243 insertions, 43 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 598fd6e5a1..15edc30dd5 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
@@ -74,13 +74,13 @@ public class CcToolchainFeatures implements Serializable {
   }
   
   /**
-   * A piece of a single flag.
+   * A piece of a single string value.
    * 
-   * <p>A single flag can contain a combination of text and variables (for example
-   * "-f %{var1}/%{var2}"). We split the flag into chunks, where each chunk represents either a
+   * <p>A single value can contain a combination of text and variables (for example
+   * "-f %{var1}/%{var2}"). We split the string into chunks, where each chunk represents either a
    * text snippet, or a variable that is to be replaced.
    */
-  interface FlagChunk {
+  interface StringChunk {
     
     /**
      * Expands this chunk.
@@ -92,13 +92,13 @@ public class CcToolchainFeatures implements Serializable {
   }
   
   /**
-   * A plain text chunk of a flag.
+   * A plain text chunk of a string (containing no variables).
    */
   @Immutable
-  private static class StringChunk implements FlagChunk, Serializable {
+  private static class StringLiteralChunk implements StringChunk, Serializable {
     private final String text;
     
-    private StringChunk(String text) {
+    private StringLiteralChunk(String text) {
       this.text = text;
     }
     
@@ -109,10 +109,10 @@ public class CcToolchainFeatures implements Serializable {
   }
   
   /**
-   * A chunk of a flag into which a variable should be expanded.
+   * A chunk of a string value into which a variable should be expanded.
    */
   @Immutable
-  private static class VariableChunk implements FlagChunk, Serializable {
+  private static class VariableChunk implements StringChunk, Serializable {
     private final String variableName;
     
     private VariableChunk(String variableName) {
@@ -120,7 +120,7 @@ public class CcToolchainFeatures implements Serializable {
     }
     
     @Override
-    public void expand(Map<String, String> variables, StringBuilder flag) {
+    public void expand(Map<String, String> variables, StringBuilder stringBuilder) {
       // We check all variables in FlagGroup.expandCommandLine.
       // If we arrive here with the variable not being available, the variable was provided, but
       // the nesting level of the NestedSequence was deeper than the nesting level of the flag
@@ -136,31 +136,29 @@ public class CcToolchainFeatures implements Serializable {
         throw new ExpansionException(
             "Internal blaze error: build variable '" + variableName + "'was set to 'null'.");
       }
-      flag.append(variables.get(variableName));
+      stringBuilder.append(variables.get(variableName));
     }
   }
   
   /**
-   * Parser for toolchain flags.
+   * Parser for toolchain string values.
    * 
-   * <p>A flag contains a snippet of text supporting variable expansion. For example, a flag value
-   * "-f %{var1}/%{var2}" will expand the values of the variables "var1" and "var2" in the
-   * corresponding places in the string.
+   * <p>A string value contains a snippet of text supporting variable expansion. For example, a
+   * string value "-f %{var1}/%{var2}" will expand the values of the variables "var1" and "var2"
+   * in the corresponding places in the string.
    * 
-   * <p>The {@code FlagParser} takes a flag string and parses it into a list of {@code FlagChunk}
-   * objects, where each chunk represents either a snippet of text or a variable to be expanded. In
-   * the above example, the resulting chunks would be ["-f ", var1, "/", var2].
+   * <p>The {@code StringValueParser} takes a string and parses it into a list of
+   * {@link StringChunk} objects, where each chunk represents either a snippet of text or a
+   * variable to be expanded. In the above example, the resulting chunks would be
+   * ["-f ", var1, "/", var2].
    * 
-   * <p>In addition to the list of chunks, the {@code FlagParser} also provides the set of variables
-   * necessary for the expansion of this flag via {@code getUsedVariables}.
+   * <p>In addition to the list of chunks, the {@link StringValueParser} also provides the set of
+   * variables necessary for the expansion of this flag via {@link #getUsedVariables}.
    * 
-   * <p>To get a literal percent character, "%%" can be used in the flag text.
+   * <p>To get a literal percent character, "%%" can be used in the string.
    */
-  private static class FlagParser {
-    
-    /**
-     * The given flag value.
-     */
+  private static class StringValueParser {
+
     private final String value;
     
     /**
@@ -168,30 +166,30 @@ public class CcToolchainFeatures implements Serializable {
      */
     private int current = 0;
     
-    private final ImmutableList.Builder<FlagChunk> chunks = ImmutableList.builder();
+    private final ImmutableList.Builder<StringChunk> chunks = ImmutableList.builder();
     private final ImmutableSet.Builder<String> usedVariables = ImmutableSet.builder();
     
-    private FlagParser(String value) throws InvalidConfigurationException {
+    private StringValueParser(String value) throws InvalidConfigurationException {
       this.value = value;
       parse();
     }
     
     /**
-     * @return the parsed chunks for this flag.
+     * @return the parsed chunks for this string.
      */
-    private ImmutableList<FlagChunk> getChunks() {
+    private ImmutableList<StringChunk> getChunks() {
       return chunks.build();
     }
     
     /**
-     * @return all variable names needed to expand this flag.
+     * @return all variable names needed to expand this string.
      */
     private ImmutableSet<String> getUsedVariables() {
       return usedVariables.build();
     }
     
     /**
-     * Parses the flag.
+     * Parses the string.
      * 
      * @throws InvalidConfigurationException if there is a parsing error.
      */
@@ -223,7 +221,7 @@ public class CcToolchainFeatures implements Serializable {
       // We only parse string chunks starting with '%' if they also start with '%%'.
       // In that case, we want to have a single '%' in the string, so we start at the second
       // character.
-      // Note that for flags like "abc%%def" this will lead to two string chunks, the first
+      // Note that for strings like "abc%%def" this will lead to two string chunks, the first
       // referencing the subtring "abc", and a second referencing the substring "%def".
       if (value.charAt(current) == '%') {
         current = current + 1;
@@ -234,7 +232,7 @@ public class CcToolchainFeatures implements Serializable {
         current = value.length();
       }
       final String text = value.substring(start, current);
-      chunks.add(new StringChunk(text));
+      chunks.add(new StringLiteralChunk(text));
     }
     
     /**
@@ -260,7 +258,7 @@ public class CcToolchainFeatures implements Serializable {
     
     /**
      * @throws InvalidConfigurationException with the given error text, adding information about
-     * the current position in the flag.
+     * the current position in the string.
      */
     private void abort(String error) throws InvalidConfigurationException {
       throw new InvalidConfigurationException("Invalid toolchain configuration: " + error
@@ -297,9 +295,9 @@ public class CcToolchainFeatures implements Serializable {
    */
   @Immutable
   private static class Flag implements Serializable, Expandable {
-    private final ImmutableList<FlagChunk> chunks;
+    private final ImmutableList<StringChunk> chunks;
     
-    private Flag(ImmutableList<FlagChunk> chunks) {
+    private Flag(ImmutableList<StringChunk> chunks) {
       this.chunks = chunks;
     }
     
@@ -309,7 +307,7 @@ public class CcToolchainFeatures implements Serializable {
     @Override
     public void expand(Variables.View view, List<String> commandLine) {
       StringBuilder flag = new StringBuilder();
-      for (FlagChunk chunk : chunks) {
+      for (StringChunk chunk : chunks) {
         chunk.expand(view.getVariables(), flag);
       }
       commandLine.add(flag.toString());
@@ -323,6 +321,36 @@ public class CcToolchainFeatures implements Serializable {
   }
   
   /**
+   * A single environment key/value pair to be expanded under a set of variables.
+   */
+  @Immutable
+  private static class EnvEntry implements Serializable {
+    private final String key;
+    private final ImmutableList<StringChunk> valueChunks;
+    private final ImmutableSet<String> usedVariables;
+    
+    private EnvEntry(CToolchain.EnvEntry envEntry) throws InvalidConfigurationException {
+      this.key = envEntry.getKey();
+      StringValueParser parser = new StringValueParser(envEntry.getValue());
+      this.valueChunks = parser.getChunks();
+      this.usedVariables = parser.getUsedVariables();
+    }
+
+    /**
+     * Adds the key/value pair this object represents to the given map of environment variables.
+     * The value of the entry is expanded with the given {@code variables}.
+     */
+    public void addEnvEntry(Variables variables, ImmutableMap.Builder<String, String> envBuilder) {
+      Variables.View view = variables.getView(usedVariables);
+      StringBuilder value = new StringBuilder();
+      for (StringChunk chunk : valueChunks) {
+        chunk.expand(view.getVariables(), value);
+      }
+      envBuilder.put(key, value.toString());
+    }
+  }
+
+  /**
    * A group of flags.
    */
   @Immutable
@@ -342,7 +370,7 @@ public class CcToolchainFeatures implements Serializable {
                 + "and another flag_group.");
       }
       for (String flag : flags) {
-        FlagParser parser = new FlagParser(flag);
+        StringValueParser parser = new StringValueParser(flag);
         expandables.add(new Flag(parser.getChunks()));
         usedVariables.addAll(parser.getUsedVariables());
       }
@@ -425,20 +453,58 @@ public class CcToolchainFeatures implements Serializable {
   }
   
   /**
+   * Groups a set of environment variables to apply for certain actions.
+   */
+  @Immutable
+  private static class EnvSet implements Serializable {
+    private final ImmutableSet<String> actions;
+    private final ImmutableList<EnvEntry> envEntries;
+    
+    private EnvSet(CToolchain.EnvSet envSet) throws InvalidConfigurationException {
+      this.actions = ImmutableSet.copyOf(envSet.getActionList());
+      ImmutableList.Builder<EnvEntry> builder = ImmutableList.builder();
+      for (CToolchain.EnvEntry envEntry : envSet.getEnvEntryList()) {
+        builder.add(new EnvEntry(envEntry));
+      }
+      this.envEntries = builder.build();
+    }
+
+    /**
+     * Adds the environment key/value pairs that apply to the given {@code action} to
+     * {@code envBuilder}.
+     */
+    private void expandEnvironment(String action, Variables variables,
+        ImmutableMap.Builder<String, String> envBuilder) {
+      if (!actions.contains(action)) {
+        return;
+      }
+      for (EnvEntry envEntry : envEntries) {
+        envEntry.addEnvEntry(variables, envBuilder);
+      }
+    }
+  }
+
+  /**
    * Contains flags for a specific feature.
    */
   @Immutable
   private static class Feature implements Serializable {
     private final String name;
     private final ImmutableList<FlagSet> flagSets;
+    private final ImmutableList<EnvSet> envSets;
     
     private Feature(CToolchain.Feature feature) throws InvalidConfigurationException {
       this.name = feature.getName();
-      ImmutableList.Builder<FlagSet> builder = ImmutableList.builder();
+      ImmutableList.Builder<FlagSet> flagSetBuilder = ImmutableList.builder();
       for (CToolchain.FlagSet flagSet : feature.getFlagSetList()) {
-        builder.add(new FlagSet(flagSet));
+        flagSetBuilder.add(new FlagSet(flagSet));
+      }
+      this.flagSets = flagSetBuilder.build();
+      ImmutableList.Builder<EnvSet> envSetBuilder = ImmutableList.builder();
+      for (CToolchain.EnvSet flagSet : feature.getEnvSetList()) {
+        envSetBuilder.add(new EnvSet(flagSet));
       }
-      this.flagSets = builder.build();
+      this.envSets = envSetBuilder.build();
     }
 
     /**
@@ -448,6 +514,13 @@ public class CcToolchainFeatures implements Serializable {
       return name;
     }
 
+    private void expandEnvironment(String action, Variables variables,
+        ImmutableMap.Builder<String, String> envBuilder) {
+      for (EnvSet envSet : envSets) {
+        envSet.expandEnvironment(action, variables, envBuilder);
+      }
+    }
+
     /**
      * Adds the flags that apply to the given {@code action} to {@code commandLine}.
      */
@@ -793,6 +866,17 @@ public class CcToolchainFeatures implements Serializable {
       }
       return commandLine;
     }
+
+    /**
+     * @return the environment variables (key/value pairs) for the given {@code action}.
+     */
+    Map<String, String> getEnvironmentVariables(String action, Variables variables) {
+      ImmutableMap.Builder<String, String> envBuilder = ImmutableMap.builder();
+      for (Feature feature : enabledFeatures) {
+        feature.expandEnvironment(action, variables, envBuilder);
+      }
+      return envBuilder.build();
+    }
   }
   
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index f23a921c30..6faa64a728 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -610,6 +610,8 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
       environment.putAll(appleConfiguration.appleTargetPlatformEnv(
           Platform.forTargetCpu(cppConfiguration.getTargetCpu())));
     }
+    
+    environment.putAll(cppCompileCommandLine.getEnvironment());
 
     // TODO(bazel-team): Check (crosstool) host system name instead of using OS.getCurrent.
     if (OS.getCurrent() == OS.WINDOWS) {
@@ -1259,6 +1261,13 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
       this.fdoBuildStamp = fdoBuildStamp;
     }
 
+    /**
+     * Returns the environment variables that should be set for C++ compile actions.
+     */
+    protected Map<String, String> getEnvironment() {
+      return featureConfiguration.getEnvironmentVariables(getActionName(), variables);
+    }
+
     protected List<String> getArgv(PathFragment outputFile) {
       List<String> commandLine = new ArrayList<>();
 
diff --git a/src/main/protobuf/crosstool_config.proto b/src/main/protobuf/crosstool_config.proto
index a420403462..d34b29e136 100644
--- a/src/main/protobuf/crosstool_config.proto
+++ b/src/main/protobuf/crosstool_config.proto
@@ -85,6 +85,14 @@ message CToolchain {
     repeated FlagGroup flag_group = 2;
   }
 
+  // A key/value pair to be added as an environment variable. The value of
+  // this pair is expanded in the same way as is described in FlagGroup.
+  // The key remains an unexpanded string literal.
+  message EnvEntry {
+    required string key = 1;
+    required string value = 2;
+  }
+
   // A set of features; used to support logical 'and' when specifying feature
   // requirements in FlagSet and Feature.
   message FeatureSet {
@@ -119,7 +127,27 @@ message CToolchain {
     repeated string expand_if_all_available = 4;
   }
 
+  // A set of environment variables that are expanded in the command line for
+  // specific actions.
+  message EnvSet {
+    // The actions this env set applies to; each env set must specify at
+    // least one action.
+    repeated string action = 1;
+
+    // The environment variables applied via this env set.
+    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.
+    //
+    // If 'with_feature' is omitted, the env set will be applied
+    // unconditionally for every action specified.
+    repeated FeatureSet with_feature = 3;
+  }
+
   // Contains all flag specifications for one feature.
+  // Next ID: 7
   message Feature {
     // The feature's name. Feature names are generally defined by Bazel; it is
     // possible to introduce a feature without a change to Bazel by adding a
@@ -131,6 +159,10 @@ message CToolchain {
     // actions in the modes that they are specified for.
     repeated FlagSet flag_set = 2;
 
+    // If the given feature is enabled, the env sets will be applied for the
+    // actions in the modes that they are specified for.
+    repeated EnvSet env_set = 6;
+
     // A list of feature sets defining when this feature is supported by the
     // toolchain. The feature is supported if any of the feature sets fully
     // apply, that is, when all features of a feature set are enabled.
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
index 5313c403e5..0f61571d82 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
@@ -132,6 +132,45 @@ public abstract class MockCcSupport {
           + "  }"
           + "}";
 
+  /**
+   * A feature configuration snippet useful for testing environment variables.
+   */
+  public static final String ENV_VAR_FEATURE_CONFIGURATION =
+      ""
+          + "feature {"
+          + "  name: 'env_feature'"
+          + "  implies: 'static_env_feature'"
+          + "  implies: 'module_maps'"
+          + "}"
+          + "feature {"
+          + "  name: 'static_env_feature'"
+          + "  env_set {"
+          + "    action: 'c-compile'"
+          + "    action: 'c++-compile'"
+          + "    action: 'c++-header-parsing'"
+          + "    action: 'c++-header-preprocessing'"
+          + "    action: 'c++-module-compile'"
+          + "    env_entry {"
+          + "      key: 'cat'"
+          + "      value: 'meow'"
+          + "    }"
+          + "  }"
+          + "}"
+          + "feature {"
+          + "  name: 'module_maps'"
+          + "  env_set {"
+          + "    action: 'c-compile'"
+          + "    action: 'c++-compile'"
+          + "    action: 'c++-header-parsing'"
+          + "    action: 'c++-header-preprocessing'"
+          + "    action: 'c++-module-compile'"
+          + "    env_entry {"
+          + "      key: 'module'"
+          + "      value: 'module_name:%{module_name}'"
+          + "    }"
+          + "  }"
+          + "}";
+
   public static final String THIN_LTO_CONFIGURATION =
       ""
           + "feature { "
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 f06f34bf82..7c3a435bc4 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
@@ -39,6 +39,7 @@ import org.junit.runners.JUnit4;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 /**
@@ -139,7 +140,42 @@ public class CcToolchainFeaturesTest {
         CppCompileAction.CPP_COMPILE, createVariables());
     assertThat(commandLine).containsExactly("-a-c++-compile", "-b-c++-compile").inOrder();
   }
-  
+
+  @Test
+  public void testEnvVars() throws Exception {
+    FeatureConfiguration configuration = buildFeatures(
+        "feature {",
+        "  name: 'a'",
+        "  env_set {",
+        "     action: 'c++-compile'",
+        "     env_entry { key: 'foo', value: 'bar' }",
+        "     env_entry { key: 'cat', value: 'meow' }",
+        "  }",
+        "  flag_set {",
+        "     action: 'c++-compile'",
+        "     flag_group { flag: '-a-c++-compile' }",
+        "  }",
+        "}",
+        "feature {",
+        "  name: 'b'",
+        "  env_set {",
+        "     action: 'c++-compile'",
+        "     env_entry { key: 'dog', value: 'woof' }",
+        "  }",
+        "}",
+        "feature {",
+        "  name: 'c'",
+        "  env_set {",
+        "     action: 'c++-compile'",
+        "     env_entry { key: 'doNotInclude', value: 'doNotIncludePlease' }",
+        "  }",
+        "}").getFeatureConfiguration("a", "b");
+    Map<String, String> env = configuration.getEnvironmentVariables(
+        CppCompileAction.CPP_COMPILE, createVariables());
+    assertThat(env).containsExactly("foo", "bar", "cat", "meow", "dog", "woof").inOrder();
+    assertThat(env).doesNotContainEntry("doNotInclude", "doNotIncludePlease");
+  }
+
   private String getExpansionOfFlag(String value) throws Exception {
     return getExpansionOfFlag(value, createVariables());
   }