Merge pull request rust-lang#1885 from rbtcollins/bug-1870

Bug 1870 - UI edition

src/cli/self_update.rs

@@ -36,6 +36,7 @@ use crate::markdown::md;
 use crate::term2;
 use rustup::dist::dist;
 use rustup::utils::utils;
+use rustup::utils::Notification;
 use rustup::{DUP_TOOLS, TOOLS};
 use same_file::Handle;
 use std::env;
@@ -613,7 +614,7 @@ fn install_bins() -> Result<()> {
     let this_exe_path = utils::current_exe()?;
     let rustup_path = bin_path.join(&format!("rustup{}", EXE_SUFFIX));
 
-    utils::ensure_dir_exists("bin", &bin_path, &|_| {})?;
+    utils::ensure_dir_exists("bin", &bin_path, &|_: Notification<'_>| {})?;
     // NB: Even on Linux we can't just copy the new binary over the (running)
     // old binary; we must unlink it first.
     if rustup_path.exists() {
@@ -760,7 +761,7 @@ pub fn uninstall(no_prompt: bool) -> Result<()> {
     // Delete RUSTUP_HOME
     let rustup_dir = utils::rustup_home()?;
     if rustup_dir.exists() {
-        utils::remove_dir("rustup_home", &rustup_dir, &|_| {})?;
+        utils::remove_dir("rustup_home", &rustup_dir, &|_: Notification<'_>| {})?;
     }
 
     let read_dir_err = "failure reading directory";
@@ -778,7 +779,7 @@ pub fn uninstall(no_prompt: bool) -> Result<()> {
         let dirent = dirent.chain_err(|| read_dir_err)?;
         if dirent.file_name().to_str() != Some("bin") {
             if dirent.path().is_dir() {
-                utils::remove_dir("cargo_home", &dirent.path(), &|_| {})?;
+                utils::remove_dir("cargo_home", &dirent.path(), &|_: Notification<'_>| {})?;
             } else {
                 utils::remove_file("cargo_home", &dirent.path())?;
             }
@@ -798,7 +799,7 @@ pub fn uninstall(no_prompt: bool) -> Result<()> {
         let file_is_tool = name.to_str().map(|n| tools.iter().any(|t| *t == n));
         if file_is_tool == Some(false) {
             if dirent.path().is_dir() {
-                utils::remove_dir("cargo_home", &dirent.path(), &|_| {})?;
+                utils::remove_dir("cargo_home", &dirent.path(), &|_: Notification<'_>| {})?;
             } else {
                 utils::remove_file("cargo_home", &dirent.path())?;
             }
@@ -820,7 +821,7 @@ pub fn uninstall(no_prompt: bool) -> Result<()> {
 #[cfg(unix)]
 fn delete_rustup_and_cargo_home() -> Result<()> {
     let cargo_home = utils::cargo_home()?;
-    utils::remove_dir("cargo_home", &cargo_home, &|_| ())?;
+    utils::remove_dir("cargo_home", &cargo_home, &|_: Notification<'_>| ())?;
 
     Ok(())
 }
@@ -946,7 +947,7 @@ pub fn complete_windows_uninstall() -> Result<()> {
 
     // Now that the parent has exited there are hopefully no more files open in CARGO_HOME
     let cargo_home = utils::cargo_home()?;
-    utils::remove_dir("cargo_home", &cargo_home, &|_| ())?;
+    utils::remove_dir("cargo_home", &cargo_home, &|_: Notification<'_>| ())?;
 
     // Now, run a *system* binary to inherit the DELETE_ON_CLOSE
     // handle to *this* process, then exit. The OS will delete the gc

src/config.rs

@@ -52,7 +52,7 @@ impl Cfg {
         // Set up the rustup home directory
         let rustup_dir = utils::rustup_home()?;
 
-        utils::ensure_dir_exists("home", &rustup_dir, &|n| notify_handler(n.into()))?;
+        utils::ensure_dir_exists("home", &rustup_dir, notify_handler.as_ref())?;
 
         let settings_file = SettingsFile::new(rustup_dir.join("settings.toml"));
 
@@ -130,7 +130,7 @@ impl Cfg {
     pub fn get_toolchain(&self, name: &str, create_parent: bool) -> Result<Toolchain<'_>> {
         if create_parent {
             utils::ensure_dir_exists("toolchains", &self.toolchains_dir, &|n| {
-                (self.notify_handler)(n.into())
+                (self.notify_handler)(n)
             })?;
         }
 
@@ -145,9 +145,11 @@ impl Cfg {
 
     pub fn get_hash_file(&self, toolchain: &str, create_parent: bool) -> Result<PathBuf> {
         if create_parent {
-            utils::ensure_dir_exists("update-hash", &self.update_hash_dir, &|n| {
-                (self.notify_handler)(n.into())
-            })?;
+            utils::ensure_dir_exists(
+                "update-hash",
+                &self.update_hash_dir,
+                self.notify_handler.as_ref(),
+            )?;
         }
 
         Ok(self.update_hash_dir.join(toolchain))
@@ -182,9 +184,7 @@ impl Cfg {
                 let dirs = utils::read_dir("toolchains", &self.toolchains_dir)?;
                 for dir in dirs {
                     let dir = dir.chain_err(|| ErrorKind::UpgradeIoError)?;
-                    utils::remove_dir("toolchain", &dir.path(), &|n| {
-                        (self.notify_handler)(n.into())
-                    })?;
+                    utils::remove_dir("toolchain", &dir.path(), self.notify_handler.as_ref())?;
                 }
 
                 // Also delete the update hashes
@@ -205,9 +205,7 @@ impl Cfg {
 
     pub fn delete_data(&self) -> Result<()> {
         if utils::path_exists(&self.rustup_dir) {
-            utils::remove_dir("home", &self.rustup_dir, &|n| {
-                (self.notify_handler)(n.into())
-            })
+            utils::remove_dir("home", &self.rustup_dir, self.notify_handler.as_ref())
         } else {
             Ok(())
         }
@@ -297,7 +295,7 @@ impl Cfg {
         settings: &Settings,
     ) -> Result<Option<(String, OverrideReason)>> {
         let notify = self.notify_handler.as_ref();
-        let dir = utils::canonicalize_path(dir, &|n| notify(n.into()));
+        let dir = utils::canonicalize_path(dir, notify);
         let mut dir = Some(&*dir);
 
         while let Some(d) = dir {

src/dist/component/components.rs

@@ -194,7 +194,7 @@ impl Component {
         let temp = tx.temp().new_file()?;
         utils::filter_file("components", &abs_path, &temp, |l| (l != self.name))?;
         tx.modify_file(path)?;
-        utils::rename_file("components", &temp, &abs_path)?;
+        utils::rename_file("components", &temp, &abs_path, tx.notify_handler())?;
 
         // TODO: If this is the last component remove the components file
         // and the version file.

src/dist/component/transaction.rs

@@ -93,7 +93,13 @@ impl<'a> Transaction<'a> {
     /// Remove a file from a relative path to the install prefix.
     pub fn remove_file(&mut self, component: &str, relpath: PathBuf) -> Result<()> {
         assert!(relpath.is_relative());
-        let item = ChangedItem::remove_file(&self.prefix, component, relpath, &self.temp_cfg)?;
+        let item = ChangedItem::remove_file(
+            &self.prefix,
+            component,
+            relpath,
+            &self.temp_cfg,
+            self.notify_handler(),
+        )?;
         self.change(item);
         Ok(())
     }
@@ -102,7 +108,13 @@ impl<'a> Transaction<'a> {
     /// install prefix.
     pub fn remove_dir(&mut self, component: &str, relpath: PathBuf) -> Result<()> {
         assert!(relpath.is_relative());
-        let item = ChangedItem::remove_dir(&self.prefix, component, relpath, &self.temp_cfg)?;
+        let item = ChangedItem::remove_dir(
+            &self.prefix,
+            component,
+            relpath,
+            &self.temp_cfg,
+            self.notify_handler(),
+        )?;
         self.change(item);
         Ok(())
     }
@@ -136,15 +148,17 @@ impl<'a> Transaction<'a> {
     /// Move a file to a relative path of the install prefix.
     pub fn move_file(&mut self, component: &str, relpath: PathBuf, src: &Path) -> Result<()> {
         assert!(relpath.is_relative());
-        let item = ChangedItem::move_file(&self.prefix, component, relpath, src)?;
+        let item =
+            ChangedItem::move_file(&self.prefix, component, relpath, src, self.notify_handler())?;
         self.change(item);
         Ok(())
     }
 
     /// Recursively move a directory to a relative path of the install prefix.
     pub fn move_dir(&mut self, component: &str, relpath: PathBuf, src: &Path) -> Result<()> {
         assert!(relpath.is_relative());
-        let item = ChangedItem::move_dir(&self.prefix, component, relpath, src)?;
+        let item =
+            ChangedItem::move_dir(&self.prefix, component, relpath, src, self.notify_handler())?;
         self.change(item);
         Ok(())
     }
@@ -166,7 +180,7 @@ impl<'a> Drop for Transaction<'a> {
             for item in self.changes.iter().rev() {
                 // ok_ntfy!(self.notify_handler,
                 //          Notification::NonFatalError,
-                match item.roll_back(&self.prefix) {
+                match item.roll_back(&self.prefix, self.notify_handler()) {
                     Ok(()) => {}
                     Err(e) => {
                         (self.notify_handler)(Notification::NonFatalError(&e));
@@ -191,16 +205,20 @@ enum ChangedItem<'a> {
 }
 
 impl<'a> ChangedItem<'a> {
-    fn roll_back(&self, prefix: &InstallPrefix) -> Result<()> {
+    fn roll_back(
+        &self,
+        prefix: &InstallPrefix,
+        notify: &'a dyn Fn(Notification<'_>),
+    ) -> Result<()> {
         use self::ChangedItem::*;
         match self {
             AddedFile(path) => utils::remove_file("component", &prefix.abs_path(path))?,
-            AddedDir(path) => utils::remove_dir("component", &prefix.abs_path(path), &|_| ())?,
+            AddedDir(path) => utils::remove_dir("component", &prefix.abs_path(path), notify)?,
             RemovedFile(path, tmp) | ModifiedFile(path, Some(tmp)) => {
-                utils::rename_file("component", &tmp, &prefix.abs_path(path))?
+                utils::rename_file("component", &tmp, &prefix.abs_path(path), notify)?
             }
             RemovedDir(path, tmp) => {
-                utils::rename_dir("component", &tmp.join("bk"), &prefix.abs_path(path))?
+                utils::rename_dir("component", &tmp.join("bk"), &prefix.abs_path(path), notify)?
             }
             ModifiedFile(path, None) => {
                 let abs_path = prefix.abs_path(path);
@@ -225,7 +243,7 @@ impl<'a> ChangedItem<'a> {
             .into())
         } else {
             if let Some(p) = abs_path.parent() {
-                utils::ensure_dir_exists("component", p, &|_| ())?;
+                utils::ensure_dir_exists("component", p, &|_: Notification<'_>| ())?;
             }
             Ok(abs_path)
         }
@@ -253,14 +271,15 @@ impl<'a> ChangedItem<'a> {
         src: &Path,
     ) -> Result<Self> {
         let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?;
-        utils::copy_dir(src, &abs_path, &|_| ())?;
+        utils::copy_dir(src, &abs_path, &|_: Notification<'_>| ())?;
         Ok(ChangedItem::AddedDir(relpath))
     }
     fn remove_file(
         prefix: &InstallPrefix,
         component: &str,
         relpath: PathBuf,
         temp_cfg: &'a temp::Cfg,
+        notify: &'a dyn Fn(Notification<'_>),
     ) -> Result<Self> {
         let abs_path = prefix.abs_path(&relpath);
         let backup = temp_cfg.new_file()?;
@@ -271,7 +290,7 @@ impl<'a> ChangedItem<'a> {
             }
             .into())
         } else {
-            utils::rename_file("component", &abs_path, &backup)?;
+            utils::rename_file("component", &abs_path, &backup, notify)?;
             Ok(ChangedItem::RemovedFile(relpath, backup))
         }
     }
@@ -280,6 +299,7 @@ impl<'a> ChangedItem<'a> {
         component: &str,
         relpath: PathBuf,
         temp_cfg: &'a temp::Cfg,
+        notify: &'a dyn Fn(Notification<'_>),
     ) -> Result<Self> {
         let abs_path = prefix.abs_path(&relpath);
         let backup = temp_cfg.new_directory()?;
@@ -290,7 +310,7 @@ impl<'a> ChangedItem<'a> {
             }
             .into())
         } else {
-            utils::rename_dir("component", &abs_path, &backup.join("bk"))?;
+            utils::rename_dir("component", &abs_path, &backup.join("bk"), notify)?;
             Ok(ChangedItem::RemovedDir(relpath, backup))
         }
     }
@@ -307,7 +327,7 @@ impl<'a> ChangedItem<'a> {
             Ok(ChangedItem::ModifiedFile(relpath, Some(backup)))
         } else {
             if let Some(p) = abs_path.parent() {
-                utils::ensure_dir_exists("component", p, &|_| {})?;
+                utils::ensure_dir_exists("component", p, &|_: Notification<'_>| {})?;
             }
             Ok(ChangedItem::ModifiedFile(relpath, None))
         }
@@ -317,19 +337,21 @@ impl<'a> ChangedItem<'a> {
         component: &str,
         relpath: PathBuf,
         src: &Path,
+        notify: &'a dyn Fn(Notification<'_>),
     ) -> Result<Self> {
         let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?;
-        utils::rename_file("component", src, &abs_path)?;
+        utils::rename_file("component", src, &abs_path, notify)?;
         Ok(ChangedItem::AddedFile(relpath))
     }
     fn move_dir(
         prefix: &InstallPrefix,
         component: &str,
         relpath: PathBuf,
         src: &Path,
+        notify: &'a dyn Fn(Notification<'_>),
     ) -> Result<Self> {
         let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?;
-        utils::rename_dir("component", src, &abs_path)?;
+        utils::rename_dir("component", src, &abs_path, notify)?;
         Ok(ChangedItem::AddedDir(relpath))
     }
 }

src/dist/dist.rs

@@ -543,9 +543,7 @@ pub fn update_from_dist<'a>(
     // Don't leave behind an empty / broken installation directory
     if res.is_err() && fresh_install {
         // FIXME Ignoring cascading errors
-        let _ = utils::remove_dir("toolchain", prefix.path(), &|n| {
-            (download.notify_handler)(n.into())
-        });
+        let _ = utils::remove_dir("toolchain", prefix.path(), download.notify_handler);
     }
 
     res

src/dist/download.rs

@@ -37,9 +37,11 @@ impl<'a> DownloadCfg<'a> {
     /// target file already exists, then the hash is checked and it is returned
     /// immediately without re-downloading.
     pub fn download(&self, url: &Url, hash: &str) -> Result<File> {
-        utils::ensure_dir_exists("Download Directory", &self.download_dir, &|n| {
-            (self.notify_handler)(n.into())
-        })?;
+        utils::ensure_dir_exists(
+            "Download Directory",
+            &self.download_dir,
+            &self.notify_handler,
+        )?;
         let target_file = self.download_dir.join(Path::new(hash));
 
         if target_file.exists() {
@@ -86,7 +88,12 @@ impl<'a> DownloadCfg<'a> {
         } else {
             (self.notify_handler)(Notification::ChecksumValid(&url.to_string()));
 
-            utils::rename_file("downloaded", &partial_file_path, &target_file)?;
+            utils::rename_file(
+                "downloaded",
+                &partial_file_path,
+                &target_file,
+                self.notify_handler,
+            )?;
             Ok(File { path: target_file })
         }
     }

src/install.rs

@@ -39,11 +39,11 @@ impl<'a> InstallMethod<'a> {
 
         match self {
             InstallMethod::Copy(src) => {
-                utils::copy_dir(src, path, &|n| notify_handler(n.into()))?;
+                utils::copy_dir(src, path, notify_handler)?;
                 Ok(true)
             }
             InstallMethod::Link(src) => {
-                utils::symlink_dir(src, &path, &|n| notify_handler(n.into()))?;
+                utils::symlink_dir(src, &path, notify_handler)?;
                 Ok(true)
             }
             InstallMethod::Installer(src, temp_cfg) => {
@@ -105,5 +105,5 @@ impl<'a> InstallMethod<'a> {
 }
 
 pub fn uninstall(path: &Path, notify_handler: &dyn Fn(Notification<'_>)) -> Result<()> {
-    utils::remove_dir("install", path, &|n| notify_handler(n.into()))
+    utils::remove_dir("install", path, notify_handler)
 }

src/settings.rs

@@ -84,7 +84,7 @@ impl Default for Settings {
 impl Settings {
     fn path_to_key(path: &Path, notify_handler: &dyn Fn(Notification<'_>)) -> String {
         if path.exists() {
-            utils::canonicalize_path(path, &|n| notify_handler(n.into()))
+            utils::canonicalize_path(path, notify_handler)
                 .display()
                 .to_string()
         } else {