Remove safe_create_dir(_all) in favor of stdlib implementation. (#1…

…8605)

Fixes #18548.

src/rust/engine/fs/src/lib.rs

@@ -833,40 +833,5 @@ pub fn increase_limits() -> Result<String, String> {
   }
 }
 
-///
-/// Like std::fs::create_dir_all, except handles concurrent calls among multiple
-/// threads or processes. Originally lifted from rustc.
-///
-pub fn safe_create_dir_all_ioerror(path: &Path) -> Result<(), io::Error> {
-  match fs::create_dir(path) {
-    Ok(()) => return Ok(()),
-    Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists => return Ok(()),
-    Err(ref e) if e.kind() == io::ErrorKind::NotFound => {}
-    Err(e) => return Err(e),
-  }
-  match path.parent() {
-    Some(p) => safe_create_dir_all_ioerror(p)?,
-    None => return Ok(()),
-  }
-  match fs::create_dir(path) {
-    Ok(()) => Ok(()),
-    Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists => Ok(()),
-    Err(e) => Err(e),
-  }
-}
-
-pub fn safe_create_dir_all(path: &Path) -> Result<(), String> {
-  safe_create_dir_all_ioerror(path)
-    .map_err(|e| format!("Failed to create dir {path:?} due to {e:?}"))
-}
-
-pub fn safe_create_dir(path: &Path) -> Result<(), String> {
-  match fs::create_dir(path) {
-    Ok(()) => Ok(()),
-    Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists => Ok(()),
-    Err(err) => Err(format!("{err}")),
-  }
-}
-
 #[cfg(test)]
 mod tests;

src/rust/engine/fs/store/benches/store.rs

@@ -321,7 +321,7 @@ fn tempdir_containing(max_files: usize, file_target_size: usize) -> (TempDir, Ve
     // We use the (repeated) path as the content as well.
     let abs_path = tempdir.path().join(path.path());
     if let Some(parent) = abs_path.parent() {
-      fs::safe_create_dir_all(parent).unwrap();
+      std::fs::create_dir_all(parent).unwrap();
     }
     let mut f = BufWriter::new(std::fs::File::create(abs_path).unwrap());
     let bytes = path.path().as_os_str().as_bytes();

src/rust/engine/fs/store/src/lib.rs

@@ -56,7 +56,7 @@ use fs::{
   FileEntry, Link, PathStat, Permissions, RelativePath, SymlinkBehavior, SymlinkEntry,
   EMPTY_DIRECTORY_DIGEST,
 };
-use futures::future::{self, BoxFuture, Either, FutureExt, TryFutureExt};
+use futures::future::{self, BoxFuture, Either, FutureExt};
 use grpc_util::prost::MessageExt;
 use hashing::{Digest, Fingerprint};
 use local::ByteStore;
@@ -1244,7 +1244,9 @@ impl Store {
           {
             let destination = destination.clone();
             move || {
-              fs::safe_create_dir_all(&destination)?;
+              std::fs::create_dir_all(&destination).map_err(|e| {
+                format!("Failed to create directory {}: {e}", destination.display())
+              })?;
               let dest_device = destination
                 .metadata()
                 .map_err(|e| {
@@ -1259,8 +1261,7 @@ impl Store {
           },
           |e| Err(format!("Directory creation task failed: {e}")),
         )
-        .await
-        .map_err(|e| format!("Failed to create directory {}: {e}", destination.display()))?
+        .await?
     };
 
     self
@@ -1289,16 +1290,12 @@ impl Store {
     let store = self.clone();
     async move {
       if !is_root {
-        let destination2 = destination.clone();
-        store
-          .local
-          .executor()
-          .spawn_blocking(
-            move || fs::safe_create_dir(&destination2),
-            |e| Err(format!("Directory creation task failed: {e}")),
-          )
-          .map_err(|e| format!("Failed to create directory {}: {e}", destination.display(),))
-          .await?;
+        // NB: Although we know that all parent directories already exist, we use `create_dir_all`
+        // because it succeeds even if _this_ directory already exists (which it might, if we're
+        // materializing atop an existing directory structure).
+        tokio::fs::create_dir_all(&destination)
+          .await
+          .map_err(|e| format!("Failed to create directory {}: {e}", destination.display()))?;
       }
 
       if let Some(children) = parent_to_child.get(&destination) {

src/rust/engine/fs/store/src/local.rs

@@ -425,7 +425,8 @@ impl ByteStore {
     let lmdb_directories_root = root.join("directories");
     let fsdb_files_root = root.join("immutable").join("files");
 
-    fs::safe_create_dir_all(path.as_ref())?;
+    std::fs::create_dir_all(root)
+      .map_err(|e| format!("Failed to create {}: {e}", root.display()))?;
 
     let filesystem_device = root
       .metadata()

src/rust/engine/sharded_lmdb/src/lib.rs

@@ -220,7 +220,7 @@ impl ShardedLmdb {
     let mut envs = Vec::with_capacity(shard_count as usize);
     for b in 0..shard_count {
       let dir = root_path.join(format!("{b:x}"));
-      fs::safe_create_dir_all(&dir)
+      std::fs::create_dir_all(&dir)
         .map_err(|err| format!("Error making directory for store at {dir:?}: {err:?}"))?;
       let fingerprint_prefix = b.rotate_left(shard_shift as u32);
       envs.push((

src/rust/engine/src/context.rs

@@ -20,7 +20,7 @@ use crate::types::Types;
 
 use async_oncecell::OnceCell;
 use cache::PersistentCache;
-use fs::{safe_create_dir_all_ioerror, GitignoreStyleExcludes, PosixFS};
+use fs::{GitignoreStyleExcludes, PosixFS};
 use futures::FutureExt;
 use graph::{self, EntryId, Graph, InvalidationResult, NodeContext};
 use hashing::Digest;
@@ -495,7 +495,7 @@ impl Core {
       None
     };
 
-    safe_create_dir_all_ioerror(&local_store_options.store_dir).map_err(|e| {
+    std::fs::create_dir_all(&local_store_options.store_dir).map_err(|e| {
       format!(
         "Error making directory {:?}: {:?}",
         local_store_options.store_dir, e