Add FileSystem#createDirectoryAndParents.

A native implementation of this (instead of using FileSystemUtils, which can only use public interfaces) should be more efficient and more easy to make correct.

In particular, it should allow removing FileSystemUtils#createDirectoriesAndParents, which has poor thread safety characteristics. The latter method has a lot of logic that forces certain unnatural atomicity guarantees on createDirectory, and it also has logic that is conditional on sub-string content of exception messages.

PiperOrigin-RevId: 179819623

9 files changed, 84 insertions, 10 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/unix/NativePosixFiles.java b/src/main/java/com/google/devtools/build/lib/unix/NativePosixFiles.java
index 781467d366..238ef4d2e4 100644
--- a/src/main/java/com/google/devtools/build/lib/unix/NativePosixFiles.java
+++ b/src/main/java/com/google/devtools/build/lib/unix/NativePosixFiles.java
@@ -242,6 +242,15 @@ public final class NativePosixFiles {
       throws IOException;
 
   /**
+   * Implements (effectively) mkdir -p.
+   *
+   * @param path the directory to recursively create.
+   * @param mode the mode with which to create the directories.
+   * @throws IOException if the directory creation failed for any reason.
+   */
+  public static native void mkdirs(String path, int mode) throws IOException;
+
+  /**
    * Native wrapper around POSIX opendir(2)/readdir(3)/closedir(3) syscall.
    *
    * @param path the directory to read.
diff --git a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
index 7b82dcc06a..0af17cfb12 100644
--- a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
@@ -325,6 +325,11 @@ public class UnixFileSystem extends AbstractFileSystemWithCustomStat {
   }
 
   @Override
+  public void createDirectoryAndParents(Path path) throws IOException {
+    NativePosixFiles.mkdirs(path.toString(), 0777);
+  }
+
+  @Override
   protected void createSymbolicLink(Path linkPath, PathFragment targetFragment)
       throws IOException {
     synchronized (linkPath) {
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
index fef88b892b..e914663695 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
@@ -257,6 +257,12 @@ public abstract class FileSystem {
   public abstract boolean createDirectory(Path path) throws IOException;
 
   /**
+   * Creates all directories up to the path. See {@link Path#createDirectoryAndParents} for
+   * specification.
+   */
+  public abstract void createDirectoryAndParents(Path path) throws IOException;
+
+  /**
    * Returns the size in bytes of the file denoted by {@code path}. See {@link
    * Path#getFileSize(Symlinks)} for specification.
    *
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 dca8b95906..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
@@ -258,6 +258,12 @@ public class JavaIoFileSystem extends AbstractFileSystemWithCustomStat {
     }
   }
 
+  @Override
+  public void createDirectoryAndParents(Path path) throws IOException {
+    java.nio.file.Path nioPath = getNioPath(path);
+    Files.createDirectories(nioPath);
+  }
+
   private boolean linkExists(File file) {
     String shortName = file.getName();
     File parentFile = file.getParentFile();
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Path.java b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
index 1d3947dcdc..06a884b4a9 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/Path.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
@@ -1026,6 +1026,19 @@ public class Path implements Comparable<Path>, Serializable, SkylarkPrintable {
     return fileSystem.createDirectory(this);
   }
 
+  /**
+   * Ensures that the directory with the name of the current path and all its ancestor directories
+   * exist.
+   *
+   * <p>Does not return whether the directory already existed or was created by some other
+   * concurrent call to this method.
+   *
+   * @throws IOException if the directory creation failed for any reason
+   */
+  public void createDirectoryAndParents() throws IOException {
+    fileSystem.createDirectoryAndParents(this);
+  }
+
   /** Prefer to use {@link #createSymbolicLink(FileSystem, Path)}. */
   @Deprecated
   public void createSymbolicLink(Path target) throws IOException {
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystem.java
index bfcc4f97e0..938f7a7ffb 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystem.java
@@ -17,20 +17,21 @@ import java.io.IOException;
 import java.io.OutputStream;
 
 /**
- * An abstract partial implementation of FileSystem for read-only
- * implementations.
+ * An abstract partial implementation of FileSystem for read-only implementations.
  *
  * <p>Any ReadonlyFileSystem does not support the following:
+ *
  * <ul>
- * <li>{@link #createDirectory(Path)}</li>
- * <li>{@link #createSymbolicLink(Path, PathFragment)}</li>
- * <li>{@link #delete(Path)}</li>
- * <li>{@link #getOutputStream(Path)}</li>
- * <li>{@link #renameTo(Path, Path)}</li>
- * <li>{@link #setExecutable(Path, boolean)}</li>
- * <li>{@link #setLastModifiedTime(Path, long)}</li>
- * <li>{@link #setWritable(Path, boolean)}</li>
+ *   <li>{@link #createDirectory(Path)}
+ *   <li>{@link #createSymbolicLink(Path, PathFragment)}
+ *   <li>{@link #delete(Path)}
+ *   <li>{@link #getOutputStream(Path)}
+ *   <li>{@link #renameTo(Path, Path)}
+ *   <li>{@link #setExecutable(Path, boolean)}
+ *   <li>{@link #setLastModifiedTime(Path, long)}
+ *   <li>{@link #setWritable(Path, boolean)}
  * </ul>
+ *
  * The above calls will always result in an {@link IOException}.
  */
 public abstract class ReadonlyFileSystem extends AbstractFileSystem {
@@ -91,6 +92,11 @@ public abstract class ReadonlyFileSystem extends AbstractFileSystem {
   }
 
   @Override
+  public void createDirectoryAndParents(Path path) throws IOException {
+    throw modificationException();
+  }
+
+  @Override
   protected void createSymbolicLink(Path linkPath, PathFragment targetFragment) throws IOException {
     throw modificationException();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystemWithCustomStat.java b/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystemWithCustomStat.java
index de5daca181..f9ca4ebb87 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystemWithCustomStat.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystemWithCustomStat.java
@@ -76,6 +76,11 @@ public abstract class ReadonlyFileSystemWithCustomStat extends AbstractFileSyste
   }
 
   @Override
+  public void createDirectoryAndParents(Path path) throws IOException {
+    throw modificationException();
+  }
+
+  @Override
   protected void createSymbolicLink(Path linkPath, PathFragment targetFragment) throws IOException {
     throw modificationException();
   }
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 824e71dc69..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
@@ -206,6 +206,13 @@ public class UnionFileSystem extends FileSystem {
   }
 
   @Override
+  public void createDirectoryAndParents(Path path) throws IOException {
+    checkModifiable(path);
+    FileSystem delegate = getDelegate(path);
+    delegate.createDirectoryAndParents(path);
+  }
+
+  @Override
   protected long getFileSize(Path path, boolean followSymlinks) throws IOException {
     path = followSymlinks ? internalResolveSymlink(path) : path;
     FileSystem delegate = getDelegate(path);
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java
index ff6d88a387..0008423cc9 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.vfs.inmemoryfs;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.clock.JavaClock;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
@@ -624,6 +625,22 @@ public class InMemoryFileSystem extends FileSystem {
   }
 
   @Override
+  public synchronized void createDirectoryAndParents(Path path) throws IOException {
+    List<Path> subdirs = new ArrayList<>();
+    for (; !path.isRootDirectory(); path = path.getParentDirectory()) {
+      if (path.isDirectory()) {
+        break;
+      } else if (path.exists()) {
+        throw new IOException("Not a directory: " + path);
+      }
+      subdirs.add(path);
+    }
+    for (Path subdir : Lists.reverse(subdirs)) {
+      subdir.createDirectory();
+    }
+  }
+
+  @Override
   protected void createSymbolicLink(Path path, PathFragment targetFragment)
       throws IOException {
     if (path.equals(getRootDirectory())) {