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

8 files changed, 26 insertions, 43 deletions

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,