Mount whole directories into the sandbox when possible

This halves the overhead with sandboxing enabled vs disabled for a test
that basically only mounts a bunch of files out of a directory, and
slows that same test with a single extra file added to the directory
(but not mounted) by only ~4%.

The test is <https://gist.github.com/bsilver8192/10527a862ce16bb7f79a>;
with 30000 inputs moved to a subdirectory and only 10 genrules.

This change means symlinks will be mounted directly as their target
rather than as a symlink, but this solves some weird behavior with
multi-level symlinks and will only break things which don't declare all
of their dependencies.

--
Change-Id: I1aa39dccb2e5fca2893bdab9065ee043d34019b2
Reviewed-on: https://bazel-review.googlesource.com/#/c/3220/
MOS_MIGRATED_REVID=119138157

1 files changed, 87 insertions, 9 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategyTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategyTest.java
index b2fe0e6720..d5d2439b77 100644
--- a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategyTest.java
@@ -29,7 +29,6 @@ import org.junit.runners.JUnit4;
 
 import java.io.IOException;
 import java.nio.charset.Charset;
-import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -105,6 +104,9 @@ public class LinuxSandboxedStrategyTest extends LinuxSandboxedStrategyTestCase {
   private ImmutableMap<String, String> userFriendlyAsserts(List<String> asserts) {
     return userFriendlyMap(asserts(asserts));
   }
+  private ImmutableMap<String, String> userFriendlyAsserts(Map<String, String> asserts) {
+    return userFriendlyMap(asserts(asserts));
+  }
 
   private ImmutableMap<Path, Path> asserts(List<String> asserts) {
     ImmutableMap.Builder<Path, Path> pathifiedAsserts = ImmutableMap.builder();
@@ -115,6 +117,15 @@ public class LinuxSandboxedStrategyTest extends LinuxSandboxedStrategyTestCase {
     return pathifiedAsserts.build();
   }
 
+  private ImmutableMap<Path, Path> asserts(Map<String, String> asserts) {
+    ImmutableMap.Builder<Path, Path> pathifiedAsserts = ImmutableMap.builder();
+    for (Map.Entry<String, String> file : asserts.entrySet()) {
+      pathifiedAsserts.put(
+          workspaceDir.getRelative(file.getKey()), workspaceDir.getRelative(file.getValue()));
+    }
+    return pathifiedAsserts.build();
+  }
+
   private void createTreeStructure(Map<String, String> linksAndFiles) throws Exception {
     for (Entry<String, String> entry : linksAndFiles.entrySet()) {
       Path filePath = workspaceDir.getRelative(entry.getKey());
@@ -137,10 +148,9 @@ public class LinuxSandboxedStrategyTest extends LinuxSandboxedStrategyTestCase {
     Map<String, String> testFiles = new LinkedHashMap<>();
     testFiles.put("symlink.txt", "goal.txt");
     testFiles.put("goal.txt", "");
+    testFiles.put("other.txt", "");
 
-    List<String> assertMounts = new ArrayList<>();
-    assertMounts.add("symlink.txt");
-    assertMounts.add("goal.txt");
+    Map<String, String> assertMounts = ImmutableMap.of("symlink.txt", "goal.txt");
 
     assertThat(userFriendlyMounts(testFiles)).isEqualTo(userFriendlyAsserts(assertMounts));
   }
@@ -150,9 +160,10 @@ public class LinuxSandboxedStrategyTest extends LinuxSandboxedStrategyTestCase {
     Map<String, String> testFiles =
         ImmutableMap.of(
             "symlink.txt", "x/goal.txt",
-            "x/goal.txt", "");
+            "x/goal.txt", "",
+            "x/other.txt", "");
 
-    List<String> assertMounts = ImmutableList.of("symlink.txt", "x/goal.txt");
+    Map<String, String> assertMounts = ImmutableMap.of("symlink.txt", "x/goal.txt");
     assertThat(userFriendlyMounts(testFiles)).isEqualTo(userFriendlyAsserts(assertMounts));
   }
 
@@ -161,9 +172,10 @@ public class LinuxSandboxedStrategyTest extends LinuxSandboxedStrategyTestCase {
     Map<String, String> testFiles =
         ImmutableMap.of(
             "x/symlink.txt", "../goal.txt",
-            "goal.txt", "");
+            "goal.txt", "",
+            "x/other.txt", "");
 
-    List<String> assertMounts = ImmutableList.of("x/symlink.txt", "goal.txt");
+    Map<String, String> assertMounts = ImmutableMap.of("x/symlink.txt", "goal.txt");
 
     assertThat(userFriendlyMounts(testFiles)).isEqualTo(userFriendlyAsserts(assertMounts));
   }
@@ -178,7 +190,73 @@ public class LinuxSandboxedStrategyTest extends LinuxSandboxedStrategyTestCase {
             "a/b/y.txt", "z.txt",
             "a/b/z.txt", "");
 
-    List<String> assertMounts = ImmutableList.of("a/b/x.txt", "a/b/y.txt", "a/b/z.txt");
+    List<String> assertMounts = ImmutableList.of("a/b");
+
+    assertThat(userFriendlyMounts(testFiles, inputFile))
+        .isEqualTo(userFriendlyAsserts(assertMounts));
+  }
+
+  @Test
+  public void testDetectsWholeDir() throws Exception {
+    ImmutableList<String> inputFile = ImmutableList.of("a/x.txt", "a/z.txt");
+
+    Map<String, String> testFiles =
+        ImmutableMap.of(
+            "a/x.txt", "",
+            "a/z.txt", "");
+
+    List<String> assertMounts = ImmutableList.of("a");
+
+    assertThat(userFriendlyMounts(testFiles, inputFile))
+        .isEqualTo(userFriendlyAsserts(assertMounts));
+  }
+
+  @Test
+  public void testExcludesOtherDir() throws Exception {
+    ImmutableList<String> inputFile = ImmutableList.of("a/x.txt", "a/y.txt");
+
+    Map<String, String> testFiles =
+        ImmutableMap.of(
+            "a/x.txt", "",
+            "a/y.txt", "",
+            "a/b/", "");
+
+    List<String> assertMounts = ImmutableList.of("a/x.txt", "a/y.txt");
+
+    assertThat(userFriendlyMounts(testFiles, inputFile))
+        .isEqualTo(userFriendlyAsserts(assertMounts));
+  }
+
+  @Test
+  public void testExcludesOtherFiles() throws Exception {
+    ImmutableList<String> inputFile = ImmutableList.of("a/x.txt", "a/z.txt");
+
+    Map<String, String> testFiles =
+        ImmutableMap.of(
+            "a/x.txt", "",
+            "a/y.txt", "z.txt",
+            "a/z.txt", "");
+
+    List<String> assertMounts = ImmutableList.of("a/x.txt", "a/z.txt");
+
+    assertThat(userFriendlyMounts(testFiles, inputFile))
+        .isEqualTo(userFriendlyAsserts(assertMounts));
+  }
+
+  @Test
+  public void testRecognizesOtherSymlinks() throws Exception {
+    ImmutableList<String> inputFile = ImmutableList.of("a/a/x.txt", "a/a/y.txt");
+
+    Map<String, String> testFiles =
+        ImmutableMap.of(
+            "a/a/x.txt", "../b/x.txt",
+            "a/a/y.txt", "",
+            "a/b/x.txt", "");
+
+    Map<String, String> assertMounts =
+        ImmutableMap.of(
+            "a/a/x.txt", "a/b/x.txt",
+            "a/a/y.txt", "a/a/y.txt");
 
     assertThat(userFriendlyMounts(testFiles, inputFile))
         .isEqualTo(userFriendlyAsserts(assertMounts));