Update SpawnAction to take an ActionEnvironment

Some of the callers are not using the proper one from the configuration, and
are thus ignoring --action_env. Some are only using part of the configuration's
action environment. I tried to carefully not change the semantics in any case.

Semantic-changing changes come later.

PiperOrigin-RevId: 160891204

13 files changed, 78 insertions, 113 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index 26a68a5c7e..a7e1033a22 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -102,7 +102,7 @@ public abstract class AbstractAction implements Action, SkylarkValue {
   @GuardedBy("this")
   private Iterable<Artifact> inputs;
 
-  private final Iterable<String> clientEnvironmentVariables;
+  protected final ActionEnvironment env;
   private final RunfilesSupplier runfilesSupplier;
   private final ImmutableSet<Artifact> outputs;
 
@@ -142,22 +142,22 @@ public abstract class AbstractAction implements Action, SkylarkValue {
       Iterable<Artifact> inputs,
       RunfilesSupplier runfilesSupplier,
       Iterable<Artifact> outputs) {
-    this(owner, tools, inputs, ImmutableList.<String>of(), runfilesSupplier, outputs);
+    this(owner, tools, inputs, runfilesSupplier, outputs, ActionEnvironment.EMPTY);
   }
 
   protected AbstractAction(
       ActionOwner owner,
       Iterable<Artifact> tools,
       Iterable<Artifact> inputs,
-      Iterable<String> clientEnvironmentVariables,
       RunfilesSupplier runfilesSupplier,
-      Iterable<Artifact> outputs) {
+      Iterable<Artifact> outputs,
+      ActionEnvironment env) {
     Preconditions.checkNotNull(owner);
     // TODO(bazel-team): Use RuleContext.actionOwner here instead
     this.owner = owner;
     this.tools = CollectionUtils.makeImmutable(tools);
     this.inputs = CollectionUtils.makeImmutable(inputs);
-    this.clientEnvironmentVariables = clientEnvironmentVariables;
+    this.env = env;
     this.outputs = ImmutableSet.copyOf(outputs);
     this.runfilesSupplier = Preconditions.checkNotNull(runfilesSupplier,
         "runfilesSupplier may not be null");
@@ -251,7 +251,7 @@ public abstract class AbstractAction implements Action, SkylarkValue {
 
   @Override
   public Iterable<String> getClientEnvironmentVariables() {
-    return clientEnvironmentVariables;
+    return env.getInheritedEnv();
   }
 
   @Override
@@ -540,6 +540,7 @@ public abstract class AbstractAction implements Action, SkylarkValue {
    *     throws that exception.
    * @throws InterruptedException if interrupted
    */
+  @Override
   public Iterable<Artifact> getInputFilesForExtraAction(
       ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java b/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java
index a70fc1fc56..ba39e801ee 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java
@@ -37,6 +37,14 @@ import java.util.TreeSet;
  */
 public final class ActionEnvironment {
   /**
+   * An empty environment, mainly for testing. Production code should never use this, but instead
+   * get the proper environment from the current configuration.
+   */
+  // TODO(ulfjack): Migrate all production code to use the proper action environment, and then make
+  // this @VisibleForTesting or rename it to clarify.
+  public static final ActionEnvironment EMPTY = new ActionEnvironment(ImmutableMap.of());
+
+  /**
    * Splits the given map into a map of variables with a fixed value, and a set of variables that
    * should be inherited, the latter of which are identified by having a {@code null} value in the
    * given map. Returns these two parts as a new {@link ActionEnvironment} instance.
@@ -70,6 +78,10 @@ public final class ActionEnvironment {
     this.inheritedEnv = ImmutableSet.copyOf(inheritedEnv);
   }
 
+  public ActionEnvironment(Map<String, String> fixedEnv) {
+    this(fixedEnv, ImmutableSet.of());
+  }
+
   public ImmutableMap<String, String> getFixedEnv() {
     return fixedEnv;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CommandAction.java b/src/main/java/com/google/devtools/build/lib/actions/CommandAction.java
index 8babc0d8a3..38e9f2ce95 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/CommandAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/CommandAction.java
@@ -24,11 +24,11 @@ import java.util.List;
 public interface CommandAction extends Action, ExecutionInfoSpecifier {
 
   /** Returns a list of command line arguments that implements this action. */
-  public List<String> getArguments();
+  List<String> getArguments();
 
   /** 
    * Returns a map of command line variables to their values that constitute the environment
    * in which this action should be run.
    */
-  public ImmutableMap<String, String> getEnvironment();
+  ImmutableMap<String, String> getEnvironment();
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index 11e52d2ef4..0cf88d3074 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -20,11 +20,11 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.CharMatcher;
 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.common.collect.Lists;
 import com.google.devtools.build.lib.actions.AbstractAction;
 import com.google.devtools.build.lib.actions.Action;
+import com.google.devtools.build.lib.actions.ActionEnvironment;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
 import com.google.devtools.build.lib.actions.ActionInput;
@@ -65,7 +65,6 @@ import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import javax.annotation.CheckReturnValue;
 import javax.annotation.Nullable;
 
@@ -98,8 +97,6 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
   private final String mnemonic;
 
   private final ResourceSet resourceSet;
-  private final ImmutableMap<String, String> environment;
-  private final ImmutableSet<String> clientEnvironmentVariables;
   private final ImmutableMap<String, String> executionInfo;
 
   private final ExtraActionInfoSupplier<?> extraActionInfoSupplier;
@@ -115,9 +112,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
    *     modified.
    * @param outputs the set of all files written by this action; must not be subsequently modified.
    * @param resourceSet the resources consumed by executing this Action
-   * @param environment the map of environment variables.
-   * @param clientEnvironmentVariables the set of variables to be inherited from the client
-   *     environment.
+   * @param env the action environment
    * @param argv the command line to execute. This is merely a list of options to the executable,
    *     and is uninterpreted by the build tool for the purposes of dependency checking; typically
    *     it may include the names of input and output files, but this is not necessary.
@@ -134,8 +129,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
       ResourceSet resourceSet,
       CommandLine argv,
       boolean isShellCommand,
-      Map<String, String> environment,
-      Set<String> clientEnvironmentVariables,
+      ActionEnvironment env,
       String progressMessage,
       String mnemonic) {
     this(
@@ -146,8 +140,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
         resourceSet,
         argv,
         isShellCommand,
-        ImmutableMap.copyOf(environment),
-        ImmutableSet.copyOf(clientEnvironmentVariables),
+        env,
         ImmutableMap.<String, String>of(),
         progressMessage,
         EmptyRunfilesSupplier.INSTANCE,
@@ -161,17 +154,15 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
    *
    * <p>All collections provided must not be subsequently modified.
    *
-   * @param owner the owner of the Action.
+   * @param owner the owner of the Action
    * @param tools the set of files comprising the tool that does the work (e.g. compiler). This is a
-   *     subset of "inputs" and is only used by the WorkerSpawnStrategy.
+   *     subset of "inputs" and is only used by the WorkerSpawnStrategy
    * @param inputs the set of all files potentially read by this action; must not be subsequently
-   *     modified.
+   *     modified
    * @param outputs the set of all files written by this action; must not be subsequently modified.
    * @param resourceSet the resources consumed by executing this Action
-   * @param environment the map of environment variables.
-   * @param clientEnvironmentVariables the set of variables to be inherited from the client
-   *     environment.
-   * @param executionInfo out-of-band information for scheduling the spawn.
+   * @param env the action's environment
+   * @param executionInfo out-of-band information for scheduling the spawn
    * @param argv the argv array (including argv[0]) of arguments to pass. This is merely a list of
    *     options to the executable, and is uninterpreted by the build tool for the purposes of
    *     dependency checking; typically it may include the names of input and output files, but this
@@ -180,7 +171,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
    *     executable. This is used to give better error messages.
    * @param progressMessage the message printed during the progression of the build
    * @param runfilesSupplier {@link RunfilesSupplier}s describing the runfiles for the action
-   * @param mnemonic the mnemonic that is reported in the master log.
+   * @param mnemonic the mnemonic that is reported in the master log
    */
   public SpawnAction(
       ActionOwner owner,
@@ -190,19 +181,16 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
       ResourceSet resourceSet,
       CommandLine argv,
       boolean isShellCommand,
-      ImmutableMap<String, String> environment,
-      ImmutableSet<String> clientEnvironmentVariables,
+      ActionEnvironment env,
       ImmutableMap<String, String> executionInfo,
       String progressMessage,
       RunfilesSupplier runfilesSupplier,
       String mnemonic,
       boolean executeUnconditionally,
       ExtraActionInfoSupplier<?> extraActionInfoSupplier) {
-    super(owner, tools, inputs, runfilesSupplier, outputs);
+    super(owner, tools, inputs, runfilesSupplier, outputs, env);
     this.resourceSet = resourceSet;
     this.executionInfo = executionInfo;
-    this.environment = environment;
-    this.clientEnvironmentVariables = clientEnvironmentVariables;
     this.argv = argv;
     this.isShellCommand = isShellCommand;
     this.progressMessage = progressMessage;
@@ -417,12 +405,11 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
 
   @Override
   public ImmutableMap<String, String> getEnvironment() {
-    return environment;
-  }
-
-  @Override
-  public Iterable<String> getClientEnvironmentVariables() {
-    return clientEnvironmentVariables;
+    // TODO(ulfjack): AbstractAction should declare getEnvironment with a return value of type
+    // ActionEnvironment to avoid developers misunderstanding the purpose of this method. That
+    // requires first updating all subclasses and callers to actually handle environments correctly,
+    // so it's not a small change.
+    return env.getFixedEnv();
   }
 
   /**
@@ -511,7 +498,6 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
     private final List<RunfilesSupplier> toolRunfilesSuppliers = new ArrayList<>();
     private ResourceSet resourceSet = AbstractAction.DEFAULT_RESOURCE_SET;
     private ImmutableMap<String, String> environment = ImmutableMap.of();
-    private ImmutableSet<String> clientEnvironmentVariables = ImmutableSet.of();
     private ImmutableMap<String, String> executionInfo = ImmutableMap.of();
     private boolean isShellCommand = false;
     private boolean useDefaultShellEnvironment = false;
@@ -544,7 +530,6 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
       this.toolRunfilesSuppliers.addAll(other.toolRunfilesSuppliers);
       this.resourceSet = other.resourceSet;
       this.environment = other.environment;
-      this.clientEnvironmentVariables = other.clientEnvironmentVariables;
       this.executionInfo = other.executionInfo;
       this.isShellCommand = other.isShellCommand;
       this.useDefaultShellEnvironment = other.useDefaultShellEnvironment;
@@ -608,8 +593,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
       actions.add(
           buildSpawnAction(
               owner,
-              configuration.getLocalShellEnvironment(),
-              configuration.getVariableShellEnvironment(),
+              configuration.getActionEnvironment(),
               configuration.getShellExecutable(),
               paramsFile));
       if (paramFileWriteAction != null) {
@@ -630,18 +614,15 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
      * referenced in this function to prevent them from bleeding into the execution phase.
      *
      * @param owner the {@link ActionOwner} for the SpawnAction
-     * @param defaultShellEnvironment the default shell environment to use. May be null if not used.
+     * @param configEnv the config's action environment to use. May be null if not used.
      * @param defaultShellExecutable the default shell executable path. May be null if not used.
      * @param paramsFile the parameter file for the SpawnAction. May be null if not used.
-     * @param paramFileWriteAction the action generating the parameter file. May be null if not
-     *     used.
      * @return the SpawnAction and any actions required by it, with the first item always being the
      *     SpawnAction itself.
      */
     SpawnAction buildSpawnAction(
         ActionOwner owner,
-        @Nullable Map<String, String> defaultShellEnvironment,
-        @Nullable Set<String> variableShellEnvironment,
+        @Nullable ActionEnvironment configEnv,
         @Nullable PathFragment defaultShellExecutable,
         @Nullable Artifact paramsFile) {
       List<String> argv = buildExecutableArgs(defaultShellExecutable);
@@ -663,14 +644,11 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
               .addTransitive(tools)
               .build();
 
-      Map<String, String> env;
-      Set<String> clientEnv;
+      ActionEnvironment env;
       if (useDefaultShellEnvironment) {
-        env = Preconditions.checkNotNull(defaultShellEnvironment);
-        clientEnv = Preconditions.checkNotNull(variableShellEnvironment);
+        env = Preconditions.checkNotNull(configEnv);
       } else {
-        env = this.environment;
-        clientEnv = this.clientEnvironmentVariables;
+        env = new ActionEnvironment(this.environment);
       }
 
       if (disableSandboxing) {
@@ -688,8 +666,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
           resourceSet,
           actualCommandLine,
           isShellCommand,
-          ImmutableMap.copyOf(env),
-          ImmutableSet.copyOf(clientEnv),
+          env,
           ImmutableMap.copyOf(executionInfo),
           progressMessage,
           new CompositeRunfilesSupplier(
@@ -706,8 +683,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
         ResourceSet resourceSet,
         CommandLine actualCommandLine,
         boolean isShellCommand,
-        ImmutableMap<String, String> env,
-        ImmutableSet<String> clientEnvironmentVariables,
+        ActionEnvironment env,
         ImmutableMap<String, String> executionInfo,
         String progressMessage,
         RunfilesSupplier runfilesSupplier,
@@ -721,7 +697,6 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
           actualCommandLine,
           isShellCommand,
           env,
-          clientEnvironmentVariables,
           executionInfo,
           progressMessage,
           runfilesSupplier,
@@ -830,7 +805,8 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
     }
 
     /**
-     * Sets the map of environment variables.
+     * Sets the map of environment variables. Do not use! This makes the builder ignore the
+     * 'default shell environment', which is computed from the --action_env command line option.
      */
     public Builder setEnvironment(Map<String, String> environment) {
       this.environment = ImmutableMap.copyOf(environment);
@@ -838,13 +814,6 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
       return this;
     }
 
-    /** Sets the environment variables to be inherited from the client environment. */
-    public Builder setClientEnvironmentVariables(Set<String> clientEnvironmentVariables) {
-      this.clientEnvironmentVariables = ImmutableSet.copyOf(clientEnvironmentVariables);
-      this.useDefaultShellEnvironment = false;
-      return this;
-    }
-
     /**
      * Sets the map of execution info.
      */
@@ -859,7 +828,6 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
      */
     public Builder useDefaultShellEnvironment() {
       this.environment = null;
-      this.clientEnvironmentVariables = null;
       this.useDefaultShellEnvironment  = true;
       return this;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
index c25d729b78..f489cea11f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
@@ -128,8 +128,7 @@ public final class SpawnActionTemplate implements ActionTemplate<SpawnAction> {
     // explicitly via builder method #setExecutable and #setEnvironment.
     return actionBuilder.buildSpawnAction(
         getOwner(),
-        /*defaultShellEnvironment=*/ null,
-        /*variableShellEnvironment=*/ null,
+        /*configEnv=*/ null,
         /*defaultShellExecutable=*/ null,
         /*paramsFile=*/ null);
   }
@@ -206,7 +205,7 @@ public final class SpawnActionTemplate implements ActionTemplate<SpawnAction> {
   @Override
   public Iterable<String> getClientEnvironmentVariables() {
     return spawnActionBuilder
-        .buildSpawnAction(getOwner(), null, null, null, null)
+        .buildSpawnAction(getOwner(), null, null, null)
         .getClientEnvironmentVariables();
   }
 
@@ -307,6 +306,8 @@ public final class SpawnActionTemplate implements ActionTemplate<SpawnAction> {
     }
 
     /** Sets the map of environment variables for expanded actions. */
+    @Deprecated // TODO(ulfjack): Add env variables to the common environment, rather than replacing
+    // it wholesale, which ignores --action_env (unless the client code explicitly handles it).
     public Builder setEnvironment(Map<String, String> environment) {
       spawnActionBuilder.setEnvironment(environment);
       return this;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendAction.java
index d31a7167d1..945c7bc2bd 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendAction.java
@@ -16,8 +16,8 @@ package com.google.devtools.build.lib.rules.cpp;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.AbstractAction;
+import com.google.devtools.build.lib.actions.ActionEnvironment;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
 import com.google.devtools.build.lib.actions.ActionOwner;
@@ -67,8 +67,7 @@ public final class LTOBackendAction extends SpawnAction {
       ActionOwner owner,
       CommandLine argv,
       boolean isShellCommand,
-      Map<String, String> environment,
-      Set<String> clientEnvironmentVariables,
+      ActionEnvironment env,
       Map<String, String> executionInfo,
       String progressMessage,
       RunfilesSupplier runfilesSupplier,
@@ -81,8 +80,7 @@ public final class LTOBackendAction extends SpawnAction {
         AbstractAction.DEFAULT_RESOURCE_SET,
         argv,
         isShellCommand,
-        ImmutableMap.copyOf(environment),
-        ImmutableSet.copyOf(clientEnvironmentVariables),
+        env,
         ImmutableMap.copyOf(executionInfo),
         progressMessage,
         runfilesSupplier,
@@ -211,8 +209,7 @@ public final class LTOBackendAction extends SpawnAction {
         ResourceSet resourceSet,
         CommandLine actualCommandLine,
         boolean isShellCommand,
-        ImmutableMap<String, String> env,
-        ImmutableSet<String> clientEnvironmentVariables,
+        ActionEnvironment env,
         ImmutableMap<String, String> executionInfo,
         String progressMessage,
         RunfilesSupplier runfilesSupplier,
@@ -226,7 +223,6 @@ public final class LTOBackendAction extends SpawnAction {
           actualCommandLine,
           isShellCommand,
           env,
-          clientEnvironmentVariables,
           executionInfo,
           progressMessage,
           runfilesSupplier,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java
index a947232147..9a0c33886f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java
@@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.actions.AbstractAction;
 import com.google.devtools.build.lib.actions.Action;
+import com.google.devtools.build.lib.actions.ActionEnvironment;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -39,7 +40,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Map;
-import java.util.Set;
 import javax.annotation.Nullable;
 
 /**
@@ -72,8 +72,7 @@ public final class ExtraAction extends SpawnAction {
       Action shadowedAction,
       boolean createDummyOutput,
       CommandLine argv,
-      Map<String, String> environment,
-      Set<String> clientEnvironmentVariables,
+      ActionEnvironment env,
       Map<String, String> executionInfo,
       String progressMessage,
       String mnemonic) {
@@ -89,8 +88,7 @@ public final class ExtraAction extends SpawnAction {
         AbstractAction.DEFAULT_RESOURCE_SET,
         argv,
         false,
-        ImmutableMap.copyOf(environment),
-        ImmutableSet.copyOf(clientEnvironmentVariables),
+        env,
         ImmutableMap.copyOf(executionInfo),
         progressMessage,
         // TODO(michajlo): Do we need the runfiles manifest as an input / should this be composite?
diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java
index 65a81d91ee..3ccda08af2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java
@@ -37,7 +37,6 @@ import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 /**
  * The specification for a particular extra action type.
@@ -120,9 +119,6 @@ public final class ExtraActionSpec implements TransitiveInfoProvider {
     // See {@link #createExpandedCommand} for list of supported variables.
     String command = createExpandedCommand(owningRule, actionToShadow, extraActionInfoFile);
 
-    Map<String, String> env = owningRule.getConfiguration().getLocalShellEnvironment();
-    Set<String> clientEnvVars = owningRule.getConfiguration().getVariableShellEnvironment();
-
     CommandHelper commandHelper =
         new CommandHelper(
             owningRule,
@@ -145,8 +141,7 @@ public final class ExtraActionSpec implements TransitiveInfoProvider {
             actionToShadow,
             createDummyOutput,
             CommandLine.of(argv),
-            env,
-            clientEnvVars,
+            owningRule.getConfiguration().getActionEnvironment(),
             executionInfo,
             commandMessage,
             label.getName()));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java
index 7f4501d189..1d949f0956 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java
@@ -15,7 +15,7 @@
 package com.google.devtools.build.lib.rules.genrule;
 
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.ActionEnvironment;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -43,8 +43,7 @@ public class GenRuleAction extends SpawnAction {
       Iterable<Artifact> inputs,
       Iterable<Artifact> outputs,
       List<String> argv,
-      ImmutableMap<String, String> environment,
-      ImmutableSet<String> clientEnvironmentVariables,
+      ActionEnvironment env,
       ImmutableMap<String, String> executionInfo,
       RunfilesSupplier runfilesSupplier,
       String progressMessage) {
@@ -56,8 +55,7 @@ public class GenRuleAction extends SpawnAction {
         GENRULE_RESOURCES,
         CommandLine.of(argv),
         false,
-        environment,
-        clientEnvironmentVariables,
+        env,
         executionInfo,
         progressMessage,
         runfilesSupplier,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
index f35627b6eb..8b90b48217 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.rules.genrule;
 
 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.common.collect.Maps;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -168,10 +167,6 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory {
       message = "Executing genrule";
     }
 
-    ImmutableMap<String, String> env = ruleContext.getConfiguration().getLocalShellEnvironment();
-    ImmutableSet<String> clientEnvVars =
-        ruleContext.getConfiguration().getVariableShellEnvironment();
-
     Map<String, String> executionInfo = Maps.newLinkedHashMap();
     executionInfo.putAll(TargetUtils.getExecutionInfo(ruleContext.getRule()));
 
@@ -219,8 +214,7 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory {
             inputs.build(),
             filesToBuild,
             argv,
-            env,
-            clientEnvVars,
+            ruleContext.getConfiguration().getActionEnvironment(),
             ImmutableMap.copyOf(executionInfo),
             new CompositeRunfilesSupplier(commandHelper.getToolsRunfilesSuppliers()),
             message + ' ' + ruleContext.getLabel()));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
index cd90dd74fb..024ea9feaa 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
@@ -29,6 +29,7 @@ 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.Action;
+import com.google.devtools.build.lib.actions.ActionEnvironment;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactOwner;
@@ -193,8 +194,8 @@ public final class JavaCompileAction extends SpawnAction {
         LOCAL_RESOURCES,
         commandLine,
         false,
-        ImmutableMap.copyOf(UTF8_ENVIRONMENT),
-        ImmutableSet.copyOf(ImmutableSet.<String>of()),
+        // TODO(#3320): This is missing the configuration's action environment!
+        new ActionEnvironment(UTF8_ENVIRONMENT),
         ImmutableMap.copyOf(executionInfo),
         progressMessage,
         EmptyRunfilesSupplier.INSTANCE,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
index 0917a472a8..a82ca291a3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
@@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Action;
+import com.google.devtools.build.lib.actions.ActionEnvironment;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionOwner;
@@ -111,8 +112,8 @@ public class JavaHeaderCompileAction extends SpawnAction {
         LOCAL_RESOURCES,
         transitiveCommandLine,
         false,
-        JavaCompileAction.UTF8_ENVIRONMENT,
-        /*executionInfo=*/ ImmutableSet.<String>of(),
+        // TODO(#3320): This is missing the config's action environment.
+        new ActionEnvironment(JavaCompileAction.UTF8_ENVIRONMENT),
         progressMessage,
         "Turbine");
     this.directInputs = checkNotNull(directInputs);
@@ -434,8 +435,8 @@ public class JavaHeaderCompileAction extends SpawnAction {
               LOCAL_RESOURCES,
               transitiveCommandLine,
               false,
-              JavaCompileAction.UTF8_ENVIRONMENT,
-              /*executionInfo=*/ ImmutableSet.<String>of(),
+              // TODO(b/63280599): This is missing the config's action environment.
+              new ActionEnvironment(JavaCompileAction.UTF8_ENVIRONMENT),
               getProgressMessageWithAnnotationProcessors(),
               "JavacTurbine")
         };
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
index aa2f6ffa9d..f04dc7542d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
@@ -22,6 +22,7 @@ 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.ActionEnvironment;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
 import com.google.devtools.build.lib.actions.ActionInput;
@@ -113,7 +114,7 @@ public class ObjcCompileAction extends SpawnAction {
       ResourceSet resourceSet,
       CommandLine argv,
       boolean isShellCommand,
-      ImmutableMap<String, String> environment,
+      ActionEnvironment env,
       ImmutableMap<String, String> executionInfo,
       String progressMessage,
       RunfilesSupplier runfilesSupplier,
@@ -134,8 +135,7 @@ public class ObjcCompileAction extends SpawnAction {
         resourceSet,
         argv,
         isShellCommand,
-        environment,
-        ImmutableSet.<String>of(),
+        env,
         executionInfo,
         progressMessage,
         runfilesSupplier,
@@ -440,8 +440,7 @@ public class ObjcCompileAction extends SpawnAction {
         ResourceSet resourceSet,
         CommandLine actualCommandLine,
         boolean isShellCommand,
-        ImmutableMap<String, String> env,
-        ImmutableSet<String> clientEnvironmentVariables,
+        ActionEnvironment env,
         ImmutableMap<String, String> executionInfo,
         String progressMessage,
         RunfilesSupplier runfilesSupplier,
@@ -454,7 +453,8 @@ public class ObjcCompileAction extends SpawnAction {
           resourceSet,
           actualCommandLine,
           isShellCommand,
-          env,
+          // TODO(#3320): This is missing the inherited action env from --action_env.
+          new ActionEnvironment(env.getFixedEnv()),
           executionInfo,
           progressMessage,
           runfilesSupplier,