about summary refs log tree commit diff
path: root/src/bootstrap
diff options
context:
space:
mode:
authorCorey Farwell <coreyf@rwell.org>2017-04-05 23:51:41 -0400
committerGitHub <noreply@github.com>2017-04-05 23:51:41 -0400
commit1a4aab94c314f7564e825bea7be2032841487c22 (patch)
tree46eb8e3ce5554f1cbdd4d965cd8011a210f6eab9 /src/bootstrap
parent083c7a93beb2951465732b015afc7deb651a7ce5 (diff)
parentefd6eab366ad44ac6e9b0b12cd4b9688ab71df6d (diff)
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. :-)
Diffstat (limited to 'src/bootstrap')
-rw-r--r--src/bootstrap/clean.rs42
1 files changed, 22 insertions, 20 deletions
diff --git a/src/bootstrap/clean.rs b/src/bootstrap/clean.rs
index e9547ee42d0..308a0ab3076 100644
--- a/src/bootstrap/clean.rs
+++ b/src/bootstrap/clean.rs
@@ -44,26 +44,25 @@ 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)
@@ -71,9 +70,12 @@ fn do_op<F>(path: &Path, desc: &str, mut f: F)
 {
     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| {