sandbox: add env[TMPDIR] instead of `tmpDir`

Bazel now adds env[TMPDIR] to the set of
sandbox-writable paths, instead of adding the
caller-defined `tmpDir` as it used to.

Since every caller of getWritableDirs passes the
LocalEnvProvider-processed environment to
getWritableDirs, and because all such callers use
either PosixLocalEnvProvider or
XCodeLocalEnvProvider, we can be sure that the
environment has an entry for TMPDIR.

Change-Id: Ia89544a009e56d9cc922ab56823d16d20465545e
PiperOrigin-RevId: 181595606

4 files changed, 20 insertions, 12 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
index 8d7a3389a6..6985392380 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.sandbox;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
 import com.google.devtools.build.lib.actions.ExecException;
@@ -227,8 +228,8 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
    *
    * @throws IOException because we might resolve symlinks, which throws {@link IOException}.
    */
-  protected ImmutableSet<Path> getWritableDirs(
-      Path sandboxExecRoot, Map<String, String> env, Path tmpDir) throws IOException {
+  protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env)
+      throws IOException {
     // We have to make the TEST_TMPDIR directory writable if it is specified.
     ImmutableSet.Builder<Path> writablePaths = ImmutableSet.builder();
     writablePaths.add(sandboxExecRoot);
@@ -237,14 +238,20 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
       addWritablePath(
           sandboxExecRoot,
           writablePaths,
-          sandboxExecRoot.getRelative(testTmpdir),
+          testTmpdir,
           "Cannot resolve symlinks in TEST_TMPDIR because it doesn't exist: \"%s\"");
     }
     addWritablePath(
         sandboxExecRoot,
         writablePaths,
-        tmpDir,
-        "Cannot resolve symlinks in tmpDir because it doesn't exist: \"%s\"");
+        // As of 2018-01-09:
+        // - every caller of `getWritableDirs` passes a LocalEnvProvider-processed environment as
+        //   `env`, and in every case that's either PosixLocalEnvProvider or XCodeLocalEnvProvider,
+        //   therefore `env` surely has an entry for TMPDIR
+        // - Bazel-on-Windows does not yet support sandboxing, so we don't need to add env[TMP] and
+        //   env[TEMP] as writable paths.
+        Preconditions.checkNotNull(env.get("TMPDIR")),
+        "Cannot resolve symlinks in TMPDIR because it doesn't exist: \"%s\"");
 
     FileSystem fileSystem = sandboxExecRoot.getFileSystem();
     for (String writablePath : sandboxOptions.sandboxWritablePath) {
@@ -259,9 +266,10 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
   private void addWritablePath(
       Path sandboxExecRoot,
       ImmutableSet.Builder<Path> writablePaths,
-      Path path,
+      String pathString,
       String pathDoesNotExistErrorTemplate)
       throws IOException {
+    Path path = sandboxExecRoot.getRelative(pathString);
     if (path.startsWith(sandboxExecRoot)) {
       // We add this path even though it is below sandboxExecRoot (and thus already writable as a
       // subpath) to take advantage of the side-effect that SymlinkedExecRoot also creates this
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
index 6199b5d27f..0859c3ad59 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
@@ -226,7 +226,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
         localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir, productName);
 
     final HashSet<Path> writableDirs = new HashSet<>(alwaysWritableDirs);
-    ImmutableSet<Path> extraWritableDirs = getWritableDirs(sandboxExecRoot, environment, tmpDir);
+    ImmutableSet<Path> extraWritableDirs = getWritableDirs(sandboxExecRoot, environment);
     writableDirs.addAll(extraWritableDirs);
 
     ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn);
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
index 72ef7e0ee0..2ff65d778e 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
@@ -184,7 +184,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
     Map<String, String> environment =
         localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir, productName);
 
-    Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, environment, tmpDir);
+    Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, environment);
     ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn);
     Duration timeout = policy.getTimeout();
 
@@ -234,10 +234,10 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
   }
 
   @Override
-  protected ImmutableSet<Path> getWritableDirs(
-      Path sandboxExecRoot, Map<String, String> env, Path tmpDir) throws IOException {
+  protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env)
+      throws IOException {
     ImmutableSet.Builder<Path> writableDirs = ImmutableSet.builder();
-    writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env, tmpDir));
+    writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env));
 
     FileSystem fs = sandboxExecRoot.getFileSystem();
     writableDirs.add(fs.getPath("/dev/shm").resolveSymbolicLinks());
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
index 974a641715..c5f95193c3 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
@@ -129,7 +129,7 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne
             environment,
             SandboxHelpers.getInputFiles(spawn, policy, execRoot),
             SandboxHelpers.getOutputFiles(spawn),
-            getWritableDirs(sandboxExecRoot, environment, tmpDir));
+            getWritableDirs(sandboxExecRoot, environment));
 
     return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout, statisticsPath);
   }