Auto merge of #1201 - RalfJung:symlinks, r=alexcrichton

correctly uninstall toolchains that are stale symlinks

Fixes #1169

I'd like to add a test for this -- what is the best way to do that?

Author: bors

src/rustup/toolchain.rs

@@ -68,13 +68,16 @@ impl<'a> Toolchain<'a> {
     pub fn path(&self) -> &Path {
         &self.path
     }
+    fn is_symlink(&self) -> bool {
+        use std::fs;
+        fs::symlink_metadata(&self.path).map(|m| m.file_type().is_symlink()).unwrap_or(false)
+    }
     pub fn exists(&self) -> bool {
         // HACK: linked toolchains are symlinks, and, contrary to what std docs
         // lead me to believe `fs::metadata`, used by `is_directory` does not
         // seem to follow symlinks on windows.
         let is_symlink = if cfg!(windows) {
-            use std::fs;
-            fs::symlink_metadata(&self.path).map(|m| m.file_type().is_symlink()).unwrap_or(false)
+            self.is_symlink()
         } else {
             false
         };
@@ -84,7 +87,7 @@ impl<'a> Toolchain<'a> {
         Ok(try!(utils::assert_is_directory(&self.path)))
     }
     pub fn remove(&self) -> Result<()> {
-        if self.exists() {
+        if self.exists() || self.is_symlink() {
             (self.cfg.notify_handler)(Notification::UninstallingToolchain(&self.name));
         } else {
             (self.cfg.notify_handler)(Notification::ToolchainNotInstalled(&self.name));

tests/cli-misc.rs

@@ -8,7 +8,7 @@ extern crate time;
 extern crate tempdir;
 
 use rustup_mock::clitools::{self, Config, Scenario,
-                            expect_stdout_ok, expect_stderr_ok,
+                            expect_stdout_ok, expect_stderr_ok, expect_ok_ex,
                             expect_ok, expect_err, expect_timeout_ok,
                             run, this_host_triple};
 use rustup_utils::{raw, utils};
@@ -481,3 +481,40 @@ fn with_no_prompt_install_succeeds_if_rustc_exists() {
         assert!(out.ok);
     });
 }
+
+// issue #1169
+#[test]
+#[cfg(any(unix, windows))]
+fn toolchain_broken_symlink() {
+    use std::path::Path;
+    use std::fs;
+
+    #[cfg(unix)]
+    fn create_symlink_dir<P: AsRef<Path>, Q: AsRef<Path>>(src: P, dst: Q) {
+        use std::os::unix::fs;
+        fs::symlink(src, dst).unwrap();
+    }
+
+    #[cfg(windows)]
+    fn create_symlink_dir<P: AsRef<Path>, Q: AsRef<Path>>(src: P, dst: Q) {
+        use std::os::windows::fs;
+        fs::symlink_dir(src, dst).unwrap();
+    }
+
+    setup(&|config| {
+        // We artifically create a broken symlink toolchain -- but this can also happen "legitimately"
+        // by having a proper toolchain there, using "toolchain link", and later removing the directory.
+        fs::create_dir(config.rustupdir.join("toolchains")).unwrap();
+        create_symlink_dir(config.rustupdir.join("this-directory-does-not-exist"), config.rustupdir.join("toolchains").join("test"));
+        // Make sure this "fake install" actually worked
+        expect_ok_ex(config, &["rustup", "toolchain", "list"], "test\n", "");
+        // Now try to uninstall it.  That should work only once.
+        expect_ok_ex(config, &["rustup", "toolchain", "uninstall", "test"], "",
+r"info: uninstalling toolchain 'test'
+info: toolchain 'test' uninstalled
+");
+        expect_ok_ex(config, &["rustup", "toolchain", "uninstall", "test"], "",
+r"info: no toolchain installed for 'test'
+");
+    });
+}