Skip to content

Commit 1a4aab9

Browse files
authored
Rollup merge of #41026 - CleanCut:rust-40860, r=alexcrichton
Handle symlinks in src/bootstrap/clean.rs (mostly) -- resolves #40860. In response to #40860 The broken condition can be replicated with: ```shell export MYARCH=x86_64-apple-darwin && mkdir -p build/$MYARCH/subdir && touch build/$MYARCH/subdir/file && ln -s build/$MYARCH/subdir/file build/$MYARCH/subdir/symlink ``` `src/bootstrap/clean.rs` has a custom implementation of removing a tree `fn rm_rf` that used `std::path::Path::{is_file, is_dir, exists}` while recursively deleting directories and files. Unfortunately, `Path`'s implementation of `is_file()` and `is_dir()` and `exists()` always unconditionally follow symlinks, which is the exact opposite of standard implementations of deleting file trees. It appears that this custom implementation is being used to workaround a behavior in Windows where the files often get marked as read-only, which prevents us from simply using something nice and simple like `std::fs::remove_dir_all`, which properly deletes links instead of following them. So it looks like the fix is to use `.symlink_metadata()` to figure out whether tree items are files/symlinks/directories. The one corner case this won't cover is if there is a broken symlink in the "root" `build/$MYARCH` directory, because those initial entries are run through `Path::canonicalize()`, which panics with broken symlinks. So lets just never use symlinks in that one directory. :-)
2 parents 083c7a9 + efd6eab commit 1a4aab9

File tree

1 file changed

+22
-20
lines changed

1 file changed

+22
-20
lines changed

src/bootstrap/clean.rs

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,36 +44,38 @@ pub fn clean(build: &Build) {
4444
}
4545

4646
fn rm_rf(path: &Path) {
47-
if !path.exists() {
48-
return
49-
}
50-
if path.is_file() {
51-
return do_op(path, "remove file", |p| fs::remove_file(p));
52-
}
53-
54-
for file in t!(fs::read_dir(path)) {
55-
let file = t!(file).path();
47+
match path.symlink_metadata() {
48+
Err(e) => {
49+
if e.kind() == ErrorKind::NotFound {
50+
return;
51+
}
52+
panic!("failed to get metadata for file {}: {}", path.display(), e);
53+
},
54+
Ok(metadata) => {
55+
if metadata.file_type().is_file() || metadata.file_type().is_symlink() {
56+
do_op(path, "remove file", |p| fs::remove_file(p));
57+
return;
58+
}
5659

57-
if file.is_dir() {
58-
rm_rf(&file);
59-
} else {
60-
// On windows we can't remove a readonly file, and git will
61-
// often clone files as readonly. As a result, we have some
62-
// special logic to remove readonly files on windows.
63-
do_op(&file, "remove file", |p| fs::remove_file(p));
64-
}
65-
}
66-
do_op(path, "remove dir", |p| fs::remove_dir(p));
60+
for file in t!(fs::read_dir(path)) {
61+
rm_rf(&t!(file).path());
62+
}
63+
do_op(path, "remove dir", |p| fs::remove_dir(p));
64+
},
65+
};
6766
}
6867

6968
fn do_op<F>(path: &Path, desc: &str, mut f: F)
7069
where F: FnMut(&Path) -> io::Result<()>
7170
{
7271
match f(path) {
7372
Ok(()) => {}
73+
// On windows we can't remove a readonly file, and git will often clone files as readonly.
74+
// As a result, we have some special logic to remove readonly files on windows.
75+
// This is also the reason that we can't use things like fs::remove_dir_all().
7476
Err(ref e) if cfg!(windows) &&
7577
e.kind() == ErrorKind::PermissionDenied => {
76-
let mut p = t!(path.metadata()).permissions();
78+
let mut p = t!(path.symlink_metadata()).permissions();
7779
p.set_readonly(false);
7880
t!(fs::set_permissions(path, p));
7981
f(path).unwrap_or_else(|e| {

0 commit comments

Comments
 (0)