Move deleteTree and deleteTreesBelow into FileSystem and Path.
The current implementation of these functions is very inefficient and
degrades overall performance significantly, especially when sandboxing is
enabled. However, that's almost the best we can do with a generic
algorithm.
To make room for optimizations that rely on specific file system features,
move these functions into the FileSystem class. I will supply a custom
implementation for UnixFileSystem later.
Note that this is intended to be a pure code move. I haven't applied any
improvements to the code nor tests yet (with the exception of cleaning up
docstrings).
Addresses https://github.com/bazelbuild/bazel/issues/7527.
RELNOTES: None.
PiperOrigin-RevId: 239412965
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCacheTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCacheTest.java
index 35ce53c..029c2d5 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCacheTest.java
@@ -60,7 +60,7 @@
@After
public void tearDown() throws IOException {
- FileSystemUtils.deleteTree(repositoryCachePath);
+ repositoryCachePath.deleteTree();
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java
index cd24422..f58c04c 100644
--- a/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java
@@ -234,7 +234,7 @@
fileSystem = FileSystems.getNativeFileSystem();
testRoot = fileSystem.getPath(TestUtils.tmpDir());
- FileSystemUtils.deleteTreesBelow(testRoot);
+ testRoot.deleteTreesBelow();
executorService = Executors.newCachedThreadPool();
inputArtifact =
new Artifact(
diff --git a/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java b/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
index 764bb8f..569dbb7 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
@@ -22,7 +22,6 @@
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.build.lib.util.Pair;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import java.util.ArrayList;
import java.util.Arrays;
@@ -111,7 +110,7 @@
@After
public final void deleteFiles() throws Exception {
- FileSystemUtils.deleteTreesBelow(scratch.getFileSystem().getPath("/"));
+ scratch.getFileSystem().getPath("/").deleteTreesBelow();
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockToolsConfig.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockToolsConfig.java
index c4af0c6..b76bbf1 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/MockToolsConfig.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockToolsConfig.java
@@ -93,7 +93,7 @@
public Path overwrite(String relativePath, String... lines) throws IOException {
Path path = rootDirectory.getRelative(relativePath);
if (path.exists()) {
- FileSystemUtils.deleteTree(path);
+ path.deleteTree();
}
return create(relativePath, lines);
}
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
index 7cb82ad..23360f6 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
@@ -1083,7 +1083,7 @@
workspace,
/* defaultSystemJavabase= */ null,
analysisMock.getProductName());
- FileSystemUtils.deleteTree(workspace.getRelative("base"));
+ workspace.getRelative("base").deleteTree();
ConfiguredRuleClassProvider ruleClassProvider = analysisMock.createRuleClassProvider();
PackageFactory pkgFactory =
diff --git a/src/test/java/com/google/devtools/build/lib/query2/engine/PostAnalysisQueryHelper.java b/src/test/java/com/google/devtools/build/lib/query2/engine/PostAnalysisQueryHelper.java
index 2423b2d..6563664 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/engine/PostAnalysisQueryHelper.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/engine/PostAnalysisQueryHelper.java
@@ -133,7 +133,7 @@
@Override
public void clearAllFiles() throws IOException {
- FileSystemUtils.deleteTree(analysisHelper.getRootDirectory());
+ analysisHelper.getRootDirectory().deleteTree();
}
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/BaseSandboxfsProcessTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/BaseSandboxfsProcessTest.java
index 4754032..9f5fa0c 100644
--- a/src/test/java/com/google/devtools/build/lib/sandbox/BaseSandboxfsProcessTest.java
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/BaseSandboxfsProcessTest.java
@@ -54,7 +54,7 @@
@After
public void tearDown() throws IOException {
- FileSystemUtils.deleteTreesBelow(tmpDir);
+ tmpDir.deleteTreesBelow();
tmpDir = null;
}
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/FakeSandboxfsProcess.java b/src/test/java/com/google/devtools/build/lib/sandbox/FakeSandboxfsProcess.java
index ec3f713a..86cdf42 100644
--- a/src/test/java/com/google/devtools/build/lib/sandbox/FakeSandboxfsProcess.java
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/FakeSandboxfsProcess.java
@@ -17,7 +17,6 @@
import static com.google.common.base.Preconditions.checkState;
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 java.io.IOException;
@@ -114,6 +113,6 @@
checkState(mapping.isAbsolute(), "Mapping specifications are expected to be absolute"
+ " but %s is not", mapping);
- FileSystemUtils.deleteTree(fileSystem.getPath(mountPoint).getRelative(mapping.toRelative()));
+ fileSystem.getPath(mountPoint).getRelative(mapping.toRelative()).deleteTree();
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
index d2016cd..79cd067 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
@@ -519,7 +519,7 @@
// We're done fiddling with this... restore the original state
outEmpty.getPath().delete();
- FileSystemUtils.deleteTree(dummyEmptyDir);
+ dummyEmptyDir.deleteTree();
FileSystemUtils.createDirectoryAndParents(outEmpty.getPath());
/* **** Tests for files and directory contents ****/
diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
index 2dbcced..42cb2c6 100644
--- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
@@ -53,7 +53,6 @@
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.util.io.FileOutErr;
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.common.options.Options;
@@ -89,7 +88,7 @@
fileSystem = FileSystems.getNativeFileSystem();
Path testRoot = fileSystem.getPath(TestUtils.tmpDir());
try {
- FileSystemUtils.deleteTreesBelow(testRoot);
+ testRoot.deleteTreesBelow();
} catch (IOException e) {
System.err.println("Failed to remove directory " + testRoot + ": " + e.getMessage());
throw e;
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
index b34c2f5..4bda98a 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
@@ -802,6 +802,87 @@
assertThat(xNonEmptyDirectoryFoo.isFile()).isTrue();
}
+ @Test
+ public void testDeleteTreeCommandDeletesTree() throws IOException {
+ Path topDir = absolutize("top-dir");
+ Path file1 = absolutize("top-dir/file-1");
+ Path file2 = absolutize("top-dir/file-2");
+ Path aDir = absolutize("top-dir/a-dir");
+ Path file3 = absolutize("top-dir/a-dir/file-3");
+ Path file4 = absolutize("file-4");
+
+ topDir.createDirectory();
+ FileSystemUtils.createEmptyFile(file1);
+ FileSystemUtils.createEmptyFile(file2);
+ aDir.createDirectory();
+ FileSystemUtils.createEmptyFile(file3);
+ FileSystemUtils.createEmptyFile(file4);
+
+ Path toDelete = topDir;
+ toDelete.deleteTree();
+
+ assertThat(file4.exists()).isTrue();
+ assertThat(topDir.exists()).isFalse();
+ assertThat(file1.exists()).isFalse();
+ assertThat(file2.exists()).isFalse();
+ assertThat(aDir.exists()).isFalse();
+ assertThat(file3.exists()).isFalse();
+ }
+
+ @Test
+ public void testDeleteTreeCommandsDeletesUnreadableDirectories() throws IOException {
+ Path topDir = absolutize("top-dir");
+ Path aDir = absolutize("top-dir/a-dir");
+
+ topDir.createDirectory();
+ aDir.createDirectory();
+
+ Path toDelete = topDir;
+
+ try {
+ aDir.setReadable(false);
+ } catch (UnsupportedOperationException e) {
+ // For file systems that do not support setting readable attribute to
+ // false, this test is simply skipped.
+
+ return;
+ }
+
+ toDelete.deleteTree();
+ assertThat(topDir.exists()).isFalse();
+ assertThat(aDir.exists()).isFalse();
+ }
+
+ @Test
+ public void testDeleteTreeCommandDoesNotFollowLinksOut() throws IOException {
+ Path topDir = absolutize("top-dir");
+ Path file1 = absolutize("top-dir/file-1");
+ Path file2 = absolutize("top-dir/file-2");
+ Path aDir = absolutize("top-dir/a-dir");
+ Path file3 = absolutize("top-dir/a-dir/file-3");
+ Path file4 = absolutize("file-4");
+
+ topDir.createDirectory();
+ FileSystemUtils.createEmptyFile(file1);
+ FileSystemUtils.createEmptyFile(file2);
+ aDir.createDirectory();
+ FileSystemUtils.createEmptyFile(file3);
+ FileSystemUtils.createEmptyFile(file4);
+
+ Path toDelete = topDir;
+ Path outboundLink = absolutize("top-dir/outbound-link");
+ outboundLink.createSymbolicLink(file4);
+
+ toDelete.deleteTree();
+
+ assertThat(file4.exists()).isTrue();
+ assertThat(topDir.exists()).isFalse();
+ assertThat(file1.exists()).isFalse();
+ assertThat(file2.exists()).isFalse();
+ assertThat(aDir.exists()).isFalse();
+ assertThat(file3.exists()).isFalse();
+ }
+
// Test the date functions
@Test
public void testCreateFileChangesTimeOfDirectory() throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemUtilsTest.java b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemUtilsTest.java
index cac3e38..2b1d57b 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemUtilsTest.java
@@ -18,7 +18,6 @@
import static com.google.devtools.build.lib.vfs.FileSystemUtils.commonAncestor;
import static com.google.devtools.build.lib.vfs.FileSystemUtils.copyFile;
import static com.google.devtools.build.lib.vfs.FileSystemUtils.copyTool;
-import static com.google.devtools.build.lib.vfs.FileSystemUtils.deleteTree;
import static com.google.devtools.build.lib.vfs.FileSystemUtils.moveFile;
import static com.google.devtools.build.lib.vfs.FileSystemUtils.relativePath;
import static com.google.devtools.build.lib.vfs.FileSystemUtils.removeExtension;
@@ -655,56 +654,6 @@
}
@Test
- public void testDeleteTreeCommandDeletesTree() throws IOException {
- createTestDirectoryTree();
- Path toDelete = topDir;
- deleteTree(toDelete);
-
- assertThat(file4.exists()).isTrue();
- assertThat(topDir.exists()).isFalse();
- assertThat(file1.exists()).isFalse();
- assertThat(file2.exists()).isFalse();
- assertThat(aDir.exists()).isFalse();
- assertThat(file3.exists()).isFalse();
- }
-
- @Test
- public void testDeleteTreeCommandsDeletesUnreadableDirectories() throws IOException {
- createTestDirectoryTree();
- Path toDelete = topDir;
-
- try {
- aDir.setReadable(false);
- } catch (UnsupportedOperationException e) {
- // For file systems that do not support setting readable attribute to
- // false, this test is simply skipped.
-
- return;
- }
-
- deleteTree(toDelete);
- assertThat(topDir.exists()).isFalse();
- assertThat(aDir.exists()).isFalse();
- }
-
- @Test
- public void testDeleteTreeCommandDoesNotFollowLinksOut() throws IOException {
- createTestDirectoryTree();
- Path toDelete = topDir;
- Path outboundLink = fileSystem.getPath("/top-dir/outbound-link");
- outboundLink.createSymbolicLink(file4);
-
- deleteTree(toDelete);
-
- assertThat(file4.exists()).isTrue();
- assertThat(topDir.exists()).isFalse();
- assertThat(file1.exists()).isFalse();
- assertThat(file2.exists()).isFalse();
- assertThat(aDir.exists()).isFalse();
- assertThat(file3.exists()).isFalse();
- }
-
- @Test
public void testWriteIsoLatin1() throws Exception {
Path file = fileSystem.getPath("/does/not/exist/yet.txt");
FileSystemUtils.writeIsoLatin1(file, "Line 1", "Line 2", "Line 3");
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/PathGetParentTest.java b/src/test/java/com/google/devtools/build/lib/vfs/PathGetParentTest.java
index 04def1c..7138b4c 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/PathGetParentTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/PathGetParentTest.java
@@ -42,7 +42,7 @@
@After
public final void deleteTestRoot() throws Exception {
- FileSystemUtils.deleteTree(testRoot); // (comment out during debugging)
+ testRoot.deleteTree(); // (comment out during debugging)
}
private Path getParent(String path) {
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/util/FsApparatus.java b/src/test/java/com/google/devtools/build/lib/vfs/util/FsApparatus.java
index 1f78b18..7c765fa 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/util/FsApparatus.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/util/FsApparatus.java
@@ -59,7 +59,7 @@
Path wd = fs.getPath(TMP_DIR);
try {
- FileSystemUtils.deleteTree(wd);
+ wd.deleteTree();
} catch (IOException e) {
throw new AssertionError(e.getMessage());
}
