Skip to content

Commit

Permalink
Merge pull request #1885 from rbtcollins/bug-1870
Browse files Browse the repository at this point in the history
Bug 1870 - UI edition
  • Loading branch information
kinnison authored Jun 3, 2019
2 parents a253f95 + dde38d2 commit 2e97e32
Show file tree
Hide file tree
Showing 12 changed files with 167 additions and 78 deletions.
13 changes: 7 additions & 6 deletions src/cli/self_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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";
Expand All @@ -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())?;
}
Expand All @@ -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())?;
}
Expand All @@ -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(())
}
Expand Down Expand Up @@ -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
Expand Down
22 changes: 10 additions & 12 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));

Expand Down Expand Up @@ -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)
})?;
}

Expand All @@ -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))
Expand Down Expand Up @@ -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
Expand All @@ -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(())
}
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/dist/component/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
54 changes: 38 additions & 16 deletions src/dist/component/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand All @@ -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(())
}
Expand Down Expand Up @@ -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(())
}
Expand All @@ -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));
Expand All @@ -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);
Expand All @@ -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)
}
Expand Down Expand Up @@ -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()?;
Expand All @@ -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))
}
}
Expand All @@ -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()?;
Expand All @@ -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))
}
}
Expand All @@ -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))
}
Expand All @@ -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))
}
}
4 changes: 1 addition & 3 deletions src/dist/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 11 additions & 4 deletions src/dist/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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 })
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 2e97e32

Please sign in to comment.