Roll forward of https://github.com/bazelbuild/bazel/commit/656a0bab1e025ff3c27d595284a4bf1c5a8d8028 with test (unknown commit) and fix.

Big round of sandbox fixes / performance improvements.

- The number of stat() syscalls in the SymlinkedSandboxedSpawn was way too high. Do less, feel better.

- When using --experimental_sandbox_base, ensure that symlinks in the path are resolved. Before this, you had to check whether on your system /dev/shm is a symlink to /run/shm and then use that instead. Now it no longer matters, as symlinks are resolved.

- Remove an unnecessary directory creation from each sandboxed invocation. Turns out that the "tmpdir" that we created was no longer used after some changes to Bazel's TMPDIR handling.

- Use simpler sandbox paths, by using the unique ID for each Spawn provided by SpawnExecutionPolicy instead of a randomly generated temp folder name. This also saves a round-trip from our VFS to NIO and back. Clean up the sandbox base before each build to ensure that the unique IDs are actually unique. ;)

- Use Java 8's Process#isAlive to check whether a process is alive instead of trying to get the exitcode and catching an exception.

Closes #4913.

PiperOrigin-RevId: 193031017

5 files changed, 93 insertions, 37 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 7299b9e549..330c8d7792 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -1516,6 +1516,7 @@ java_test(
         ":foundations_testutil",
         ":guava_junit_truth",
         ":test_runner",
+        ":testutil",
         "//src/main/java/com/google/devtools/build/lib:os_util",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxTestCase.java b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxTestCase.java
index 39f2227af5..8489c7d914 100644
--- a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxTestCase.java
@@ -15,9 +15,8 @@ package com.google.devtools.build.lib.sandbox;
 
 import com.google.devtools.build.lib.testutil.TestUtils;
 import com.google.devtools.build.lib.vfs.FileSystem;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
-import com.google.devtools.build.lib.vfs.util.FileSystems;
+import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import org.junit.Before;
 
 /** Common parts of all sandbox tests. */
@@ -27,8 +26,8 @@ public class SandboxTestCase {
 
   @Before
   public final void createTestRoot() throws Exception {
-    fileSystem = FileSystems.getNativeFileSystem();
+    fileSystem = new InMemoryFileSystem();
     testRoot = fileSystem.getPath(TestUtils.tmpDir());
-    FileSystemUtils.deleteTreesBelow(testRoot);
+    testRoot.createDirectoryAndParents();
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawnTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawnTest.java
index 5e24deea4a..89fd5eecff 100644
--- a/src/test/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawnTest.java
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawnTest.java
@@ -58,7 +58,7 @@ public class SymlinkedSandboxedSpawnTest extends SandboxTestCase {
             sandboxDir,
             execRoot,
             ImmutableList.of("/bin/true"),
-            ImmutableMap.<String, String>of(),
+            ImmutableMap.of(),
             ImmutableMap.of(PathFragment.create("such/input.txt"), helloTxt),
             ImmutableSet.of(PathFragment.create("very/output.txt")),
             ImmutableSet.of(execRoot.getRelative("wow/writable")));
@@ -72,35 +72,6 @@ public class SymlinkedSandboxedSpawnTest extends SandboxTestCase {
   }
 
   @Test
-  public void cleanFileSystem() throws Exception {
-    Path helloTxt = workspaceDir.getRelative("hello.txt");
-    FileSystemUtils.createEmptyFile(helloTxt);
-
-    SymlinkedSandboxedSpawn symlinkedExecRoot = new SymlinkedSandboxedSpawn(
-        sandboxDir,
-        execRoot,
-        ImmutableList.of("/bin/true"),
-        ImmutableMap.<String, String>of(),
-        ImmutableMap.of(PathFragment.create("such/input.txt"), helloTxt),
-        ImmutableSet.of(PathFragment.create("very/output.txt")),
-        ImmutableSet.of(execRoot.getRelative("wow/writable")));
-    symlinkedExecRoot.createFileSystem();
-
-    // Pretend to do some work inside the execRoot.
-    execRoot.getRelative("tempdir").createDirectory();
-    FileSystemUtils.createEmptyFile(execRoot.getRelative("very/output.txt"));
-    FileSystemUtils.createEmptyFile(execRoot.getRelative("wow/writable/temp.txt"));
-
-    // Reuse the same execRoot.
-    symlinkedExecRoot.createFileSystem();
-
-    assertThat(execRoot.getRelative("such/input.txt").exists()).isTrue();
-    assertThat(execRoot.getRelative("tempdir").exists()).isFalse();
-    assertThat(execRoot.getRelative("very/output.txt").exists()).isFalse();
-    assertThat(execRoot.getRelative("wow/writable/temp.txt").exists()).isFalse();
-  }
-
-  @Test
   public void copyOutputs() throws Exception {
     // These tests are very simple because we just rely on SandboxedSpawnTest.testMoveOutputs to
     // properly verify all corner cases.
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerExecRootTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerExecRootTest.java
new file mode 100644
index 0000000000..b7758e66a5
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerExecRootTest.java
@@ -0,0 +1,88 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.worker;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.testutil.TestUtils;
+import com.google.devtools.build.lib.vfs.FileSystem;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import java.io.IOException;
+import java.nio.charset.Charset;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link WorkerExecRoot}. */
+@RunWith(JUnit4.class)
+public class WorkerExecRootTest {
+  private FileSystem fileSystem;
+  private Path testRoot;
+  private Path workspaceDir;
+  private Path sandboxDir;
+  private Path execRoot;
+
+  @Before
+  public final void setupTestDirs() throws IOException {
+    fileSystem = new InMemoryFileSystem();
+    testRoot = fileSystem.getPath(TestUtils.tmpDir());
+    testRoot.createDirectoryAndParents();
+
+    workspaceDir = testRoot.getRelative("workspace");
+    workspaceDir.createDirectory();
+    sandboxDir = testRoot.getRelative("sandbox");
+    sandboxDir.createDirectory();
+    execRoot = sandboxDir.getRelative("execroot");
+    execRoot.createDirectory();
+  }
+
+  @Test
+  public void cleanFileSystem() throws Exception {
+    Path workerSh = workspaceDir.getRelative("worker.sh");
+    FileSystemUtils.writeContentAsLatin1(workerSh, "#!/bin/bash");
+
+    WorkerExecRoot workerExecRoot =
+        new WorkerExecRoot(
+            execRoot,
+            ImmutableMap.of(PathFragment.create("worker.sh"), workerSh),
+            ImmutableSet.of(PathFragment.create("very/output.txt")),
+            ImmutableSet.of(PathFragment.create("worker.sh")));
+    workerExecRoot.createFileSystem();
+
+    // Pretend to do some work inside the execRoot.
+    execRoot.getRelative("tempdir").createDirectory();
+    FileSystemUtils.createEmptyFile(execRoot.getRelative("very/output.txt"));
+    FileSystemUtils.createEmptyFile(execRoot.getRelative("temp.txt"));
+    // Modify the worker.sh so that we're able to detect whether it gets rewritten or not.
+    FileSystemUtils.writeContentAsLatin1(workerSh, "#!/bin/sh");
+
+    // Reuse the same execRoot.
+    workerExecRoot.createFileSystem();
+
+    assertThat(execRoot.getRelative("worker.sh").exists()).isTrue();
+    assertThat(
+            FileSystemUtils.readContent(
+                execRoot.getRelative("worker.sh"), Charset.defaultCharset()))
+        .isEqualTo("#!/bin/sh");
+    assertThat(execRoot.getRelative("tempdir").exists()).isFalse();
+    assertThat(execRoot.getRelative("very/output.txt").exists()).isFalse();
+    assertThat(execRoot.getRelative("temp.txt").exists()).isFalse();
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java
index 27c0d4af1f..0552cb8c5a 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java
@@ -17,7 +17,6 @@ import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.hash.HashCode;
 import com.google.devtools.build.lib.vfs.FileSystem;
@@ -49,8 +48,6 @@ public class WorkerFactoryTest {
             "dummy",
             HashCode.fromInt(0),
             ImmutableSortedMap.of(),
-            ImmutableMap.of(),
-            ImmutableSet.of(),
             true);
     Path sandboxedWorkerPath = workerFactory.getSandboxedWorkerPath(workerKey, 1);