diff --git a/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java index ea541b4a1e020a..25fafac67bb3f7 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java @@ -343,22 +343,42 @@ protected long getFileSize(Path path, boolean followSymlinks) throws IOException @Override public boolean delete(Path path) throws IOException { - File file = getIoFile(path); + java.nio.file.Path nioPath = getNioPath(path); long startTime = Profiler.nanoTimeMaybe(); try { - if (file.delete()) { - return true; - } - if (file.exists()) { - if (file.isDirectory() && file.list().length > 0) { - throw new IOException(path + ERR_DIRECTORY_NOT_EMPTY); - } else { - throw new IOException(path + ERR_PERMISSION_DENIED); - } + return Files.deleteIfExists(nioPath); + } catch (java.nio.file.DirectoryNotEmptyException e) { + throw new IOException(path.getPathString() + ERR_DIRECTORY_NOT_EMPTY); + } catch (java.nio.file.AccessDeniedException e) { + throw new IOException(path.getPathString() + ERR_PERMISSION_DENIED); + } catch (java.nio.file.AtomicMoveNotSupportedException + | java.nio.file.FileAlreadyExistsException + | java.nio.file.FileSystemLoopException + | java.nio.file.NoSuchFileException + | java.nio.file.NotDirectoryException + | java.nio.file.NotLinkException e) { + // All known but unexpected subclasses of FileSystemException. + throw new IOException(path.getPathString() + ": unexpected FileSystemException", e); + } catch (java.nio.file.FileSystemException e) { + // Files.deleteIfExists() throws FileSystemException on Linux if a path component is a file. + // We caught all known subclasses of FileSystemException so `e` is either an unknown + // subclass or it is indeed a "Not a directory" error. Non-English JDKs may use a different + // error message than "Not a directory", so we should not look for that text. Checking the + // parent directory if it's indeed a directory is unrealiable, because another process may + // modify it concurrently... but we have no better choice. + if (e.getClass().equals(java.nio.file.FileSystemException.class) + && !nioPath.getParent().toFile().isDirectory()) { + // Hopefully the try-block failed because a parent directory was in fact not a directory. + // Theoretically it's possible that the try-block failed for some other reason and all + // parent directories were indeed directories, but another process changed a parent + // directory into a file after the try-block failed but before this catch-block started, and + // we return false here losing the real exception in `e`, but we cannot know. + return false; + } else { + throw new IOException(path.getPathString() + ": unexpected FileSystemException", e); } - return false; } finally { - profiler.logSimpleTask(startTime, ProfilerTask.VFS_DELETE, file.getPath()); + profiler.logSimpleTask(startTime, ProfilerTask.VFS_DELETE, path.getPathString()); } } diff --git a/src/main/java/com/google/devtools/build/lib/windows/BUILD b/src/main/java/com/google/devtools/build/lib/windows/BUILD index 4b6e08c73a3285..61eb83acee2bcf 100644 --- a/src/main/java/com/google/devtools/build/lib/windows/BUILD +++ b/src/main/java/com/google/devtools/build/lib/windows/BUILD @@ -33,6 +33,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:os_util", "//src/main/java/com/google/devtools/build/lib/clock", "//src/main/java/com/google/devtools/build/lib/concurrent", + "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/windows/jni", diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java index 0fd7028d6b7b62..869ba85d627039 100644 --- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java @@ -15,6 +15,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; +import com.google.devtools.build.lib.profiler.Profiler; +import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.JavaIoFileSystem; @@ -48,6 +50,20 @@ public String getFileSystemType(Path path) { return "ntfs"; } + @Override + public boolean delete(Path path) throws IOException { + long startTime = Profiler.nanoTimeMaybe(); + try { + return WindowsFileOperations.deletePath(path.getPathString()); + } catch (java.nio.file.DirectoryNotEmptyException e) { + throw new IOException(path.getPathString() + ERR_DIRECTORY_NOT_EMPTY); + } catch (java.nio.file.AccessDeniedException e) { + throw new IOException(path.getPathString() + ERR_PERMISSION_DENIED); + } finally { + profiler.logSimpleTask(startTime, ProfilerTask.VFS_DELETE, path.getPathString()); + } + } + @Override protected void createSymbolicLink(Path linkPath, PathFragment targetFragment) throws IOException { Path targetPath = diff --git a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java index 32868c204db75a..6a16ab8aab7668 100644 --- a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java +++ b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java @@ -49,12 +49,21 @@ private WindowsFileOperations() { private static final int IS_JUNCTION_NO = 1; private static final int IS_JUNCTION_ERROR = 2; + // Keep DELETE_PATH_* values in sync with src/main/native/windows/file.cc. + private static final int DELETE_PATH_SUCCESS = 0; + private static final int DELETE_PATH_DOES_NOT_EXIST = 1; + private static final int DELETE_PATH_DIRECTORY_NOT_EMPTY = 2; + private static final int DELETE_PATH_ACCESS_DENIED = 3; + private static final int DELETE_PATH_ERROR = 4; + private static native int nativeIsJunction(String path, String[] error); private static native boolean nativeGetLongPath(String path, String[] result, String[] error); private static native boolean nativeCreateJunction(String name, String target, String[] error); + private static native int nativeDeletePath(String path, String[] error); + /** Determines whether `path` is a junction point or directory symlink. */ public static boolean isJunction(String path) throws IOException { WindowsJniLoader.loadJni(); @@ -121,4 +130,22 @@ public static void createJunction(String name, String target) throws IOException String.format("Cannot create junction (name=%s, target=%s): %s", name, target, error[0])); } } + + public static boolean deletePath(String path) throws IOException { + WindowsJniLoader.loadJni(); + String[] error = new String[] {null}; + int result = nativeDeletePath(asLongPath(path), error); + switch (result) { + case DELETE_PATH_SUCCESS: + return true; + case DELETE_PATH_DOES_NOT_EXIST: + return false; + case DELETE_PATH_DIRECTORY_NOT_EMPTY: + throw new java.nio.file.DirectoryNotEmptyException(path); + case DELETE_PATH_ACCESS_DENIED: + throw new java.nio.file.AccessDeniedException(path); + default: + throw new IOException(String.format("Cannot delete path '%s': %s", path, error[0])); + } + } }