diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/build_helper/src/fs/mod.rs | 69 | ||||
| -rw-r--r-- | src/build_helper/src/fs/tests.rs | 214 | ||||
| -rw-r--r-- | src/build_helper/src/lib.rs | 1 | ||||
| -rw-r--r-- | src/tools/compiletest/Cargo.toml | 2 | ||||
| -rw-r--r-- | src/tools/compiletest/src/runtest.rs | 23 | ||||
| -rw-r--r-- | src/tools/compiletest/src/runtest/run_make.rs | 12 | ||||
| -rw-r--r-- | src/tools/run-make-support/src/fs.rs | 87 |
7 files changed, 352 insertions, 56 deletions
diff --git a/src/build_helper/src/fs/mod.rs b/src/build_helper/src/fs/mod.rs new file mode 100644 index 00000000000..02029846fd1 --- /dev/null +++ b/src/build_helper/src/fs/mod.rs @@ -0,0 +1,69 @@ +//! Misc filesystem related helpers for use by bootstrap and tools. +use std::fs::Metadata; +use std::path::Path; +use std::{fs, io}; + +#[cfg(test)] +mod tests; + +/// Helper to ignore [`std::io::ErrorKind::NotFound`], but still propagate other +/// [`std::io::ErrorKind`]s. +pub fn ignore_not_found<Op>(mut op: Op) -> io::Result<()> +where + Op: FnMut() -> io::Result<()>, +{ + match op() { + Ok(()) => Ok(()), + Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(e), + } +} + +/// 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. +/// - 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. +#[track_caller] +pub fn recursive_remove<P: AsRef<Path>>(path: P) -> io::Result<()> { + let path = path.as_ref(); + let metadata = fs::symlink_metadata(path)?; + #[cfg(windows)] + let is_dir_like = |meta: &fs::Metadata| { + use std::os::windows::fs::FileTypeExt; + meta.is_dir() || meta.file_type().is_symlink_dir() + }; + #[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) + } +} + +fn try_remove_op_set_perms<'p, Op>(mut op: Op, path: &'p Path, metadata: Metadata) -> io::Result<()> +where + Op: FnMut(&'p Path) -> io::Result<()>, +{ + match op(path) { + Ok(()) => Ok(()), + Err(e) if e.kind() == io::ErrorKind::PermissionDenied => { + let mut perms = metadata.permissions(); + perms.set_readonly(false); + fs::set_permissions(path, perms)?; + op(path) + } + Err(e) => Err(e), + } +} diff --git a/src/build_helper/src/fs/tests.rs b/src/build_helper/src/fs/tests.rs new file mode 100644 index 00000000000..1e694393127 --- /dev/null +++ b/src/build_helper/src/fs/tests.rs @@ -0,0 +1,214 @@ +#![deny(unused_must_use)] + +use std::{env, fs, io}; + +use super::recursive_remove; + +mod recursive_remove_tests { + use super::*; + + // Basic cases + + #[test] + fn nonexistent_path() { + 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)); + } + + #[test] + fn file() { + let tmpdir = env::temp_dir(); + let path = tmpdir.join("__INTERNAL_BOOTSTRAP_file"); + fs::write(&path, b"").unwrap(); + assert!(fs::symlink_metadata(&path).is_ok()); + assert!(recursive_remove(&path).is_ok()); + assert!(fs::symlink_metadata(&path).is_err_and(|e| e.kind() == io::ErrorKind::NotFound)); + } + + mod dir_tests { + use super::*; + + #[test] + fn dir_empty() { + let tmpdir = env::temp_dir(); + let path = tmpdir.join("__INTERNAL_BOOTSTRAP_dir_tests_dir_empty"); + fs::create_dir_all(&path).unwrap(); + assert!(fs::symlink_metadata(&path).is_ok()); + assert!(recursive_remove(&path).is_ok()); + assert!( + fs::symlink_metadata(&path).is_err_and(|e| e.kind() == io::ErrorKind::NotFound) + ); + } + + #[test] + fn dir_recursive() { + let tmpdir = env::temp_dir(); + let path = tmpdir.join("__INTERNAL_BOOTSTRAP_dir_tests_dir_recursive"); + fs::create_dir_all(&path).unwrap(); + assert!(fs::symlink_metadata(&path).is_ok()); + + let file_a = path.join("a.txt"); + fs::write(&file_a, b"").unwrap(); + assert!(fs::symlink_metadata(&file_a).is_ok()); + + let dir_b = path.join("b"); + fs::create_dir_all(&dir_b).unwrap(); + assert!(fs::symlink_metadata(&dir_b).is_ok()); + + let file_c = dir_b.join("c.rs"); + fs::write(&file_c, b"").unwrap(); + assert!(fs::symlink_metadata(&file_c).is_ok()); + + assert!(recursive_remove(&path).is_ok()); + + assert!( + fs::symlink_metadata(&file_a).is_err_and(|e| e.kind() == io::ErrorKind::NotFound) + ); + assert!( + fs::symlink_metadata(&dir_b).is_err_and(|e| e.kind() == io::ErrorKind::NotFound) + ); + assert!( + fs::symlink_metadata(&file_c).is_err_and(|e| e.kind() == io::ErrorKind::NotFound) + ); + } + } + + /// Check that [`recursive_remove`] does not traverse symlinks and only removes symlinks + /// themselves. + /// + /// Symlink-to-file versus symlink-to-dir is a distinction that's important on Windows, but not + /// on Unix. + mod symlink_tests { + use super::*; + + #[cfg(unix)] + #[test] + fn unix_symlink() { + let tmpdir = env::temp_dir(); + let path = tmpdir.join("__INTERNAL_BOOTSTRAP_symlink_tests_unix_symlink"); + let symlink_path = + tmpdir.join("__INTERNAL_BOOTSTRAP__symlink_tests_unix_symlink_symlink"); + fs::write(&path, b"").unwrap(); + + assert!(fs::symlink_metadata(&path).is_ok()); + assert!( + fs::symlink_metadata(&symlink_path) + .is_err_and(|e| e.kind() == io::ErrorKind::NotFound) + ); + + std::os::unix::fs::symlink(&path, &symlink_path).unwrap(); + + assert!(recursive_remove(&symlink_path).is_ok()); + + // Check that the symlink got removed... + assert!( + fs::symlink_metadata(&symlink_path) + .is_err_and(|e| e.kind() == io::ErrorKind::NotFound) + ); + // ... but pointed-to file still exists. + assert!(fs::symlink_metadata(&path).is_ok()); + + fs::remove_file(&path).unwrap(); + } + + #[cfg(windows)] + #[test] + fn windows_symlink_to_file() { + let tmpdir = env::temp_dir(); + let path = tmpdir.join("__INTERNAL_BOOTSTRAP_symlink_tests_windows_symlink_to_file"); + let symlink_path = tmpdir + .join("__INTERNAL_BOOTSTRAP_SYMLINK_symlink_tests_windows_symlink_to_file_symlink"); + fs::write(&path, b"").unwrap(); + + assert!(fs::symlink_metadata(&path).is_ok()); + assert!( + fs::symlink_metadata(&symlink_path) + .is_err_and(|e| e.kind() == io::ErrorKind::NotFound) + ); + + std::os::windows::fs::symlink_file(&path, &symlink_path).unwrap(); + + assert!(recursive_remove(&symlink_path).is_ok()); + + // Check that the symlink-to-file got removed... + assert!( + fs::symlink_metadata(&symlink_path) + .is_err_and(|e| e.kind() == io::ErrorKind::NotFound) + ); + // ... but pointed-to file still exists. + assert!(fs::symlink_metadata(&path).is_ok()); + + fs::remove_file(&path).unwrap(); + } + + #[cfg(windows)] + #[test] + fn windows_symlink_to_dir() { + let tmpdir = env::temp_dir(); + let path = tmpdir.join("__INTERNAL_BOOTSTRAP_symlink_tests_windows_symlink_to_dir"); + let symlink_path = + tmpdir.join("__INTERNAL_BOOTSTRAP_symlink_tests_windows_symlink_to_dir_symlink"); + fs::create_dir_all(&path).unwrap(); + + assert!(fs::symlink_metadata(&path).is_ok()); + assert!( + fs::symlink_metadata(&symlink_path) + .is_err_and(|e| e.kind() == io::ErrorKind::NotFound) + ); + + std::os::windows::fs::symlink_dir(&path, &symlink_path).unwrap(); + + assert!(recursive_remove(&symlink_path).is_ok()); + + // Check that the symlink-to-dir got removed... + assert!( + fs::symlink_metadata(&symlink_path) + .is_err_and(|e| e.kind() == io::ErrorKind::NotFound) + ); + // ... but pointed-to dir still exists. + assert!(fs::symlink_metadata(&path).is_ok()); + + fs::remove_dir_all(&path).unwrap(); + } + } + + /// Read-only file and directories only need special handling on Windows. + #[cfg(windows)] + mod readonly_tests { + use super::*; + + #[test] + fn overrides_readonly() { + let tmpdir = env::temp_dir(); + let path = tmpdir.join("__INTERNAL_BOOTSTRAP_readonly_tests_overrides_readonly"); + + // In case of a previous failed test: + if let Ok(mut perms) = fs::symlink_metadata(&path).map(|m| m.permissions()) { + perms.set_readonly(false); + fs::set_permissions(&path, perms).unwrap(); + fs::remove_file(&path).unwrap(); + } + + fs::write(&path, b"").unwrap(); + + let mut perms = fs::symlink_metadata(&path).unwrap().permissions(); + perms.set_readonly(true); + fs::set_permissions(&path, perms).unwrap(); + + // Check that file exists but is read-only, and that normal `std::fs::remove_file` fails + // to delete the file. + assert!(fs::symlink_metadata(&path).is_ok_and(|m| m.permissions().readonly())); + assert!( + fs::remove_file(&path).is_err_and(|e| e.kind() == io::ErrorKind::PermissionDenied) + ); + + assert!(recursive_remove(&path).is_ok()); + + assert!( + fs::symlink_metadata(&path).is_err_and(|e| e.kind() == io::ErrorKind::NotFound) + ); + } + } +} diff --git a/src/build_helper/src/lib.rs b/src/build_helper/src/lib.rs index 4a4f0ca2a9d..dceb5fdeeea 100644 --- a/src/build_helper/src/lib.rs +++ b/src/build_helper/src/lib.rs @@ -2,6 +2,7 @@ pub mod ci; pub mod drop_bomb; +pub mod fs; pub mod git; pub mod metrics; pub mod stage0_parser; diff --git a/src/tools/compiletest/Cargo.toml b/src/tools/compiletest/Cargo.toml index b784bdb7139..16cc1d2a565 100644 --- a/src/tools/compiletest/Cargo.toml +++ b/src/tools/compiletest/Cargo.toml @@ -16,7 +16,7 @@ indexmap = "2.0.0" miropt-test-tools = { path = "../miropt-test-tools" } build_helper = { path = "../../build_helper" } tracing = "0.1" -tracing-subscriber = { version = "0.3.3", default-features = false, features = ["fmt", "env-filter", "smallvec", "parking_lot", "ansi"] } +tracing-subscriber = { version = "0.3.3", default-features = false, features = ["ansi", "env-filter", "fmt", "parking_lot", "smallvec"] } regex = "1.0" semver = { version = "1.0.23", features = ["serde"] } serde = { version = "1.0", features = ["derive"] } diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 6a4f0b96bb4..108fde1c899 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -2809,29 +2809,6 @@ impl<'test> TestCx<'test> { println!("init_incremental_test: incremental_dir={}", incremental_dir.display()); } } - - fn aggressive_rm_rf(&self, path: &Path) -> io::Result<()> { - for e in path.read_dir()? { - let entry = e?; - let path = entry.path(); - if entry.file_type()?.is_dir() { - self.aggressive_rm_rf(&path)?; - } else { - // Remove readonly files as well on windows (by default we can't) - fs::remove_file(&path).or_else(|e| { - if cfg!(windows) && e.kind() == io::ErrorKind::PermissionDenied { - let mut meta = entry.metadata()?.permissions(); - meta.set_readonly(false); - fs::set_permissions(&path, meta)?; - fs::remove_file(&path) - } else { - Err(e) - } - })?; - } - } - fs::remove_dir(path) - } } struct ProcArgs { diff --git a/src/tools/compiletest/src/runtest/run_make.rs b/src/tools/compiletest/src/runtest/run_make.rs index 85ade5b727a..ee7aed2a39c 100644 --- a/src/tools/compiletest/src/runtest/run_make.rs +++ b/src/tools/compiletest/src/runtest/run_make.rs @@ -2,6 +2,8 @@ use std::path::Path; use std::process::{Command, Output, Stdio}; use std::{env, fs}; +use build_helper::fs::{ignore_not_found, recursive_remove}; + use super::{ProcRes, TestCx, disable_error_reporting}; use crate::util::{copy_dir_all, dylib_env_var}; @@ -27,9 +29,8 @@ impl TestCx<'_> { // are hopefully going away, it seems safer to leave this perilous code // as-is until it can all be deleted. let tmpdir = cwd.join(self.output_base_name()); - if tmpdir.exists() { - self.aggressive_rm_rf(&tmpdir).unwrap(); - } + ignore_not_found(|| recursive_remove(&tmpdir)).unwrap(); + fs::create_dir_all(&tmpdir).unwrap(); let host = &self.config.host; @@ -218,9 +219,8 @@ impl TestCx<'_> { // // This setup intentionally diverges from legacy Makefile run-make tests. let base_dir = self.output_base_dir(); - if base_dir.exists() { - self.aggressive_rm_rf(&base_dir).unwrap(); - } + ignore_not_found(|| recursive_remove(&base_dir)).unwrap(); + let rmake_out_dir = base_dir.join("rmake_out"); fs::create_dir_all(&rmake_out_dir).unwrap(); diff --git a/src/tools/run-make-support/src/fs.rs b/src/tools/run-make-support/src/fs.rs index 2ca55ad3b3a..7ebe4a9ca13 100644 --- a/src/tools/run-make-support/src/fs.rs +++ b/src/tools/run-make-support/src/fs.rs @@ -1,7 +1,51 @@ +use std::fs::FileType; use std::io; use std::path::{Path, PathBuf}; -/// Copy a directory into another. +/// Given a symlink at `src`, read its target, then create a new symlink at `dst` also pointing to +/// target. +pub fn copy_symlink(src: impl AsRef<Path>, dst: impl AsRef<Path>) { + let src = src.as_ref(); + let dst = dst.as_ref(); + let metadata = symlink_metadata(src); + if let Err(e) = copy_symlink_raw(metadata.file_type(), src, dst) { + panic!("failed to copy symlink from `{}` to `{}`: {e}", src.display(), dst.display(),); + } +} + +fn copy_symlink_raw(ty: FileType, src: impl AsRef<Path>, dst: impl AsRef<Path>) -> io::Result<()> { + // Traverse symlink once to find path of target entity. + let target_path = std::fs::read_link(src)?; + + let new_symlink_path = dst.as_ref(); + #[cfg(windows)] + { + use std::os::windows::fs::FileTypeExt; + if ty.is_symlink_dir() { + std::os::windows::fs::symlink_dir(&target_path, new_symlink_path)?; + } else { + // Target may be a file or another symlink, in any case we can use + // `symlink_file` here. + std::os::windows::fs::symlink_file(&target_path, new_symlink_path)?; + } + } + #[cfg(unix)] + { + let _ = ty; + std::os::unix::fs::symlink(target_path, new_symlink_path)?; + } + #[cfg(not(any(windows, unix)))] + { + let _ = ty; + // Technically there's also wasi, but I have no clue about wasi symlink + // semantics and which wasi targets / environment support symlinks. + unimplemented!("unsupported target"); + } + Ok(()) +} + +/// Copy a directory into another. This will not traverse symlinks; instead, it will create new +/// symlinks pointing at target paths that symlinks in the original directory points to. 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(); @@ -14,31 +58,7 @@ pub fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) { if ty.is_dir() { copy_dir_all_inner(entry.path(), dst.join(entry.file_name()))?; } else if ty.is_symlink() { - // Traverse symlink once to find path of target entity. - let target_path = std::fs::read_link(entry.path())?; - - let new_symlink_path = dst.join(entry.file_name()); - #[cfg(windows)] - { - use std::os::windows::fs::FileTypeExt; - if ty.is_symlink_dir() { - std::os::windows::fs::symlink_dir(&target_path, new_symlink_path)?; - } else { - // Target may be a file or another symlink, in any case we can use - // `symlink_file` here. - std::os::windows::fs::symlink_file(&target_path, new_symlink_path)?; - } - } - #[cfg(unix)] - { - std::os::unix::fs::symlink(target_path, new_symlink_path)?; - } - #[cfg(not(any(windows, unix)))] - { - // Technically there's also wasi, but I have no clue about wasi symlink - // semantics and which wasi targets / environment support symlinks. - unimplemented!("unsupported target"); - } + copy_symlink_raw(ty, entry.path(), dst.join(entry.file_name()))?; } else { std::fs::copy(entry.path(), dst.join(entry.file_name()))?; } @@ -64,6 +84,21 @@ pub fn read_dir_entries<P: AsRef<Path>, F: FnMut(&Path)>(dir: P, mut callback: F } } +/// A wrapper around [`build_helper::fs::recursive_remove`] which includes the file path in the +/// panic message. +/// +/// This handles removing symlinks on Windows (e.g. symlink-to-file will be removed via +/// [`std::fs::remove_file`] while symlink-to-dir will be removed via [`std::fs::remove_dir`]). +#[track_caller] +pub fn recursive_remove<P: AsRef<Path>>(path: P) { + if let Err(e) = build_helper::fs::recursive_remove(path.as_ref()) { + panic!( + "failed to recursive remove filesystem entities at `{}`: {e}", + path.as_ref().display() + ); + } +} + /// 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) { |