tmpdir,local-exec: implement --local_tmp_root

Add new flag called `--local_tmp_root`, which (if
specified) tells Bazel what temp directory should
locally executed actions use.

Fixes https://github.com/bazelbuild/bazel/issues/4621
Related to https://github.com/bazelbuild/bazel/issues/3215

RELNOTES[NEW]: The new "--local_tmp_root=<path>" flag allows specifying the temp directory for locally executed actions.

Change-Id: Ice69a5e63d0bf4d3b5c9ef4dbdd1ed1c5025f85e
PiperOrigin-RevId: 185509555

3 files changed, 73 insertions, 81 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
index 54f098907d..b59253392f 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
@@ -626,6 +626,8 @@ public class LocalSpawnRunnerTest {
     LocalEnvProvider localEnvProvider = mock(LocalEnvProvider.class);
 
     LocalExecutionOptions options = Options.getDefaults(LocalExecutionOptions.class);
+    options.localTmpRoot = "local-tmp-root";
+
     LocalSpawnRunner runner =
         new TestedLocalSpawnRunner(
             fs.getPath("/execroot"),
@@ -646,6 +648,7 @@ public class LocalSpawnRunnerTest {
         .rewriteLocalEnv(
             any(),
             eq(fs.getPath("/execroot")),
+            eq("local-tmp-root"),
             matches("^/execroot/tmp[0-9a-fA-F]+_[0-9a-fA-F]+/work$"),
             eq("product-name"));
   }
diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/PosixLocalEnvProviderTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/PosixLocalEnvProviderTest.java
index 314d0f0191..788f7b343c 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/local/PosixLocalEnvProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/local/PosixLocalEnvProviderTest.java
@@ -27,28 +27,40 @@ import org.junit.runners.JUnit4;
 public final class PosixLocalEnvProviderTest {
 
   private static Map<String, String> rewriteEnv(
-      PosixLocalEnvProvider p, ImmutableMap<String, String> env) {
-    return p.rewriteLocalEnv(env, null, null, null);
+      PosixLocalEnvProvider p,
+      ImmutableMap<String, String> env,
+      String localTmpRoot,
+      String fallbackDir) {
+    return p.rewriteLocalEnv(env, null, localTmpRoot, fallbackDir, null);
   }
 
-  /** Should use the client environment's TMPDIR envvar if specified. */
   @Test
-  public void testRewriteEnvWithClientTmpdir() throws Exception {
-    PosixLocalEnvProvider p =
-        new PosixLocalEnvProvider(ImmutableMap.of("TMPDIR", "client-env/tmp"));
-    assertThat(rewriteEnv(p, ImmutableMap.of("key1", "value1")))
-        .isEqualTo(ImmutableMap.of("key1", "value1", "TMPDIR", "client-env/tmp"));
-    assertThat(rewriteEnv(p, ImmutableMap.of("key1", "value1", "TMPDIR", "ignored")))
-        .isEqualTo(ImmutableMap.of("key1", "value1", "TMPDIR", "client-env/tmp"));
-  }
+  public void testRewriteEnv() throws Exception {
+    // localTmpRoot is specified, so ignore everything else.
+    assertThat(
+            rewriteEnv(
+                new PosixLocalEnvProvider(ImmutableMap.of("TMPDIR", "client/env/tmpdir")),
+                ImmutableMap.of("key1", "value1", "TMPDIR", "spawn/tmpdir"),
+                "local/tmp",
+                "fallback/dir"))
+        .isEqualTo(ImmutableMap.of("key1", "value1", "TMPDIR", "local/tmp"));
 
-  /** Should use the default temp dir when the client env doesn't define TMPDIR. */
-  @Test
-  public void testRewriteEnvWithDefaultTmpdir() throws Exception {
-    PosixLocalEnvProvider p = new PosixLocalEnvProvider(ImmutableMap.<String, String>of());
-    assertThat(rewriteEnv(p, ImmutableMap.of("key1", "value1")))
-        .isEqualTo(ImmutableMap.of("key1", "value1", "TMPDIR", "/tmp"));
-    assertThat(rewriteEnv(p, ImmutableMap.of("key1", "value1", "TMPDIR", "ignored")))
+    // localTmpRoot is empty, fall back to the client environment's TMPDIR.
+    assertThat(
+            rewriteEnv(
+                new PosixLocalEnvProvider(ImmutableMap.of("TMPDIR", "client/tmpdir")),
+                ImmutableMap.of("key1", "value1", "TMPDIR", "spawn/tmpdir"),
+                "",
+                "fallback/dir"))
+        .isEqualTo(ImmutableMap.of("key1", "value1", "TMPDIR", "client/tmpdir"));
+
+    // localTmpRoot and the client environment's TMPDIR are empty, fall back to /tmp.
+    assertThat(
+            rewriteEnv(
+                new PosixLocalEnvProvider(ImmutableMap.of("TMPDIR", "")),
+                ImmutableMap.of("key1", "value1", "TMPDIR", "spawn/tmpdir"),
+                "",
+                "fallback/dir"))
         .isEqualTo(ImmutableMap.of("key1", "value1", "TMPDIR", "/tmp"));
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/WindowsLocalEnvProviderTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/WindowsLocalEnvProviderTest.java
index 08881b107e..8ebbd6b145 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/local/WindowsLocalEnvProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/local/WindowsLocalEnvProviderTest.java
@@ -27,76 +27,53 @@ import org.junit.runners.JUnit4;
 public final class WindowsLocalEnvProviderTest {
 
   private static Map<String, String> rewriteEnv(
-      WindowsLocalEnvProvider p, ImmutableMap<String, String> env) {
-    return p.rewriteLocalEnv(env, null, null, null);
+      WindowsLocalEnvProvider p,
+      ImmutableMap<String, String> env,
+      String localTmpRoot,
+      String fallbackDir) {
+    return p.rewriteLocalEnv(env, null, localTmpRoot, fallbackDir, null);
   }
 
-  private static Map<String, String> rewriteEnv(
-      WindowsLocalEnvProvider p, ImmutableMap<String, String> env, String fallback) {
-    return p.rewriteLocalEnv(env, null, fallback, null);
-  }
-
-  /** Should use the client environment's TMP envvar if specified. */
-  @Test
-  public void testRewriteEnvWithClientTmp() throws Exception {
-    WindowsLocalEnvProvider p =
-        new WindowsLocalEnvProvider(
-            ImmutableMap.of("TMP", "client-env/tmp", "TEMP", "ignore/when/tmp/is/present"));
-
-    assertThat(rewriteEnv(p, ImmutableMap.of("key1", "value1", "TMP", "ignore", "TEMP", "ignore")))
-        .isEqualTo(
-            ImmutableMap.of("key1", "value1", "TMP", "client-env\\tmp", "TEMP", "client-env\\tmp"));
-
-    assertThat(rewriteEnv(p, ImmutableMap.of("key1", "value1", "TMP", "ignore")))
-        .isEqualTo(
-            ImmutableMap.of("key1", "value1", "TMP", "client-env\\tmp", "TEMP", "client-env\\tmp"));
-
-    assertThat(rewriteEnv(p, ImmutableMap.of("key1", "value1")))
-        .isEqualTo(
-            ImmutableMap.of("key1", "value1", "TMP", "client-env\\tmp", "TEMP", "client-env\\tmp"));
-  }
-
-  /** Should use the client environment's TEMP envvar if TMP is unspecified. */
-  @Test
-  public void testRewriteEnvWithoutClientTmpWithClientTemp() throws Exception {
-    WindowsLocalEnvProvider p =
-        new WindowsLocalEnvProvider(ImmutableMap.of("TEMP", "client-env/temp"));
-
-    assertThat(rewriteEnv(p, ImmutableMap.of("key1", "value1", "TMP", "ignore", "TEMP", "ignore")))
-        .isEqualTo(
-            ImmutableMap.of(
-                "key1", "value1", "TMP", "client-env\\temp", "TEMP", "client-env\\temp"));
-
-    assertThat(rewriteEnv(p, ImmutableMap.of("key1", "value1", "TMP", "ignore")))
-        .isEqualTo(
-            ImmutableMap.of(
-                "key1", "value1", "TMP", "client-env\\temp", "TEMP", "client-env\\temp"));
-
-    assertThat(rewriteEnv(p, ImmutableMap.of("key1", "value1")))
-        .isEqualTo(
-            ImmutableMap.of(
-                "key1", "value1", "TMP", "client-env\\temp", "TEMP", "client-env\\temp"));
-  }
-
-  /** Should use the fallback temp dir when the client env defines neither TMP nor TEMP. */
   @Test
-  public void testRewriteEnvWithFallbackTmp() throws Exception {
-    WindowsLocalEnvProvider p = new WindowsLocalEnvProvider(ImmutableMap.<String, String>of());
-
+  public void testRewriteEnv() throws Exception {
+    // localTmpRoot is specified, so ignore everything else.
     assertThat(
             rewriteEnv(
-                p,
-                ImmutableMap.of("key1", "value1", "TMP", "ignore", "TEMP", "ignore"),
-                "fallback/tmp"))
-        .isEqualTo(
-            ImmutableMap.of("key1", "value1", "TMP", "fallback\\tmp", "TEMP", "fallback\\tmp"));
-
-    assertThat(rewriteEnv(p, ImmutableMap.of("key1", "value1", "TMP", "ignore"), "fallback/tmp"))
+                new WindowsLocalEnvProvider(
+                    ImmutableMap.of("TMP", "client/env/tmp", "TEMP", "client/env/temp")),
+                ImmutableMap.of("key1", "value1", "TMP", "spawn/tmp", "TEMP", "spawn/temp"),
+                "local/tmp",
+                "fallback/dir"))
+        .isEqualTo(ImmutableMap.of("key1", "value1", "TMP", "local\\tmp", "TEMP", "local\\tmp"));
+
+    // localTmpRoot is empty, fall back to the client environment's TMP.
+    assertThat(
+            rewriteEnv(
+                new WindowsLocalEnvProvider(
+                    ImmutableMap.of("TMP", "client/tmp", "TEMP", "client/temp")),
+                ImmutableMap.of("key1", "value1", "TMP", "spawn/tmp", "TEMP", "spawn/temp"),
+                "",
+                "fallback/dir"))
+        .isEqualTo(ImmutableMap.of("key1", "value1", "TMP", "client\\tmp", "TEMP", "client\\tmp"));
+
+    // localTmpRoot and the client environment's TMP are empty, fall back to TEMP.
+    assertThat(
+            rewriteEnv(
+                new WindowsLocalEnvProvider(ImmutableMap.of("TMP", "", "TEMP", "client/temp")),
+                ImmutableMap.of("key1", "value1", "TMP", "spawn/tmp", "TEMP", "spawn/temp"),
+                "",
+                "fallback/dir"))
         .isEqualTo(
-            ImmutableMap.of("key1", "value1", "TMP", "fallback\\tmp", "TEMP", "fallback\\tmp"));
+            ImmutableMap.of("key1", "value1", "TMP", "client\\temp", "TEMP", "client\\temp"));
 
-    assertThat(rewriteEnv(p, ImmutableMap.of("key1", "value1"), "fallback/tmp"))
+    // localTmpRoot and the client environment's TMP and TEMP are empty, fall back to fallbackDir.
+    assertThat(
+            rewriteEnv(
+                new WindowsLocalEnvProvider(ImmutableMap.of("TMP", "", "TEMP", "")),
+                ImmutableMap.of("key1", "value1", "TMP", "spawn/tmp", "TEMP", "spawn/temp"),
+                "",
+                "fallback/dir"))
         .isEqualTo(
-            ImmutableMap.of("key1", "value1", "TMP", "fallback\\tmp", "TEMP", "fallback\\tmp"));
+            ImmutableMap.of("key1", "value1", "TMP", "fallback\\dir", "TEMP", "fallback\\dir"));
   }
 }