Automated rollback of commit f7f1a8f756f33f86b1bdc5023d5052117b62523e.

*** Reason for rollback ***

Updated to avoid https://github.com/bazelbuild/bazel/issues/3501

This is a rollback of a rollback, with additional
modifications to BazelConfiguration.java to fix
https://github.com/bazelbuild/bazel/issues/3501,
the issue that was the reason we rolled back the
original change.

The additional updates serve to propagate the
client's TMP and TEMP envvars to the action, which
is a short-term solution to allow actions have a
TMP/TEMP envvar on Windows. They need at least one
of those to create temp directories.

The long-term solution is to set a value for TMP
or TEMP in the executor just before executing the
actions, so the TMP/TEMP would not be part of the
action key.

All of this only affects Bazel on Windows.

*** Original change description ***

Automated rollback of commit 0abf5fa2d64c76def5a8fa0f960b73ce0566af4d.

*** Reason for rollback ***

Breaks Bazel CI (https://github.com/bazelbuild/bazel/issues/3501)

*** Original change description ***

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

RELNOTES: none
PiperOrigin-RevId: 164696600

11 files changed, 27 insertions, 3 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfiguration.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfiguration.java
index f04104851b..3d8cbdafe4 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfiguration.java
@@ -133,6 +133,18 @@ public class BazelConfiguration extends Fragment {
       // TODO(ulfjack): Avoid using System.getenv; it's the wrong environment!
       builder.put("PATH", pathOrDefault(os, System.getenv("PATH"), getShellExecutable()));
 
+      // TODO(laszlocsomor): Remove setting TMP/TEMP here, and set a meaningful value just before
+      // executing the action.
+      // Setting TMP=null, TEMP=null has the effect of copying the client's TMP/TEMP to the action's
+      // environment. This is a short-term workaround to get temp-requiring actions working on
+      // Windows. Its detrimental effect is that the client's TMP/TEMP becomes part of the actions's
+      // key. Yet, we need this for now to build Android programs, because the Android BusyBox is
+      // written in Java and tries to create temp directories using
+      // java.nio.file.Files.createTempDirectory, which needs TMP or TEMP (or USERPROFILE) on
+      // Windows, otherwise they return c:\windows which is non-writable.
+      builder.put("TMP", null);
+      builder.put("TEMP", null);
+
       String ldLibraryPath = System.getenv("LD_LIBRARY_PATH");
       if (ldLibraryPath != null) {
         builder.put("LD_LIBRARY_PATH", ldLibraryPath);
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 ee098453e6..c4c194bcb7 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
@@ -141,6 +141,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 abb7735e7c..3e7e853d57 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
@@ -201,6 +201,7 @@ public class AndroidResourceMergingActionBuilder {
     // Create the spawn action.
     ruleContext.registerAction(
         spawnActionBuilder
+            .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 62ea11d86e..67d083074f 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
@@ -160,6 +160,7 @@ public class AndroidResourceParsingActionBuilder {
     // Create the spawn action.
     ruleContext.registerAction(
         spawnActionBuilder
+            .useDefaultShellEnvironment()
             .addTransitiveInputs(inputs.build())
             .addOutputs(ImmutableList.of(output))
             .setCommandLine(builder.build())
@@ -195,6 +196,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 563570c384..4d4143c2ca 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
@@ -305,6 +305,7 @@ public class AndroidResourcesProcessorBuilder {
     ruleContext.registerAction(
         this.spawnActionBuilder
             .useDefaultShellEnvironment()
+            .useDefaultShellEnvironment()
             .addTool(sdk.getAapt2())
             .addTransitiveInputs(inputs.build())
             .addOutputs(ImmutableList.<Artifact>copyOf(outs))
@@ -364,6 +365,7 @@ public class AndroidResourcesProcessorBuilder {
     ruleContext.registerAction(
         this.spawnActionBuilder
             .useDefaultShellEnvironment()
+            .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 7e6aaad3e7..52c801437b 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
@@ -146,6 +146,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 8bc64cbf01..149368391a 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
@@ -133,10 +133,10 @@ public class RClassGeneratorActionBuilder {
 
     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 354b3c710e..0a22bb1da1 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
@@ -115,6 +115,7 @@ public class RobolectricResourceSymbolsActionBuilder {
 
     ruleContext.registerAction(
         spawnActionBuilder
+            .useDefaultShellEnvironment()
             .addInputs(inputs)
             .addOutput(classJarOut)
             .setCommandLine(builder.build())