Add option --action_env and make BuildConfiguration declare environment dependencies

This option will allow to specify which environment variables are to
be provided to the actions. Environment variables for the options will
become opt-in, i.e., only environment variables explicitly specified
will be provided to the actions. However, the full range of rc-files
will be able to nominate options to be added; to avoid dependency on
the invocation environment, absolute values can be provided as well.

As the configuration now no longer completely determines the action
environment extend it to also declare which environment variables are
to be taken from the client environment. This will be implemented in
follow-up patches.

The transition plan is that the newly added option takes precedence
over the environment variables added by the fragments. This is conservative,
as the new option is not yet used anywhere. Then the effect of the
fragments will be provided by rc-files, and finally, the setupShellEnvironment
Method will be removed from the fragments all together.

Also add some simple tests for static (i.e., independent of the client
environment) setting of action environment variables.

This is the first step towards implementing the design on [Specifying
environment variables for actions](http://bazel.io/designs/2016/06/21/environment.html).

--
Change-Id: I0ad36913b7d357787b4d69e341926806b3fc61bf
Reviewed-on: https://bazel-review.googlesource.com/#/c/4241
MOS_MIGRATED_REVID=131044391

3 files changed, 126 insertions, 8 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index fe2f80764f..d516d17dfa 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -59,6 +59,7 @@ import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
 import com.google.devtools.build.lib.util.CPU;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.util.RegexFilter;
 import com.google.devtools.build.lib.vfs.Path;
@@ -594,6 +595,22 @@ public final class BuildConfiguration {
     )
     public List<Map.Entry<String, String>> testEnvironment;
 
+    @Option(
+      name = "action_env",
+      converter = Converters.OptionalAssignmentConverter.class,
+      allowMultiple = true,
+      defaultValue = "",
+      category = "semantics",
+      help =
+          "Specifies the set of environment variables available available to actions. "
+              + "Variables can be either specified by name, in which case the value will be "
+              + "taken from the invocation environment, or by the name=value pair which sets "
+              + "the value independent of the invocation environment. This option can be used "
+              + "multiple times; for options given for the same variable, the latest wins, options "
+              + "for different variables accumulate."
+    )
+    public List<Map.Entry<String, String>> actionEnvironment;
+
     @Option(name = "collect_code_coverage",
         defaultValue = "false",
         category = "testing",
@@ -1046,6 +1063,7 @@ public final class BuildConfiguration {
   private final ImmutableMap<String, String> globalMakeEnv;
 
   private final ImmutableMap<String, String> localShellEnvironment;
+  private final ImmutableSet<String> envVariables;
   private final BuildOptions buildOptions;
   private final Options options;
 
@@ -1164,12 +1182,33 @@ public final class BuildConfiguration {
     }
   }
 
-  private ImmutableMap<String, String> setupShellEnvironment() {
+  /**
+   * Compute the shell environment, which, at configuration level, is a pair consisting of the
+   * statically set environment variables with their values and the set of environment variables to
+   * be inherited from the client environment.
+   */
+  private Pair<ImmutableMap<String, String>, ImmutableSet<String>> setupShellEnvironment() {
     ImmutableMap.Builder<String, String> builder = new ImmutableMap.Builder<>();
     for (Fragment fragment : fragments.values()) {
       fragment.setupShellEnvironment(builder);
     }
-    return builder.build();
+    // Shell environment variables specified via options take precedence over the
+    // ones inherited from the fragments. In the long run, these fragments will
+    // be replaced by appropriate default rc files anyway.
+    Map<String, String> shellEnv = new HashMap(builder.build());
+    for (Map.Entry<String, String> entry : options.actionEnvironment) {
+      shellEnv.put(entry.getKey(), entry.getValue());
+    }
+    Map<String, String> fixedShellEnv = new HashMap(shellEnv);
+    Set<String> variableShellEnv = new HashSet();
+    for (Map.Entry<String, String> entry : shellEnv.entrySet()) {
+      if (entry.getValue() == null) {
+        String key = entry.getKey();
+        fixedShellEnv.remove(key);
+        variableShellEnv.add(key);
+      }
+    }
+    return Pair.of(ImmutableMap.copyOf(fixedShellEnv), ImmutableSet.copyOf(variableShellEnv));
   }
 
   /**
@@ -1241,7 +1280,10 @@ public final class BuildConfiguration {
         ? outputRoots
         : new OutputRoots(directories, outputDirName);
 
-    this.localShellEnvironment = setupShellEnvironment();
+    Pair<ImmutableMap<String, String>, ImmutableSet<String>> shellEnvironment =
+        setupShellEnvironment();
+    this.localShellEnvironment = shellEnvironment.getFirst();
+    this.envVariables = shellEnvironment.getSecond();
 
     this.transitiveOptionsMap = computeOptionsMap(buildOptions, fragments.values());
 
@@ -2004,16 +2046,20 @@ public final class BuildConfiguration {
   }
 
   @SkylarkCallable(
-      name = "default_shell_env",
-      structField = true,
-      doc = "A dictionary representing the local shell environment. It maps variables "
-          + "to their values (strings).  The local shell environment contains settings that are "
-          + "machine specific, therefore its use should be avoided in rules meant to be hermetic."
+    name = "default_shell_env",
+    structField = true,
+    doc =
+        "A dictionary representing the static local shell environment. It maps variables "
+            + "to their values (strings)."
   )
   public ImmutableMap<String, String> getLocalShellEnvironment() {
     return localShellEnvironment;
   }
 
+  public ImmutableSet<String> getVariableShellEnvironment() {
+    return envVariables;
+  }
+
   /**
    * Returns the path to sh.
    */
diff --git a/src/test/shell/integration/BUILD b/src/test/shell/integration/BUILD
index 35958d4ce7..613ed750fc 100644
--- a/src/test/shell/integration/BUILD
+++ b/src/test/shell/integration/BUILD
@@ -52,6 +52,13 @@ sh_test(
     data = [":test-deps"],
 )
 
+sh_test(
+    name = "action_env_test",
+    size = "medium",
+    srcs = ["action_env_test.sh"],
+    data = [":test-deps"],
+)
+
 test_suite(
     name = "all_tests",
     visibility = ["//visibility:public"],
diff --git a/src/test/shell/integration/action_env_test.sh b/src/test/shell/integration/action_env_test.sh
new file mode 100755
index 0000000000..2074c83d27
--- /dev/null
+++ b/src/test/shell/integration/action_env_test.sh
@@ -0,0 +1,65 @@
+#!/bin/bash
+#
+# Copyright 2016 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# An end-to-end test that Bazel's provides the correct environment variables
+# to actions.
+
+# Load test environment
+source $(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/testenv.sh \
+  || { echo "testenv.sh not found!" >&2; exit 1; }
+
+create_and_cd_client
+put_bazel_on_path
+write_default_bazelrc
+
+
+#### SETUP #############################################################
+
+set -e
+
+function set_up() {
+  mkdir -p pkg
+  cat > pkg/BUILD <<EOF
+genrule(
+  name = "showenv",
+  outs = ["env.txt"],
+  cmd = "env | sort > \"\$@\""
+)
+EOF
+}
+
+#### TESTS #############################################################
+
+function test_simple() {
+  export FOO=baz
+  bazel build --action_env=FOO=bar pkg:showenv \
+      || fail "bazel build showenv failed"
+  cat `bazel info bazel-genfiles`/pkg/env.txt > $TEST_log
+  expect_log "FOO=bar"
+}
+
+function test_simple_latest_wins() {
+  export FOO=environmentfoo
+  export BAR=environmentbar
+  bazel build --action_env=FOO=foo \
+      --action_env=BAR=willbeoverridden --action_env=BAR=bar pkg:showenv \
+      || fail "bazel build showenv failed"
+  cat `bazel info bazel-genfiles`/pkg/env.txt > $TEST_log
+  expect_log "FOO=foo"
+  expect_log "BAR=bar"
+}
+
+run_suite "Tests for bazel's handling of environment variables in actions"