Skip to content

Commit

Permalink
Rollup merge of rust-lang#41026 - CleanCut:rust-40860, r=alexcrichton
Browse files Browse the repository at this point in the history
Handle symlinks in src/bootstrap/clean.rs (mostly) -- resolves rust-lang#40860.

In response to rust-lang#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. :-)
  • Loading branch information
frewsxcv authored Apr 6, 2017
2 parents 083c7a9 + efd6eab commit 1a4aab9
Showing 1 changed file with 22 additions and 20 deletions.
42 changes: 22 additions & 20 deletions src/bootstrap/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,36 +44,38 @@ pub fn clean(build: &Build) {
}

fn rm_rf(path: &Path) {
if !path.exists() {
return
}
if path.is_file() {
return do_op(path, "remove file", |p| fs::remove_file(p));
}

for file in t!(fs::read_dir(path)) {
let file = t!(file).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| fs::remove_file(p));
return;
}

if file.is_dir() {
rm_rf(&file);
} else {
// 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.
do_op(&file, "remove file", |p| fs::remove_file(p));
}
}
do_op(path, "remove dir", |p| fs::remove_dir(p));
for file in t!(fs::read_dir(path)) {
rm_rf(&t!(file).path());
}
do_op(path, "remove dir", |p| fs::remove_dir(p));
},
};
}

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().
Err(ref e) if cfg!(windows) &&
e.kind() == ErrorKind::PermissionDenied => {
let mut p = t!(path.metadata()).permissions();
let mut p = t!(path.symlink_metadata()).permissions();
p.set_readonly(false);
t!(fs::set_permissions(path, p));
f(path).unwrap_or_else(|e| {
Expand Down

0 comments on commit 1a4aab9

Please sign in to comment.