Android BusyBox: actions use the default shell env

SpawnActions that run the Android BusyBox now use
the default shell environment.

This has the following benefits:
- Bazel propagates the PATH, TMPDIR envvars to the
  action
- Bazel propagates the --action_env envvars to the
  action

This allows the Bazel client to pass
--action_env=TMP or --action_env=TEMP (whichever
of the envvars is defined) to the server, so the
BusyBox actions will have TMP/TEMP set (to the
same value as the clientenv), so they can create
temp directories using
java.nio.file.Files.createTempDirectory.

This method seems to be calling the GetTempPath
WinAPI function, which needs the TMP or TEMP
envvar, otherwise it falls back to returning
c:\windows which is non-writable.

There's one drawback of using the default shell
environment, although @ulfjack is working on it:
- PATH is now also part of the action's cache key.
  However in a single-machine environment (no
  remote execution) and assuming PATH isn't likely
  to change between builds, this probably doesn't
  poision the action cache in practice.

This change is a short-term solution. Propagating
the client env's TMP/TEMP means we make that part
of the action's cache key.

The ideal long-term solution will be to not
propagate this envvar, and instead let the
execution strategy set it to some
client-env-independent value.

See https://github.com/bazelbuild/bazel/issues/3264

Change-Id: I756a4203b5d86c881bc36cc089e35cde0d419914
PiperOrigin-RevId: 164114502

10 files changed, 15 insertions, 3 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AarGeneratorBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AarGeneratorBuilder.java
index 7733909840..fc832848dd 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AarGeneratorBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AarGeneratorBuilder.java
@@ -127,6 +127,7 @@ public class AarGeneratorBuilder {
 
     ruleContext.registerAction(
         this.builder
+            .useDefaultShellEnvironment()
             .addInputs(ImmutableList.<Artifact>copyOf(ins))
             .addOutputs(ImmutableList.<Artifact>copyOf(outs))
             .setCommandLine(CommandLine.of(args))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
index d5ba575068..d10c6e6c63 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
@@ -186,6 +186,7 @@ public class AndroidResourceMergingActionBuilder {
     ruleContext.registerAction(
         spawnActionBuilder
             .useParameterFile(ParameterFileType.UNQUOTED)
+            .useDefaultShellEnvironment()
             .addTransitiveInputs(inputs.build())
             .addOutputs(ImmutableList.copyOf(outs))
             .setCommandLine(builder.build())
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java
index 2ff46cd9d7..c114715397 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java
@@ -145,6 +145,7 @@ public class AndroidResourceParsingActionBuilder {
     ruleContext.registerAction(
         new SpawnAction.Builder()
             .useParameterFile(ParameterFileType.UNQUOTED)
+            .useDefaultShellEnvironment()
             .addTransitiveInputs(inputs.build())
             .addOutputs(ImmutableList.of(output))
             .setCommandLine(builder.build())
@@ -180,6 +181,7 @@ public class AndroidResourceParsingActionBuilder {
       ruleContext.registerAction(
           new SpawnAction.Builder()
               .useParameterFile(ParameterFileType.UNQUOTED)
+              .useDefaultShellEnvironment()
               .addTransitiveInputs(inputs.build())
               .addOutputs(ImmutableList.copyOf(outs))
               .setCommandLine(flatFileBuilder.build())
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceValidatorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceValidatorActionBuilder.java
index 0b00e35201..832fc66ced 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceValidatorActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceValidatorActionBuilder.java
@@ -186,6 +186,7 @@ public class AndroidResourceValidatorActionBuilder {
     ruleContext.registerAction(
         new SpawnAction.Builder()
             .useParameterFile(ParameterFileType.UNQUOTED)
+            .useDefaultShellEnvironment()
             .addTool(sdk.getAapt2())
             .addInputs(inputs.build())
             .addOutputs(outs.build())
@@ -256,6 +257,7 @@ public class AndroidResourceValidatorActionBuilder {
     ruleContext.registerAction(
         spawnActionBuilder
             .useParameterFile(ParameterFileType.UNQUOTED)
+            .useDefaultShellEnvironment()
             .addTool(sdk.getAapt())
             .addInputs(inputs.build())
             .addOutputs(ImmutableList.copyOf(outs))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
index fa12915c6a..63d16edd0f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
@@ -290,6 +290,7 @@ public class AndroidResourcesProcessorBuilder {
     ruleContext.registerAction(
         this.spawnActionBuilder
             .useParameterFile(ParameterFileType.UNQUOTED)
+            .useDefaultShellEnvironment()
             .addTool(sdk.getAapt2())
             .addTransitiveInputs(inputs.build())
             .addOutputs(ImmutableList.<Artifact>copyOf(outs))
@@ -335,6 +336,7 @@ public class AndroidResourcesProcessorBuilder {
     ruleContext.registerAction(
         this.spawnActionBuilder
             .useParameterFile(ParameterFileType.UNQUOTED)
+            .useDefaultShellEnvironment()
             .addTool(sdk.getAapt())
             .addTransitiveInputs(inputs.build())
             .addOutputs(ImmutableList.copyOf(outs))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java
index f0862629d4..bd866ee778 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java
@@ -87,9 +87,10 @@ public class LibraryRGeneratorActionBuilder {
     SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder();
     ruleContext.registerAction(
         spawnActionBuilder
+            .useParameterFile(ParameterFileType.UNQUOTED)
+            .useDefaultShellEnvironment()
             .addTransitiveInputs(inputs.build())
             .addOutputs(ImmutableList.<Artifact>of(rJavaClassJar))
-            .useParameterFile(ParameterFileType.UNQUOTED)
             .setCommandLine(builder.build())
             .setExecutable(executable)
             .setProgressMessage("Generating Library R Classes: %s", ruleContext.getLabel())
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java
index d9846b3c53..24fc285761 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java
@@ -132,6 +132,7 @@ public class ManifestMergerActionBuilder {
 
     ruleContext.registerAction(
         this.spawnActionBuilder
+            .useDefaultShellEnvironment()
             .addTransitiveInputs(inputs.build())
             .addOutputs(outputs.build())
             .setCommandLine(builder.build())
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
index 49ef64770c..d32d70d147 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
@@ -117,10 +117,10 @@ public class RClassGeneratorActionBuilder {
     SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder();
     ruleContext.registerAction(
         spawnActionBuilder
-            .useParameterFile(ParameterFileType.UNQUOTED)
+            .useParameterFile(ParameterFileType.SHELL_QUOTED)
+            .useDefaultShellEnvironment()
             .addTransitiveInputs(inputs.build())
             .addOutputs(ImmutableList.<Artifact>copyOf(outs))
-            .useParameterFile(ParameterFileType.SHELL_QUOTED)
             .setCommandLine(builder.build())
             .setExecutable(
                 ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java
index 4fc305f8d1..33436cd110 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java
@@ -225,6 +225,7 @@ public class ResourceShrinkerActionBuilder {
 
     ruleContext.registerAction(
         spawnActionBuilder
+            .useDefaultShellEnvironment()
             .addTool(sdk.getAapt())
             .addInputs(inputs.build())
             .addOutputs(outputs.build())
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java
index 203d61e000..b7e0da7295 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java
@@ -100,6 +100,7 @@ public class RobolectricResourceSymbolsActionBuilder {
     ruleContext.registerAction(
         spawnActionBuilder
             .useParameterFile(ParameterFileType.UNQUOTED)
+            .useDefaultShellEnvironment()
             .addInputs(inputs)
             .addOutput(classJarOut)
             .setCommandLine(builder.build())