diff options
5 files changed, 108 insertions, 54 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java index adeb9c847c..e42abdfd8a 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java @@ -616,15 +616,46 @@ public class FileSystemUtils { } /** - * Attempts to create a directory with the name of the given path, creating ancestors as - * necessary. + * Attempts to create a directory with the name of the given path, creating + * ancestors as necessary. * - * <p>Deprecated. Prefer to call {@link Path#createDirectoryAndParents()} directly. + * <p>Postcondition: completes normally iff {@code dir} denotes an existing + * directory (not necessarily canonical); completes abruptly otherwise. + * + * @return true if the directory was successfully created anew, false if it + * already existed (including the case where {@code dir} denotes a symlink + * to an existing directory) + * @throws IOException if the directory could not be created */ - @Deprecated @ThreadSafe - public static void createDirectoryAndParents(Path dir) throws IOException { - dir.createDirectoryAndParents(); + public static boolean createDirectoryAndParents(Path dir) throws IOException { + // Optimised for minimal number of I/O calls. + + // Don't attempt to create the root directory. + if (dir.getParentDirectory() == null) { + return false; + } + + FileSystem filesystem = dir.getFileSystem(); + if (filesystem instanceof UnionFileSystem) { + // If using UnionFS, make sure that we do not traverse filesystem boundaries when creating + // parent directories by rehoming the path on the most specific filesystem. + FileSystem delegate = ((UnionFileSystem) filesystem).getDelegate(dir); + dir = delegate.getPath(dir.asFragment()); + } + + try { + return dir.createDirectory(); + } catch (IOException e) { + if (e.getMessage().endsWith(" (No such file or directory)")) { // ENOENT + createDirectoryAndParents(dir.getParentDirectory()); + return dir.createDirectory(); + } else if (e.getMessage().endsWith(" (File exists)") && dir.isDirectory()) { // EEXIST + return false; + } else { + throw e; // some other error (e.g. ENOTDIR, EACCES, etc.) + } + } } /** 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 0dac1c4725..4477275f1f 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 @@ -261,15 +261,7 @@ public class JavaIoFileSystem extends AbstractFileSystemWithCustomStat { @Override public void createDirectoryAndParents(Path path) throws IOException { java.nio.file.Path nioPath = getNioPath(path); - try { - Files.createDirectories(nioPath); - } catch (java.nio.file.FileAlreadyExistsException e) { - // Files.createDirectories will handle this case normally, but if the existing - // file is a symlink to a directory then it still throws. Swallow this. - if (!path.isDirectory()) { - throw e; - } - } + Files.createDirectories(nioPath); } private boolean linkExists(File file) { diff --git a/src/main/java/com/google/devtools/build/lib/vfs/UnionFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/UnionFileSystem.java index d924e19d45..569357bcb2 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/UnionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/UnionFileSystem.java @@ -209,7 +209,7 @@ public class UnionFileSystem extends FileSystem { public void createDirectoryAndParents(Path path) throws IOException { checkModifiable(path); FileSystem delegate = getDelegate(path); - delegate.createDirectoryAndParents(adjustPath(path, delegate)); + delegate.createDirectoryAndParents(path); } @Override 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 e8896bbc0d..ba4b4e8c57 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 @@ -476,67 +476,47 @@ public abstract class FileSystemTest { } @Test - public void testCreateDirectoryAndParents() throws Exception { + public void testCreateDirectories() throws Exception { Path newPath = absolutize("new-dir/sub/directory"); newPath.createDirectoryAndParents(); assertThat(newPath.isDirectory()).isTrue(); } @Test - public void testCreateDirectoryAndParentsCreatesEmptyDirectory() throws Exception { + public void testCreateDirectoriesIsNotFile() throws Exception { Path newPath = absolutize("new-dir/sub/directory"); newPath.createDirectoryAndParents(); - assertThat(newPath.getDirectoryEntries()).isEmpty(); + assertThat(newPath.isFile()).isFalse(); } @Test - public void testCreateDirectoryAndParentsIsOnlyChildInParent() throws Exception { + public void testCreateDirectoriesIsNotSymbolicLink() throws Exception { Path newPath = absolutize("new-dir/sub/directory"); newPath.createDirectoryAndParents(); - assertThat(newPath.getParentDirectory().getDirectoryEntries()).hasSize(1); - assertThat(newPath.getParentDirectory().getDirectoryEntries()).containsExactly(newPath); + assertThat(newPath.isSymbolicLink()).isFalse(); } @Test - public void testCreateDirectoryAndParentsWhenAlreadyExistsSucceeds() throws Exception { - Path newPath = absolutize("new-dir"); - newPath.createDirectory(); + public void testCreateDirectoriesIsEmpty() throws Exception { + Path newPath = absolutize("new-dir/sub/directory"); newPath.createDirectoryAndParents(); - assertThat(newPath.isDirectory()).isTrue(); - } - - @Test - public void testCreateDirectoryAndParentsWhenAncestorIsFile() throws IOException { - Path path = absolutize("somewhere/deep/in"); - path.getParentDirectory().createDirectoryAndParents(); - FileSystemUtils.createEmptyFile(path); - Path theHierarchy = path.getChild("the-hierarchy"); - MoreAsserts.assertThrows(IOException.class, theHierarchy::createDirectoryAndParents); + assertThat(newPath.getDirectoryEntries()).isEmpty(); } @Test - public void testCreateDirectoryAndParentsWhenSymlinkToDir() throws IOException { - Path somewhereDeepIn = absolutize("somewhere/deep/in"); - somewhereDeepIn.createDirectoryAndParents(); - Path realDir = absolutize("real/dir"); - realDir.createDirectoryAndParents(); - assertThat(realDir.isDirectory()).isTrue(); - Path theHierarchy = somewhereDeepIn.getChild("the-hierarchy"); - theHierarchy.createSymbolicLink(realDir); - assertThat(theHierarchy.isDirectory()).isTrue(); - theHierarchy.createDirectoryAndParents(); + public void testCreateDirectoriesIsOnlyChildInParent() throws Exception { + Path newPath = absolutize("new-dir/sub/directory"); + newPath.createDirectoryAndParents(); + assertThat(newPath.getParentDirectory().getDirectoryEntries()).hasSize(1); + assertThat(newPath.getParentDirectory().getDirectoryEntries()).containsExactly(newPath); } @Test - public void testCreateDirectoryAndParentsWhenSymlinkEmbedded() throws IOException { - Path somewhereDeepIn = absolutize("somewhere/deep/in"); - somewhereDeepIn.createDirectoryAndParents(); - Path realDir = absolutize("real/dir"); - realDir.createDirectoryAndParents(); - Path the = somewhereDeepIn.getChild("the"); - the.createSymbolicLink(realDir); - Path theHierarchy = somewhereDeepIn.getChild("hierarchy"); - theHierarchy.createDirectoryAndParents(); + public void testCreateAlreadyExistingDirectorySucceeds() throws Exception { + Path newPath = absolutize("new-dir"); + newPath.createDirectory(); + newPath.createDirectoryAndParents(); + assertThat(newPath.isDirectory()).isTrue(); } @Test 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 656363333e..0e9b74ed3b 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,6 +18,7 @@ import static com.google.devtools.build.lib.vfs.FileSystemUtils.appendWithoutExt 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.createDirectoryAndParents; 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; @@ -644,6 +645,56 @@ public class FileSystemUtilsTest { } @Test + public void testCreateDirectories() throws IOException { + Path mainPath = fileSystem.getPath("/some/where/deep/in/the/hierarchy"); + assertThat(createDirectoryAndParents(mainPath)).isTrue(); + assertThat(mainPath.exists()).isTrue(); + assertThat(createDirectoryAndParents(mainPath)).isFalse(); + } + + @Test + public void testCreateDirectoriesWhenAncestorIsFile() throws IOException { + Path somewhereDeepIn = fileSystem.getPath("/somewhere/deep/in"); + assertThat(createDirectoryAndParents(somewhereDeepIn.getParentDirectory())).isTrue(); + FileSystemUtils.createEmptyFile(somewhereDeepIn); + Path theHierarchy = somewhereDeepIn.getChild("the-hierarchy"); + try { + createDirectoryAndParents(theHierarchy); + fail(); + } catch (IOException e) { + assertThat(e).hasMessage("/somewhere/deep/in (Not a directory)"); + } + } + + @Test + public void testCreateDirectoriesWhenSymlinkToDir() throws IOException { + Path somewhereDeepIn = fileSystem.getPath("/somewhere/deep/in"); + assertThat(createDirectoryAndParents(somewhereDeepIn)).isTrue(); + Path realDir = fileSystem.getPath("/real/dir"); + assertThat(createDirectoryAndParents(realDir)).isTrue(); + + Path theHierarchy = somewhereDeepIn.getChild("the-hierarchy"); + theHierarchy.createSymbolicLink(realDir); + + assertThat(createDirectoryAndParents(theHierarchy)).isFalse(); + } + + @Test + public void testCreateDirectoriesWhenSymlinkEmbedded() throws IOException { + Path somewhereDeepIn = fileSystem.getPath("/somewhere/deep/in"); + assertThat(createDirectoryAndParents(somewhereDeepIn)).isTrue(); + Path realDir = fileSystem.getPath("/real/dir"); + assertThat(createDirectoryAndParents(realDir)).isTrue(); + + Path the = somewhereDeepIn.getChild("the"); + the.createSymbolicLink(realDir); + + Path theHierarchy = somewhereDeepIn.getChild("hierarchy"); + assertThat(createDirectoryAndParents(theHierarchy)).isTrue(); + } + + + @Test public void testWriteIsoLatin1() throws Exception { Path file = fileSystem.getPath("/does/not/exist/yet.txt"); FileSystemUtils.writeIsoLatin1(file, "Line 1", "Line 2", "Line 3"); |