diff --git a/src/bootstrap/src/core/build_steps/clean.rs b/src/bootstrap/src/core/build_steps/clean.rs index f608e5d715e49..896033a258d9a 100644 --- a/src/bootstrap/src/core/build_steps/clean.rs +++ b/src/bootstrap/src/core/build_steps/clean.rs @@ -6,13 +6,15 @@ //! directory unless the `--all` flag is present. use std::fs; -use std::io::{self, ErrorKind}; use std::path::Path; use crate::core::builder::{crate_description, Builder, RunConfig, ShouldRun, Step}; use crate::utils::helpers::t; use crate::{Build, Compiler, Kind, Mode, Subcommand}; +#[cfg(test)] +mod tests; + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct CleanAll {} @@ -101,11 +103,11 @@ fn clean(build: &Build, all: bool, stage: Option) { return; } - rm_rf("tmp".as_ref()); + remove_dir_all_wrapper("tmp"); // Clean the entire build directory if all { - rm_rf(&build.out); + remove_dir_all_wrapper(&build.out); return; } @@ -136,17 +138,17 @@ fn clean_specific_stage(build: &Build, stage: u32) { } let path = t!(entry.path().canonicalize()); - rm_rf(&path); + remove_dir_all_wrapper(&path); } } } fn clean_default(build: &Build) { - rm_rf(&build.out.join("tmp")); - rm_rf(&build.out.join("dist")); - rm_rf(&build.out.join("bootstrap").join(".last-warned-change-id")); - rm_rf(&build.out.join("bootstrap-shims-dump")); - rm_rf(&build.out.join("rustfmt.stamp")); + remove_dir_all_wrapper(&build.out.join("tmp")); + remove_dir_all_wrapper(&build.out.join("dist")); + remove_dir_all_wrapper(&build.out.join("bootstrap").join(".last-warned-change-id")); + remove_dir_all_wrapper(&build.out.join("bootstrap-shims-dump")); + remove_dir_all_wrapper(&build.out.join("rustfmt.stamp")); let mut hosts: Vec<_> = build.hosts.iter().map(|t| build.out.join(t)).collect(); // After cross-compilation, artifacts of the host architecture (which may differ from build.host) @@ -166,78 +168,16 @@ fn clean_default(build: &Build) { continue; } let path = t!(entry.path().canonicalize()); - rm_rf(&path); + remove_dir_all_wrapper(&path); } } } -fn rm_rf(path: &Path) { - match path.symlink_metadata() { - Err(e) => { - if e.kind() == ErrorKind::NotFound { - return; - } - panic!("failed to get metadata for file {}: {}", path.display(), e); - } - Ok(metadata) => { - if metadata.file_type().is_file() || metadata.file_type().is_symlink() { - do_op(path, "remove file", |p| match fs::remove_file(p) { - #[cfg(windows)] - Err(e) - if e.kind() == std::io::ErrorKind::PermissionDenied - && p.file_name().and_then(std::ffi::OsStr::to_str) - == Some("bootstrap.exe") => - { - eprintln!("WARNING: failed to delete '{}'.", p.display()); - Ok(()) - } - r => r, - }); - - return; - } - - for file in t!(fs::read_dir(path)) { - rm_rf(&t!(file).path()); - } - - do_op(path, "remove dir", |p| match fs::remove_dir(p) { - // Check for dir not empty on Windows - // FIXME: Once `ErrorKind::DirectoryNotEmpty` is stabilized, - // match on `e.kind()` instead. - #[cfg(windows)] - Err(e) if e.raw_os_error() == Some(145) => Ok(()), - r => r, - }); - } - }; -} - -fn do_op(path: &Path, desc: &str, mut f: F) -where - F: FnMut(&Path) -> io::Result<()>, -{ - match f(path) { - Ok(()) => {} - // On windows we can't remove a readonly file, and git will often clone files as readonly. - // As a result, we have some special logic to remove readonly files on windows. - // This is also the reason that we can't use things like fs::remove_dir_all(). - #[cfg(windows)] - Err(ref e) if e.kind() == ErrorKind::PermissionDenied => { - let m = t!(path.symlink_metadata()); - let mut p = m.permissions(); - p.set_readonly(false); - t!(fs::set_permissions(path, p)); - f(path).unwrap_or_else(|e| { - // Delete symlinked directories on Windows - if m.file_type().is_symlink() && path.is_dir() && fs::remove_dir(path).is_ok() { - return; - } - panic!("failed to {} {}: {}", desc, path.display(), e); - }); - } - Err(e) => { - panic!("failed to {} {}: {}", desc, path.display(), e); - } +/// Wrapper for [`std::fs::remove_dir_all`] that panics on failure and prints the `path` we failed +/// on. +fn remove_dir_all_wrapper>(path: P) { + let path = path.as_ref(); + if let Err(e) = fs::remove_dir_all(&path) { + panic!("failed to `remove_dir_all` at `{}`: {e}", path.display()); } } diff --git a/src/bootstrap/src/core/build_steps/clean/tests.rs b/src/bootstrap/src/core/build_steps/clean/tests.rs new file mode 100644 index 0000000000000..dcb209cbd9514 --- /dev/null +++ b/src/bootstrap/src/core/build_steps/clean/tests.rs @@ -0,0 +1,401 @@ +// This place is not a place of honor... no highly esteemed deed is commemorated here... nothing +// valued is here. +// +// What is here was dangerous and repulsive to us. This message is a warning about danger. +// +// The danger is in a particular location... it increases towards a center... the center of danger +// is here... of a particular size and shape, and below us. +// +// The danger is still present, in your time, as it was in ours. +// +// (It turns out filesystems are *really* hard to get right. And different. On Windows versus Linux. +// And on different filesystems.) + +/// Expectation tests regarding [`std::fs::remove_dir_all`] for bootstrap clean implementations that +/// uses it. Read-only here does not mean immutable attribute. +/// +/// # TOCTOU for file deletion checks? +/// +/// Astute readers might be suspicious of potential TOCTOU races with checking if a file is deleted +/// after `remove_dir_all` immediately. And you should be. But here we rely on undocumented +/// implementation details of the stdlib for Windows that `remove_dir_all` attempts to use POSIX +/// deletion semantics where possible ("Files will be deleted as soon as the handle is closed. This +/// is supported for Windows 10 1607 (aka RS1) and later"). In individual tests, we make sure file +/// handles are dropped before calling `remove_dir_all` by explicitly calling `drop` on creation +/// functions that return file handles. +/// +/// See +/// . +/// +/// Remark: junctions and hard links were not considered at the time of writing. +mod std_fs_remove_dir_all_sanity_checks { + // make sure fs results are handled + #![deny(unused_must_use)] + + use std::path::{Path, PathBuf}; + use std::{fs, io}; + + use crate::{Config, Flags}; + + #[track_caller] + fn config() -> Config { + Config::parse(Flags::parse(&["check".to_owned(), "--config=/does/not/exist".to_owned()])) + } + + #[track_caller] + fn tempdir() -> PathBuf { + config().tempdir() + } + + // Yes... I know we're using the thing we're trying to test here, if we can't clear the temp dir + // then it's already problematic. + #[track_caller] + fn reset_tempdir() { + fs::remove_dir_all(tempdir()).unwrap() + } + + #[track_caller] + fn ensure_base_dir_exists() -> PathBuf { + let base_dir = config().tempdir().join("base_dir"); + fs::create_dir_all(&base_dir).unwrap(); + assert!(base_dir.exists()); + base_dir + } + + #[track_caller] + fn set_readonly(path: &Path) { + let mut perms = fs::metadata(path).unwrap().permissions(); + perms.set_readonly(true); + fs::set_permissions(path, perms).unwrap(); + } + + // --- Shallow base case, a little warmup --- + + #[test] + fn empty() { + reset_tempdir(); + let root = ensure_base_dir_exists(); + fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn empty_readonly() { + reset_tempdir(); + let root = ensure_base_dir_exists(); + set_readonly(&root); + fs::remove_dir_all(&root).unwrap(); + } + + #[test] + fn single_file_read_write() { + reset_tempdir(); + let root = ensure_base_dir_exists(); + let path = root.join("do_you_feel_cold_and_lost_in_desperation.rs"); + + drop(fs::File::create_new(&path).unwrap()); + assert!(path.exists()); + + fs::remove_dir_all(&root).unwrap(); + assert!(!path.exists()); + } + + #[test] + fn single_file_read_only() { + reset_tempdir(); + let root = ensure_base_dir_exists(); + let path = root.join("you_build_up_hope_but_failures_all_youve_known.rs"); + + drop(fs::File::create_new(&path).unwrap()); + set_readonly(&path); + assert!(path.exists()); + + fs::remove_dir_all(&root).unwrap(); + assert!(!path.exists()); + } + + #[test] + fn single_empty_directory_read_write() { + reset_tempdir(); + let root = ensure_base_dir_exists(); + let path = root.join("remember_all_the_sadness_and_frustration"); + + fs::create_dir(&path).unwrap(); + assert!(path.exists()); + + fs::remove_dir_all(&root).unwrap(); + assert!(!path.exists()); + } + + #[test] + fn single_empty_directory_read_only() { + reset_tempdir(); + let root = ensure_base_dir_exists(); + let path = root.join("and_let_it_go"); + + fs::create_dir(&path).unwrap(); + set_readonly(&path); + assert!(path.exists()); + + fs::remove_dir_all(&root).unwrap(); + assert!(!path.exists()); + } + + // Note: read-write here means for the symlink itself, not the target file/dir. + #[test] + fn single_symlink_read_write() { + reset_tempdir(); + let tmp = tempdir(); + + let root_symlink = tmp.join("root_symlink"); + fs::create_dir_all(&root_symlink).unwrap(); + assert!(root_symlink.exists()); + let root_dst = tmp.join("root_dst"); + fs::create_dir_all(&root_dst).unwrap(); + assert!(root_dst.exists()); + + // symlinks put under `root_symlink`. + let file_link_path = root_symlink.join("file_link.rs"); + let dir_link_path = root_symlink.join("dir_link"); + + // dest file/dir put under `root_dst`. + let dst_file_path = root_dst.join("and_in_a_burst_of_light_that_blinded_every_angel.rs"); + let dst_dir_path = root_dst.join("as_if_the_sky_had_blown_the_heavens_into_stars"); + + drop(fs::File::create_new(&dst_file_path).unwrap()); + assert!(dst_file_path.exists()); + fs::create_dir(&dst_dir_path).unwrap(); + assert!(dst_dir_path.exists()); + + #[cfg(unix)] + { + // Symlinks are special file types on Unix. + assert!(!file_link_path.exists()); + std::os::unix::fs::symlink(&dst_file_path, &file_link_path).unwrap(); + assert!(file_link_path.exists()); + + assert!(!dir_link_path.exists()); + std::os::unix::fs::symlink(&dst_dir_path, &dir_link_path).unwrap(); + assert!(dir_link_path.exists()); + + fs::remove_dir_all(root_symlink).unwrap(); + + // Make sure symlinks are deleted... + assert!(!file_link_path.exists()); + assert!(!dir_link_path.exists()); + // ... but not the target files (i.e. make sure `remove_dir_all` doesn't follow + // symlinks) + assert!(dst_file_path.exists()); + assert!(dst_dir_path.exists()); + } + #[cfg(windows)] + { + // Symlinks are particularly tricky on Windows because they are not special file types + // but instead has additional file attribute + metadata. To create symlinks you also + // need symlink creation priviledges. + + // Check if we have priviledge first: + assert!(!file_link_path.exists()); + match std::os::windows::fs::symlink_file(&dst_file_path, &file_link_path) { + Ok(()) => assert!(file_link_path.exists()), + Err(e) if e.kind() == io::ErrorKind::PermissionDenied => { + // No symlink priviledges, can't really test this. + return; + } + Err(e) => panic!("{e}"), + } + + assert!(!dir_link_path.exists()); + std::os::windows::fs::symlink_dir(&dst_dir_path, &dir_link_path).unwrap(); + assert!(dir_link_path.exists()); + + fs::remove_dir_all(root_symlink).unwrap(); + + // Make sure symlinks are deleted... + assert!(!file_link_path.exists()); + assert!(!dir_link_path.exists()); + // ... but not the target files (i.e. make sure `remove_dir_all` doesn't follow + // symlinks) + assert!(dst_file_path.exists()); + assert!(dst_dir_path.exists()); + } + } + + // Note: read-only here means for the symlink itself, not the target file/dir. It also does not + // mean immutable attributes. + #[test] + fn single_symlink_read_only() { + reset_tempdir(); + let tmp = tempdir(); + + let root_symlink = tmp.join("root_symlink"); + fs::create_dir_all(&root_symlink).unwrap(); + assert!(root_symlink.exists()); + let root_dst = tmp.join("root_dst"); + fs::create_dir_all(&root_dst).unwrap(); + assert!(root_dst.exists()); + + // symlinks put under `root_symlink`. + let file_link_path = root_symlink.join("file_link.rs"); + let dir_link_path = root_symlink.join("dir_link"); + + // dest file/dir put under `root_dst`. + let dst_file_path = root_dst.join("and_in_a_burst_of_light_that_blinded_every_angel.rs"); + let dst_dir_path = root_dst.join("as_if_the_sky_had_blown_the_heavens_into_stars"); + + fs::File::create_new(&dst_file_path).unwrap(); + assert!(dst_file_path.exists()); + fs::create_dir(&dst_dir_path).unwrap(); + assert!(dst_dir_path.exists()); + + #[cfg(unix)] + { + // Symlinks are special file types on Unix. + assert!(!file_link_path.exists()); + std::os::unix::fs::symlink(&dst_file_path, &file_link_path).unwrap(); + assert!(file_link_path.exists()); + set_readonly(&file_link_path); + + assert!(!dir_link_path.exists()); + std::os::unix::fs::symlink(&dst_dir_path, &dir_link_path).unwrap(); + assert!(dir_link_path.exists()); + set_readonly(&dir_link_path); + + fs::remove_dir_all(root_symlink).unwrap(); + + // Make sure symlinks are deleted... + assert!(!file_link_path.exists()); + assert!(!dir_link_path.exists()); + // ... but not the target file/dir (i.e. make sure `remove_dir_all` doesn't follow + // symlinks) + assert!(dst_file_path.exists()); + assert!(dst_dir_path.exists()); + } + #[cfg(windows)] + { + // Symlinks are particularly tricky on Windows because they are not special file types + // but instead has additional file attribute + metadata. To create symlinks you also + // need symlink creation priviledges. + + // Check if we have priviledge first: + assert!(!file_link_path.exists()); + match std::os::windows::fs::symlink_file(&dst_file_path, &file_link_path) { + Ok(()) => assert!(file_link_path.exists()), + Err(e) if e.kind() == io::ErrorKind::PermissionDenied => { + // No symlink priviledges, can't really test this. + return; + } + Err(e) => panic!("{e}"), + } + set_readonly(&file_link_path); + + assert!(!dir_link_path.exists()); + std::os::windows::fs::symlink_dir(&dst_dir_path, &dir_link_path).unwrap(); + assert!(dir_link_path.exists()); + set_readonly(&dir_link_path); + + fs::remove_dir_all(root_symlink).unwrap(); + + // Make sure symlinks are deleted... + assert!(!file_link_path.exists()); + assert!(!dir_link_path.exists()); + // ... but not the target file/dir (i.e. make sure `remove_dir_all` doesn't follow + // symlinks) + assert!(dst_file_path.exists()); + assert!(dst_dir_path.exists()); + } + } + + // --- Now we must go deeper, brave traveller! --- + + #[test] + fn remix() { + reset_tempdir(); + // Directory structure: + // ``` + // . + // ├── root_dst/ + // │   ├── dir/ + // │   └── file.rs + // └── root_symlink/ + // └── foo/ + // ├── a.rs + // └── bar/ + // ├── b.rs + // ├── symlink_to_dir + // └── symlink_to_file.rs + // ``` + + let tmp = tempdir(); + + let root_dst = tmp.join("root_dst"); + fs::create_dir_all(&root_dst).unwrap(); + assert!(root_dst.exists()); + + let dst_file = root_dst.join("file.rs"); + drop(fs::File::create_new(&dst_file).unwrap()); + assert!(dst_file.exists()); + + let dst_dir = root_dst.join("dir"); + fs::create_dir_all(&dst_dir).unwrap(); + assert!(dst_dir.exists()); + + let root_symlink = tmp.join("root_symlink"); + fs::create_dir_all(&root_symlink).unwrap(); + assert!(root_symlink.exists()); + + let foo = root_symlink.join("foo"); + fs::create_dir_all(&foo).unwrap(); + assert!(foo.exists()); + + let a = foo.join("a.rs"); + drop(fs::File::create_new(&a).unwrap()); + assert!(a.exists()); + + let bar = foo.join("bar"); + fs::create_dir_all(&bar).unwrap(); + assert!(bar.exists()); + + let b = foo.join("b.rs"); + drop(fs::File::create_new(&b).unwrap()); + assert!(b.exists()); + + set_readonly(&b); + set_readonly(&a); + + let symlink_to_dir = bar.join("symlink_to_dir"); + let symlink_to_file = bar.join("symlink_to_file.rs"); + + #[cfg(windows)] + { + std::os::windows::fs::symlink_dir(&dst_dir, &symlink_to_dir).unwrap(); + assert!(symlink_to_dir.exists()); + std::os::windows::fs::symlink_file(&dst_file, &symlink_to_file).unwrap(); + assert!(symlink_to_file.exists()); + } + #[cfg(unix)] + { + std::os::unix::fs::symlink(&dst_dir, &symlink_to_dir).unwrap(); + assert!(symlink_to_dir.exists()); + std::os::unix::fs::symlink(&dst_file, &symlink_to_file).unwrap(); + assert!(symlink_to_file.exists()); + } + #[cfg(not(any(windows, unix)))] + { + return; + } + + set_readonly(&bar); + set_readonly(&foo); + set_readonly(&root_symlink); + + fs::remove_dir_all(&root_symlink).unwrap(); + + // Make sure symlinks are deleted... + assert!(!symlink_to_dir.exists()); + assert!(!symlink_to_file.exists()); + // ... but not the target file/dir (i.e. make sure `remove_dir_all` doesn't follow + // symlinks) + assert!(dst_dir.exists()); + assert!(dst_file.exists()); + } +} diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs index 378d069672f5d..0ea30ffc6362f 100644 --- a/src/bootstrap/src/core/config/tests.rs +++ b/src/bootstrap/src/core/config/tests.rs @@ -66,6 +66,9 @@ fn detect_src_and_out() { // test if build-dir was manually given in config.toml if let Some(custom_build_dir) = build_dir { + // FIXME(jieyouxu): this assertion fails on native Windows because if the rust checkout + // and build directory is on `E:\\` not `C:\\`, then it fails with e.g. `E:\\tmp != + // C:\\tmp`. assert_eq!(&cfg.out, Path::new(custom_build_dir)); } // test the native bootstrap way