Rollup merge of #139870 - Shourya742:2025-04-15-add-retries-to-remove_and_create_dir_all, r=jieyouxu

add retries to remove and create dir all

closes: #139230

r? ```@jieyouxu```

src/build_helper/src/fs/mod.rs

@@ -22,21 +22,27 @@ where
 /// A wrapper around [`std::fs::remove_dir_all`] that can also be used on *non-directory entries*,
 /// including files and symbolic links.
 ///
-/// - This will produce an error if the target path is not found.
+/// - This will not produce an error if the target path is not found.
 /// - Like [`std::fs::remove_dir_all`], this helper does not traverse symbolic links, will remove
 ///   symbolic link itself.
 /// - This helper is **not** robust against races on the underlying filesystem, behavior is
 ///   unspecified if this helper is called concurrently.
 /// - This helper is not robust against TOCTOU problems.
 ///
-/// FIXME: this implementation is insufficiently robust to replace bootstrap's clean `rm_rf`
-/// implementation:
-///
-/// - This implementation currently does not perform retries.
+/// FIXME: Audit whether this implementation is robust enough to replace bootstrap's clean `rm_rf`.
 #[track_caller]
 pub fn recursive_remove<P: AsRef<Path>>(path: P) -> io::Result<()> {
     let path = path.as_ref();
-    let metadata = fs::symlink_metadata(path)?;
+
+    // If the path doesn't exist, we treat it as a successful no-op.
+    // From the caller's perspective, the goal is simply "ensure this file/dir is gone" —
+    // if it's already not there, that's a success, not an error.
+    let metadata = match fs::symlink_metadata(path) {
+        Ok(m) => m,
+        Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(()),
+        Err(e) => return Err(e),
+    };
+
     #[cfg(windows)]
     let is_dir_like = |meta: &fs::Metadata| {
         use std::os::windows::fs::FileTypeExt;
@@ -45,11 +51,35 @@ pub fn recursive_remove<P: AsRef<Path>>(path: P) -> io::Result<()> {
     #[cfg(not(windows))]
     let is_dir_like = fs::Metadata::is_dir;
 
-    if is_dir_like(&metadata) {
-        fs::remove_dir_all(path)
-    } else {
-        try_remove_op_set_perms(fs::remove_file, path, metadata)
+    const MAX_RETRIES: usize = 5;
+    const RETRY_DELAY_MS: u64 = 100;
+
+    let try_remove = || {
+        if is_dir_like(&metadata) {
+            fs::remove_dir_all(path)
+        } else {
+            try_remove_op_set_perms(fs::remove_file, path, metadata.clone())
+        }
+    };
+
+    // Retry deletion a few times to handle transient filesystem errors.
+    // This is unusual for local file operations, but it's a mitigation
+    // against unlikely events where malware scanners may be holding a
+    // file beyond our control, to give the malware scanners some opportunity
+    // to release their hold.
+    for attempt in 0..MAX_RETRIES {
+        match try_remove() {
+            Ok(()) => return Ok(()),
+            Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(()),
+            Err(_) if attempt < MAX_RETRIES - 1 => {
+                std::thread::sleep(std::time::Duration::from_millis(RETRY_DELAY_MS));
+                continue;
+            }
+            Err(e) => return Err(e),
+        }
     }
+
+    Ok(())
 }
 
 fn try_remove_op_set_perms<'p, Op>(mut op: Op, path: &'p Path, metadata: Metadata) -> io::Result<()>
@@ -67,3 +97,9 @@ where
         Err(e) => Err(e),
     }
 }
+
+pub fn remove_and_create_dir_all<P: AsRef<Path>>(path: P) -> io::Result<()> {
+    let path = path.as_ref();
+    recursive_remove(path)?;
+    fs::create_dir_all(path)
+}

src/build_helper/src/fs/tests.rs

@@ -14,7 +14,7 @@ mod recursive_remove_tests {
         let tmpdir = env::temp_dir();
         let path = tmpdir.join("__INTERNAL_BOOTSTRAP_nonexistent_path");
         assert!(fs::symlink_metadata(&path).is_err_and(|e| e.kind() == io::ErrorKind::NotFound));
-        assert!(recursive_remove(&path).is_err_and(|e| e.kind() == io::ErrorKind::NotFound));
+        assert!(recursive_remove(&path).is_ok());
     }
 
     #[test]

src/tools/compiletest/src/runtest.rs

@@ -9,6 +9,7 @@ use std::process::{Child, Command, ExitStatus, Output, Stdio};
 use std::sync::Arc;
 use std::{env, iter, str};
 
+use build_helper::fs::remove_and_create_dir_all;
 use camino::{Utf8Path, Utf8PathBuf};
 use colored::Colorize;
 use regex::{Captures, Regex};
