Skip to content

Commit

Permalink
Auto merge of rust-lang#129413 - jieyouxu:revert-remove-dir-all, r=co…
Browse files Browse the repository at this point in the history
…mpiler-errors

Revert rust-lang#129187 and rust-lang#129302

The two PRs naively switched to `std::fs::remove_dir_all`, but failed to gracefully handle the failure case where the top-level directory entry does not exist, causing rust-lang#129187 (comment) `./x clean` to fail locally when `tmp` does not exist.

I plan to reland the two PRs with fixed top-level dir entry handling and more testing, but let's quickly revert to unblock people.

Reverts rust-lang#129187.
Reverts rust-lang#129302.

r? bootstrap
  • Loading branch information
bors committed Aug 22, 2024
2 parents eff0948 + 3957f39 commit b5723af
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 17 deletions.
93 changes: 78 additions & 15 deletions src/bootstrap/src/core/build_steps/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//! 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};
Expand Down Expand Up @@ -100,11 +101,11 @@ fn clean(build: &Build, all: bool, stage: Option<u32>) {
return;
}

remove_dir_recursive("tmp");
rm_rf("tmp".as_ref());

// Clean the entire build directory
if all {
remove_dir_recursive(&build.out);
rm_rf(&build.out);
return;
}

Expand Down Expand Up @@ -135,17 +136,17 @@ fn clean_specific_stage(build: &Build, stage: u32) {
}

let path = t!(entry.path().canonicalize());
remove_dir_recursive(&path);
rm_rf(&path);
}
}
}

fn clean_default(build: &Build) {
remove_dir_recursive(build.out.join("tmp"));
remove_dir_recursive(build.out.join("dist"));
remove_dir_recursive(build.out.join("bootstrap").join(".last-warned-change-id"));
remove_dir_recursive(build.out.join("bootstrap-shims-dump"));
remove_dir_recursive(build.out.join("rustfmt.stamp"));
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"));

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)
Expand All @@ -165,16 +166,78 @@ fn clean_default(build: &Build) {
continue;
}
let path = t!(entry.path().canonicalize());
remove_dir_recursive(&path);
rm_rf(&path);
}
}
}

/// Wrapper for [`std::fs::remove_dir_all`] that panics on failure and prints the `path` we failed
/// on.
fn remove_dir_recursive<P: AsRef<Path>>(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());
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<F>(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);
}
}
}
27 changes: 25 additions & 2 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3265,7 +3265,7 @@ impl<'test> TestCx<'test> {

let tmpdir = cwd.join(self.output_base_name());
if tmpdir.exists() {
fs::remove_dir_all(&tmpdir).unwrap();
self.aggressive_rm_rf(&tmpdir).unwrap();
}
create_dir_all(&tmpdir).unwrap();

Expand Down Expand Up @@ -3404,6 +3404,29 @@ impl<'test> TestCx<'test> {
}
}

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)
}

fn run_rmake_v2_test(&self) {
// For `run-make` V2, we need to perform 2 steps to build and run a `run-make` V2 recipe
// (`rmake.rs`) to run the actual tests. The support library is already built as a tool rust
Expand Down Expand Up @@ -3452,7 +3475,7 @@ impl<'test> TestCx<'test> {
// This setup intentionally diverges from legacy Makefile run-make tests.
let base_dir = self.output_base_name();
if base_dir.exists() {
fs::remove_dir_all(&base_dir).unwrap();
self.aggressive_rm_rf(&base_dir).unwrap();
}
let rmake_out_dir = base_dir.join("rmake_out");
create_dir_all(&rmake_out_dir).unwrap();
Expand Down

0 comments on commit b5723af

Please sign in to comment.