Automated [] rollback of commit 525fa71b0d6f096e9bfb180f688a4418c4974eb4.

*** Reason for rollback ***

Contributor finds some bugs and after fixing some bugs there are more bugs to fix now.

*** Original change description ***


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 on...

***

ROLLBACK_OF=119138157

--
MOS_MIGRATED_REVID=119828267

1 files changed, 9 insertions, 87 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 d5d2439b77..b2fe0e6720 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,6 +29,7 @@ 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;
@@ -104,9 +105,6 @@ 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();
@@ -117,15 +115,6 @@ 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());
@@ -148,9 +137,10 @@ 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", "");
 
-    Map<String, String> assertMounts = ImmutableMap.of("symlink.txt", "goal.txt");
+    List<String> assertMounts = new ArrayList<>();
+    assertMounts.add("symlink.txt");
+    assertMounts.add("goal.txt");
 
     assertThat(userFriendlyMounts(testFiles)).isEqualTo(userFriendlyAsserts(assertMounts));
   }
@@ -160,10 +150,9 @@ public class LinuxSandboxedStrategyTest extends LinuxSandboxedStrategyTestCase {
     Map<String, String> testFiles =
         ImmutableMap.of(
             "symlink.txt", "x/goal.txt",
-            "x/goal.txt", "",
-            "x/other.txt", "");
+            "x/goal.txt", "");
 
-    Map<String, String> assertMounts = ImmutableMap.of("symlink.txt", "x/goal.txt");
+    List<String> assertMounts = ImmutableList.of("symlink.txt", "x/goal.txt");
     assertThat(userFriendlyMounts(testFiles)).isEqualTo(userFriendlyAsserts(assertMounts));
   }
 
@@ -172,10 +161,9 @@ public class LinuxSandboxedStrategyTest extends LinuxSandboxedStrategyTestCase {
     Map<String, String> testFiles =
         ImmutableMap.of(
             "x/symlink.txt", "../goal.txt",
-            "goal.txt", "",
-            "x/other.txt", "");
+            "goal.txt", "");
 
-    Map<String, String> assertMounts = ImmutableMap.of("x/symlink.txt", "goal.txt");
+    List<String> assertMounts = ImmutableList.of("x/symlink.txt", "goal.txt");
 
     assertThat(userFriendlyMounts(testFiles)).isEqualTo(userFriendlyAsserts(assertMounts));
   }
@@ -190,73 +178,7 @@ public class LinuxSandboxedStrategyTest extends LinuxSandboxedStrategyTestCase {
             "a/b/y.txt", "z.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");
+    List<String> assertMounts = ImmutableList.of("a/b/x.txt", "a/b/y.txt", "a/b/z.txt");
 
     assertThat(userFriendlyMounts(testFiles, inputFile))
         .isEqualTo(userFriendlyAsserts(assertMounts));