@@ -207,12 +208,6 @@ pub fn compute_stamp_hash(config: &Config) -> String {
     format!("{:x}", hash.finish())
 }
 
-fn remove_and_create_dir_all(path: &Utf8Path) {
-    let path = path.as_std_path();
-    let _ = fs::remove_dir_all(path);
-    fs::create_dir_all(path).unwrap();
-}
-
 #[derive(Copy, Clone, Debug)]
 struct TestCx<'test> {
     config: &'test Config,
@@ -523,7 +518,9 @@ impl<'test> TestCx<'test> {
         let mut rustc = Command::new(&self.config.rustc_path);
 
         let out_dir = self.output_base_name().with_extension("pretty-out");
-        remove_and_create_dir_all(&out_dir);
+        remove_and_create_dir_all(&out_dir).unwrap_or_else(|e| {
+            panic!("failed to remove and recreate output directory `{out_dir}`: {e}")
+        });
 
         let target = if self.props.force_host { &*self.config.host } else { &*self.config.target };
 
@@ -1098,13 +1095,19 @@ impl<'test> TestCx<'test> {
         let aux_dir = self.aux_output_dir_name();
 
         if !self.props.aux.builds.is_empty() {
-            remove_and_create_dir_all(&aux_dir);
+            remove_and_create_dir_all(&aux_dir).unwrap_or_else(|e| {
+                panic!("failed to remove and recreate output directory `{aux_dir}`: {e}")
+            });
         }
 
         if !self.props.aux.bins.is_empty() {
             let aux_bin_dir = self.aux_bin_output_dir_name();
-            remove_and_create_dir_all(&aux_dir);
-            remove_and_create_dir_all(&aux_bin_dir);
+            remove_and_create_dir_all(&aux_dir).unwrap_or_else(|e| {
+                panic!("failed to remove and recreate output directory `{aux_dir}`: {e}")
+            });
+            remove_and_create_dir_all(&aux_bin_dir).unwrap_or_else(|e| {
+                panic!("failed to remove and recreate output directory `{aux_bin_dir}`: {e}")
+            });
         }
 
         aux_dir
@@ -1509,7 +1512,9 @@ impl<'test> TestCx<'test> {
 
         let set_mir_dump_dir = |rustc: &mut Command| {
             let mir_dump_dir = self.get_mir_dump_dir();
-            remove_and_create_dir_all(&mir_dump_dir);
+            remove_and_create_dir_all(&mir_dump_dir).unwrap_or_else(|e| {
+                panic!("failed to remove and recreate output directory `{mir_dump_dir}`: {e}")
+            });
             let mut dir_opt = "-Zdump-mir-dir=".to_string();
             dir_opt.push_str(mir_dump_dir.as_str());
             debug!("dir_opt: {:?}", dir_opt);
@@ -1969,7 +1974,9 @@ impl<'test> TestCx<'test> {
         let suffix =
             self.safe_revision().map_or("nightly".into(), |path| path.to_owned() + "-nightly");
         let compare_dir = output_base_dir(self.config, self.testpaths, Some(&suffix));
-        remove_and_create_dir_all(&compare_dir);
+        remove_and_create_dir_all(&compare_dir).unwrap_or_else(|e| {
+            panic!("failed to remove and recreate output directory `{compare_dir}`: {e}")
+        });
 
         // We need to create a new struct for the lifetimes on `config` to work.
         let new_rustdoc = TestCx {

src/tools/compiletest/src/runtest/rustdoc.rs

@@ -7,7 +7,9 @@ impl TestCx<'_> {
         assert!(self.revision.is_none(), "revisions not relevant here");
 
         let out_dir = self.output_base_dir();
-        remove_and_create_dir_all(&out_dir);
+        remove_and_create_dir_all(&out_dir).unwrap_or_else(|e| {
+            panic!("failed to remove and recreate output directory `{out_dir}`: {e}")
+        });
 
         let proc_res = self.document(&out_dir, &self.testpaths);
         if !proc_res.status.success() {

src/tools/compiletest/src/runtest/rustdoc_json.rs

@@ -9,7 +9,9 @@ impl TestCx<'_> {
         assert!(self.revision.is_none(), "revisions not relevant here");
 
         let out_dir = self.output_base_dir();
-        remove_and_create_dir_all(&out_dir);
+        remove_and_create_dir_all(&out_dir).unwrap_or_else(|e| {
+            panic!("failed to remove and recreate output directory `{out_dir}`: {e}")
+        });
 
         let proc_res = self.document(&out_dir, &self.testpaths);
         if !proc_res.status.success() {