run_make_support: coalesce fs helpers into single fs module

There were *two* `read_dir` helpers, one being a simple
`std::fs::read_dir` wrapper, the other has a different callback-based
signature. We also rename the callback-based `read_dir` as
`read_dir_entries`.

Also don't top-level re-export most `fs::*` helpers.

src/tools/run-make-support/src/assertion_helpers.rs

@@ -3,8 +3,7 @@
 use std::panic;
 use std::path::{Path, PathBuf};
 
-use crate::fs_helpers;
-use crate::fs_wrapper;
+use crate::fs as rfs;
 use crate::path_helpers::cwd;
 
 /// Browse the directory `path` non-recursively and return all files which respect the parameters
@@ -15,7 +14,7 @@ pub fn shallow_find_files<P: AsRef<Path>, F: Fn(&PathBuf) -> bool>(
     filter: F,
 ) -> Vec<PathBuf> {
     let mut matching_files = Vec::new();
-    for entry in fs_wrapper::read_dir(path) {
+    for entry in rfs::read_dir(path) {
         let entry = entry.expect("failed to read directory entry.");
         let path = entry.path();
 
@@ -61,7 +60,7 @@ pub fn count_regex_matches_in_files_with_extension(re: &regex::Regex, ext: &str)
 
     let mut count = 0;
     for file in fetched_files {
-        let content = fs_wrapper::read_to_string(file);
+        let content = rfs::read_to_string(file);
         count += content.lines().filter(|line| re.is_match(&line)).count();
     }
 
@@ -69,11 +68,11 @@ pub fn count_regex_matches_in_files_with_extension(re: &regex::Regex, ext: &str)
 }
 
 /// Read the contents of a file that cannot simply be read by
-/// [`read_to_string`][crate::fs_wrapper::read_to_string], due to invalid UTF-8 data, then assert
+/// [`read_to_string`][crate::fs::read_to_string], due to invalid UTF-8 data, then assert
 /// that it contains `expected`.
 #[track_caller]
 pub fn invalid_utf8_contains<P: AsRef<Path>, S: AsRef<str>>(path: P, expected: S) {
-    let buffer = fs_wrapper::read(path.as_ref());
+    let buffer = rfs::read(path.as_ref());
     let expected = expected.as_ref();
     if !String::from_utf8_lossy(&buffer).contains(expected) {
         eprintln!("=== FILE CONTENTS (LOSSY) ===");
@@ -85,11 +84,11 @@ pub fn invalid_utf8_contains<P: AsRef<Path>, S: AsRef<str>>(path: P, expected: S
 }
 
 /// Read the contents of a file that cannot simply be read by
-/// [`read_to_string`][crate::fs_wrapper::read_to_string], due to invalid UTF-8 data, then assert
+/// [`read_to_string`][crate::fs::read_to_string], due to invalid UTF-8 data, then assert
 /// that it does not contain `expected`.
 #[track_caller]
 pub fn invalid_utf8_not_contains<P: AsRef<Path>, S: AsRef<str>>(path: P, expected: S) {
-    let buffer = fs_wrapper::read(path.as_ref());
+    let buffer = rfs::read(path.as_ref());
     let expected = expected.as_ref();
     if String::from_utf8_lossy(&buffer).contains(expected) {
         eprintln!("=== FILE CONTENTS (LOSSY) ===");
@@ -145,14 +144,14 @@ pub fn assert_not_contains<H: AsRef<str>, N: AsRef<str>>(haystack: H, needle: N)
 /// Assert that all files in `dir1` exist and have the same content in `dir2`
 pub fn assert_recursive_eq(dir1: impl AsRef<Path>, dir2: impl AsRef<Path>) {
     let dir2 = dir2.as_ref();
-    fs_helpers::read_dir(dir1, |entry_path| {
+    rfs::read_dir_entries(dir1, |entry_path| {
         let entry_name = entry_path.file_name().unwrap();
         if entry_path.is_dir() {
             assert_recursive_eq(&entry_path, &dir2.join(entry_name));
         } else {
             let path2 = dir2.join(entry_name);
-            let file1 = fs_wrapper::read(&entry_path);
-            let file2 = fs_wrapper::read(&path2);
+            let file1 = rfs::read(&entry_path);
+            let file2 = rfs::read(&path2);
 
             // We don't use `assert_eq!` because they are `Vec<u8>`, so not great for display.
             // Why not using String? Because there might be minified files or even potentially

src/tools/run-make-support/src/diff/mod.rs

@@ -2,7 +2,7 @@
 use similar::TextDiff;
 use std::path::{Path, PathBuf};
 
-use crate::fs_wrapper;
+use crate::fs as rfs;
 use build_helper::drop_bomb::DropBomb;
 
 #[cfg(test)]
@@ -43,7 +43,7 @@ pub fn new() -> Self {
     /// Specify the expected output for the diff from a file.
     pub fn expected_file<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
         let path = path.as_ref();
-        let content = fs_wrapper::read_to_string(path);
+        let content = rfs::read_to_string(path);
         let name = path.to_string_lossy().to_string();
 
         self.expected_file = Some(path.into());
@@ -62,7 +62,7 @@ pub fn expected_text<T: AsRef<[u8]>>(&mut self, name: &str, text: T) -> &mut Sel
     /// Specify the actual output for the diff from a file.
     pub fn actual_file<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
         let path = path.as_ref();
-        let content = fs_wrapper::read_to_string(path);
+        let content = rfs::read_to_string(path);
         let name = path.to_string_lossy().to_string();
 
         self.actual = Some(content);
@@ -116,7 +116,7 @@ pub fn run(&mut self) {
             if let Some(ref expected_file) = self.expected_file {
                 if std::env::var("RUSTC_BLESS_TEST").is_ok() {
                     println!("Blessing `{}`", expected_file.display());
-                    fs_wrapper::write(expected_file, actual);
+                    rfs::write(expected_file, actual);
                     return;
                 }
             }
@@ -138,7 +138,7 @@ pub fn run_fail(&mut self) {
             if let Some(ref expected_file) = self.expected_file {
                 if std::env::var("RUSTC_BLESS_TEST").is_ok() {
                     println!("Blessing `{}`", expected_file.display());
-                    fs_wrapper::write(expected_file, actual);
+                    rfs::write(expected_file, actual);
                     return;
                 }
             }

src/tools/run-make-support/src/fs.rs

@@ -1,17 +1,82 @@
-use std::fs;
+use std::io;
 use std::path::Path;
 
+// FIXME(jieyouxu): modify create_symlink to panic on windows.
+
+/// Creates a new symlink to a path on the filesystem, adjusting for Windows or Unix.
+#[cfg(target_family = "windows")]
+pub fn create_symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) {
+    if link.as_ref().exists() {
+        std::fs::remove_dir(link.as_ref()).unwrap();
+    }
+    std::os::windows::fs::symlink_file(original.as_ref(), link.as_ref()).expect(&format!(
+        "failed to create symlink {:?} for {:?}",
+        link.as_ref().display(),
+        original.as_ref().display(),
+    ));
+}
+
+/// Creates a new symlink to a path on the filesystem, adjusting for Windows or Unix.
+#[cfg(target_family = "unix")]
+pub fn create_symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) {
+    if link.as_ref().exists() {
+        std::fs::remove_dir(link.as_ref()).unwrap();
+    }
+    std::os::unix::fs::symlink(original.as_ref(), link.as_ref()).expect(&format!(
+        "failed to create symlink {:?} for {:?}",
+        link.as_ref().display(),
+        original.as_ref().display(),
+    ));
+}
+
+/// Copy a directory into another.
+pub fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) {
+    fn copy_dir_all_inner(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> io::Result<()> {
+        let dst = dst.as_ref();
+        if !dst.is_dir() {
+            std::fs::create_dir_all(&dst)?;
+        }
+        for entry in std::fs::read_dir(src)? {
+            let entry = entry?;
+            let ty = entry.file_type()?;
+            if ty.is_dir() {
+                copy_dir_all_inner(entry.path(), dst.join(entry.file_name()))?;
+            } else {
+                std::fs::copy(entry.path(), dst.join(entry.file_name()))?;
+            }
+        }
+        Ok(())
+    }
+
+    if let Err(e) = copy_dir_all_inner(&src, &dst) {
+        // Trying to give more context about what exactly caused the failure
+        panic!(
+            "failed to copy `{}` to `{}`: {:?}",
+            src.as_ref().display(),
+            dst.as_ref().display(),
+            e
+        );
+    }
+}
+
+/// Helper for reading entries in a given directory.
+pub fn read_dir_entries<P: AsRef<Path>, F: FnMut(&Path)>(dir: P, mut callback: F) {
+    for entry in read_dir(dir) {
+        callback(&entry.unwrap().path());
+    }
+}
+
 /// A wrapper around [`std::fs::remove_file`] which includes the file path in the panic message.
 #[track_caller]
 pub fn remove_file<P: AsRef<Path>>(path: P) {
-    fs::remove_file(path.as_ref())
+    std::fs::remove_file(path.as_ref())
         .expect(&format!("the file in path \"{}\" could not be removed", path.as_ref().display()));
 }
 
 /// A wrapper around [`std::fs::copy`] which includes the file path in the panic message.
 #[track_caller]
 pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) {
-    fs::copy(from.as_ref(), to.as_ref()).expect(&format!(
+    std::fs::copy(from.as_ref(), to.as_ref()).expect(&format!(
         "the file \"{}\" could not be copied over to \"{}\"",
         from.as_ref().display(),
         to.as_ref().display(),
@@ -21,21 +86,21 @@ pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) {
 /// A wrapper around [`std::fs::File::create`] which includes the file path in the panic message.
 #[track_caller]
 pub fn create_file<P: AsRef<Path>>(path: P) {
-    fs::File::create(path.as_ref())
+    std::fs::File::create(path.as_ref())
         .expect(&format!("the file in path \"{}\" could not be created", path.as_ref().display()));
 }
 
 /// A wrapper around [`std::fs::read`] which includes the file path in the panic message.
 #[track_caller]
 pub fn read<P: AsRef<Path>>(path: P) -> Vec<u8> {
-    fs::read(path.as_ref())
+    std::fs::read(path.as_ref())
         .expect(&format!("the file in path \"{}\" could not be read", path.as_ref().display()))
 }
 
 /// A wrapper around [`std::fs::read_to_string`] which includes the file path in the panic message.
 #[track_caller]
 pub fn read_to_string<P: AsRef<Path>>(path: P) -> String {
-    fs::read_to_string(path.as_ref()).expect(&format!(
+    std::fs::read_to_string(path.as_ref()).expect(&format!(
         "the file in path \"{}\" could not be read into a String",
         path.as_ref().display()
     ))
@@ -43,15 +108,15 @@ pub fn read_to_string<P: AsRef<Path>>(path: P) -> String {
 
 /// A wrapper around [`std::fs::read_dir`] which includes the file path in the panic message.
 #[track_caller]
-pub fn read_dir<P: AsRef<Path>>(path: P) -> fs::ReadDir {
-    fs::read_dir(path.as_ref())
+pub fn read_dir<P: AsRef<Path>>(path: P) -> std::fs::ReadDir {
+    std::fs::read_dir(path.as_ref())
         .expect(&format!("the directory in path \"{}\" could not be read", path.as_ref().display()))
 }
 
 /// A wrapper around [`std::fs::write`] which includes the file path in the panic message.
 #[track_caller]
 pub fn write<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) {
-    fs::write(path.as_ref(), contents.as_ref()).expect(&format!(
+    std::fs::write(path.as_ref(), contents.as_ref()).expect(&format!(
         "the file in path \"{}\" could not be written to",
         path.as_ref().display()
     ));
@@ -60,7 +125,7 @@ pub fn write<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) {
 /// A wrapper around [`std::fs::remove_dir_all`] which includes the file path in the panic message.
 #[track_caller]
 pub fn remove_dir_all<P: AsRef<Path>>(path: P) {
-    fs::remove_dir_all(path.as_ref()).expect(&format!(
+    std::fs::remove_dir_all(path.as_ref()).expect(&format!(
         "the directory in path \"{}\" could not be removed alongside all its contents",
         path.as_ref().display(),
     ));
@@ -69,7 +134,7 @@ pub fn remove_dir_all<P: AsRef<Path>>(path: P) {
 /// A wrapper around [`std::fs::create_dir`] which includes the file path in the panic message.
 #[track_caller]
 pub fn create_dir<P: AsRef<Path>>(path: P) {
-    fs::create_dir(path.as_ref()).expect(&format!(
+    std::fs::create_dir(path.as_ref()).expect(&format!(
         "the directory in path \"{}\" could not be created",
         path.as_ref().display()
     ));
@@ -78,7 +143,7 @@ pub fn create_dir<P: AsRef<Path>>(path: P) {
 /// A wrapper around [`std::fs::create_dir_all`] which includes the file path in the panic message.
 #[track_caller]
 pub fn create_dir_all<P: AsRef<Path>>(path: P) {
-    fs::create_dir_all(path.as_ref()).expect(&format!(
+    std::fs::create_dir_all(path.as_ref()).expect(&format!(
         "the directory (and all its parents) in path \"{}\" could not be created",
         path.as_ref().display()
     ));
@@ -86,8 +151,8 @@ pub fn create_dir_all<P: AsRef<Path>>(path: P) {
 
 /// A wrapper around [`std::fs::metadata`] which includes the file path in the panic message.
 #[track_caller]
-pub fn metadata<P: AsRef<Path>>(path: P) -> fs::Metadata {
-    fs::metadata(path.as_ref()).expect(&format!(
+pub fn metadata<P: AsRef<Path>>(path: P) -> std::fs::Metadata {
+    std::fs::metadata(path.as_ref()).expect(&format!(
         "the file's metadata in path \"{}\" could not be read",
         path.as_ref().display()
     ))
@@ -96,7 +161,7 @@ pub fn metadata<P: AsRef<Path>>(path: P) -> fs::Metadata {
 /// A wrapper around [`std::fs::rename`] which includes the file path in the panic message.
 #[track_caller]
 pub fn rename<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) {
-    fs::rename(from.as_ref(), to.as_ref()).expect(&format!(
+    std::fs::rename(from.as_ref(), to.as_ref()).expect(&format!(
         "the file \"{}\" could not be moved over to \"{}\"",
         from.as_ref().display(),
         to.as_ref().display(),
@@ -105,8 +170,8 @@ pub fn rename<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) {
 
 /// A wrapper around [`std::fs::set_permissions`] which includes the file path in the panic message.
 #[track_caller]
-pub fn set_permissions<P: AsRef<Path>>(path: P, perm: fs::Permissions) {
-    fs::set_permissions(path.as_ref(), perm).expect(&format!(
+pub fn set_permissions<P: AsRef<Path>>(path: P, perm: std::fs::Permissions) {
+    std::fs::set_permissions(path.as_ref(), perm).expect(&format!(
         "the file's permissions in path \"{}\" could not be changed",
         path.as_ref().display()
     ));

src/tools/run-make-support/src/fs_helpers.rs

@@ -1,71 +0,0 @@
-use std::io;
-use std::path::Path;
-
-use crate::fs_wrapper;
-
-// FIXME(jieyouxu): modify create_symlink to panic on windows.
-
-/// Creates a new symlink to a path on the filesystem, adjusting for Windows or Unix.
-#[cfg(target_family = "windows")]
-pub fn create_symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) {
-    if link.as_ref().exists() {
-        std::fs::remove_dir(link.as_ref()).unwrap();
-    }
-    use std::os::windows::fs;
-    fs::symlink_file(original.as_ref(), link.as_ref()).expect(&format!(
-        "failed to create symlink {:?} for {:?}",
-        link.as_ref().display(),
-        original.as_ref().display(),
-    ));
-}
-
-/// Creates a new symlink to a path on the filesystem, adjusting for Windows or Unix.
-#[cfg(target_family = "unix")]
-pub fn create_symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) {
-    if link.as_ref().exists() {
-        std::fs::remove_dir(link.as_ref()).unwrap();
-    }
-    use std::os::unix::fs;
-    fs::symlink(original.as_ref(), link.as_ref()).expect(&format!(
-        "failed to create symlink {:?} for {:?}",
-        link.as_ref().display(),
-        original.as_ref().display(),
-    ));
-}
-
-/// Copy a directory into another.
-pub fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) {
-    fn copy_dir_all_inner(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> io::Result<()> {
-        let dst = dst.as_ref();
-        if !dst.is_dir() {
-            std::fs::create_dir_all(&dst)?;
-        }
-        for entry in std::fs::read_dir(src)? {
-            let entry = entry?;
-            let ty = entry.file_type()?;
-            if ty.is_dir() {
-                copy_dir_all_inner(entry.path(), dst.join(entry.file_name()))?;
-            } else {
-                std::fs::copy(entry.path(), dst.join(entry.file_name()))?;
-            }
-        }
-        Ok(())
-    }
-
-    if let Err(e) = copy_dir_all_inner(&src, &dst) {
-        // Trying to give more context about what exactly caused the failure
-        panic!(
-            "failed to copy `{}` to `{}`: {:?}",
-            src.as_ref().display(),
-            dst.as_ref().display(),
-            e
-        );
-    }
-}
-
-/// Helper for reading entries in a given directory.
-pub fn read_dir<P: AsRef<Path>, F: FnMut(&Path)>(dir: P, mut callback: F) {
-    for entry in fs_wrapper::read_dir(dir) {
-        callback(&entry.unwrap().path());
-    }
-}

src/tools/run-make-support/src/lib.rs

@@ -12,8 +12,7 @@
 pub mod diff;
 pub mod env;
 pub mod external_deps;
-pub mod fs_helpers;
-pub mod fs_wrapper;
+pub mod fs;
 pub mod path_helpers;
 pub mod run;
 pub mod scoped_run;
@@ -64,9 +63,6 @@
 /// Path-related helpers.
 pub use path_helpers::{cwd, path, source_root};
 
-/// Helpers for common fs operations.
-pub use fs_helpers::{copy_dir_all, create_symlink, read_dir};
-
 /// Helpers for scoped test execution where certain properties are attempted to be maintained.
 pub use scoped_run::{run_in_tmpdir, test_while_readonly};
 

src/tools/run-make-support/src/scoped_run.rs

@@ -1,10 +1,8 @@
 //! Collection of helpers that try to maintain certain properties while running a test closure.
 
-use std::fs;
 use std::path::Path;
 
-use crate::fs_helpers::copy_dir_all;
-use crate::fs_wrapper;
+use crate::fs as rfs;
 use crate::path_helpers::cwd;
 use crate::targets::is_windows;
 
@@ -36,16 +34,16 @@ pub fn test_while_readonly<P, F>(path: P, closure: F)
         );
         panic!("`test_while_readonly` on directory detected while on Windows.");
     }
-    let metadata = fs_wrapper::metadata(&path);
+    let metadata = rfs::metadata(&path);
     let original_perms = metadata.permissions();
 
     let mut new_perms = original_perms.clone();
     new_perms.set_readonly(true);
-    fs_wrapper::set_permissions(&path, new_perms);
+    rfs::set_permissions(&path, new_perms);
 
     let success = std::panic::catch_unwind(closure);
 
-    fs_wrapper::set_permissions(&path, original_perms);
+    rfs::set_permissions(&path, original_perms);
     success.unwrap();
 }
 
@@ -62,10 +60,10 @@ pub fn test_while_readonly<P, F>(path: P, closure: F)
 pub fn run_in_tmpdir<F: FnOnce()>(callback: F) {
     let original_dir = cwd();
     let tmpdir = original_dir.join("../temporary-directory");
-    copy_dir_all(".", &tmpdir);
+    rfs::copy_dir_all(".", &tmpdir);
 
     std::env::set_current_dir(&tmpdir).unwrap();
     callback();
     std::env::set_current_dir(original_dir).unwrap();
-    fs::remove_dir_all(tmpdir).unwrap();
+    rfs::remove_dir_all(tmpdir);
 }