From 67a07ff8387df1397186d518b46dcff00ef098c6 Mon Sep 17 00:00:00 2001 From: Fredrik Larsson Date: Sun, 10 Sep 2017 20:41:12 +0200 Subject: [PATCH 1/5] Make it possible to uninstall multiple packages --- src/bin/uninstall.rs | 8 +++--- src/cargo/ops/cargo_install.rs | 45 +++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/bin/uninstall.rs b/src/bin/uninstall.rs index 2adf2d04156..3774f0c31fb 100644 --- a/src/bin/uninstall.rs +++ b/src/bin/uninstall.rs @@ -13,14 +13,14 @@ pub struct Options { #[serde(rename = "flag_Z")] flag_z: Vec, - arg_spec: String, + arg_spec: Vec, } pub const USAGE: &'static str = " Remove a Rust binary Usage: - cargo uninstall [options] + cargo uninstall [options] ... cargo uninstall (-h | --help) Options: @@ -49,7 +49,9 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { &options.flag_z)?; let root = options.flag_root.as_ref().map(|s| &s[..]); - ops::uninstall(root, &options.arg_spec, &options.flag_bin, config)?; + let specs = options.arg_spec.iter().map(|s| &s[..]).collect::>(); + + ops::uninstall(root, specs, &options.flag_bin, config)?; Ok(()) } diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 4c88e8951f7..01189bc61ee 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -557,10 +557,53 @@ pub fn install_list(dst: Option<&str>, config: &Config) -> CargoResult<()> { } pub fn uninstall(root: Option<&str>, - spec: &str, + specs: Vec<&str>, bins: &[String], config: &Config) -> CargoResult<()> { let root = resolve_root(root, config)?; + let scheduled_error = if specs.len() == 1 { + uninstall_one(root, specs[0], bins, config)?; + false + } else { + let mut succeeded = vec![]; + let mut failed = vec![]; + for spec in specs { + let root = root.clone(); + match uninstall_one(root, spec, bins, config) { + Ok(()) => succeeded.push(spec), + Err(e) => { + ::handle_error(e, &mut config.shell()); + failed.push(spec) + } + } + } + + let mut summary = vec![]; + if !succeeded.is_empty() { + summary.push(format!("Successfully uninstalled {}!", succeeded.join(", "))); + } + if !failed.is_empty() { + summary.push(format!("Failed to uninstall {} (see error(s) above).", failed.join(", "))); + } + + if !succeeded.is_empty() || !failed.is_empty() { + config.shell().status("\nSummary:", summary.join(" "))?; + } + + !failed.is_empty() + }; + + if scheduled_error { + bail!("some packages failed to uninstall"); + } + + Ok(()) +} + +pub fn uninstall_one(root: Filesystem, + spec: &str, + bins: &[String], + config: &Config) -> CargoResult<()> { let crate_metadata = metadata(config, &root)?; let mut metadata = read_crate_list(&crate_metadata)?; let mut to_remove = Vec::new(); From a8f403f1967bba658b860386dd649a3d66defdd1 Mon Sep 17 00:00:00 2001 From: Fredrik Larsson Date: Wed, 11 Oct 2017 20:43:29 +0200 Subject: [PATCH 2/5] Bailout if --bin is specified when uninstalling multiple packages --- src/cargo/ops/cargo_install.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 01189bc61ee..addfa83af13 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -560,6 +560,10 @@ pub fn uninstall(root: Option<&str>, specs: Vec<&str>, bins: &[String], config: &Config) -> CargoResult<()> { + if specs.len() > 1 && bins.len() > 0 { + bail!("A binary can only be associated with a single installed package, specifying multiple specs with --bin is redundant."); + } + let root = resolve_root(root, config)?; let scheduled_error = if specs.len() == 1 { uninstall_one(root, specs[0], bins, config)?; From 6431336a850eb96fd2200627b2ea26d5a110b1f6 Mon Sep 17 00:00:00 2001 From: Fredrik Larsson Date: Wed, 11 Oct 2017 21:34:44 +0200 Subject: [PATCH 3/5] Use multiple install command in the corresponding test --- tests/install.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/install.rs b/tests/install.rs index b7935affd3b..c4c722bfb70 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -82,16 +82,15 @@ error: some crates failed to install assert_that(cargo_home(), has_installed_exe("foo")); assert_that(cargo_home(), has_installed_exe("bar")); - assert_that(cargo_process("uninstall").arg("foo"), + assert_that(cargo_process("uninstall").args(&["foo", "bar"]), execs().with_status(0).with_stderr(&format!("\ [REMOVING] {home}[..]bin[..]foo[..] -", - home = cargo_home().display()))); - assert_that(cargo_process("uninstall").arg("bar"), - execs().with_status(0).with_stderr(&format!("\ [REMOVING] {home}[..]bin[..]bar[..] + +Summary: Successfully uninstalled foo, bar! ", home = cargo_home().display()))); + assert_that(cargo_home(), is_not(has_installed_exe("foo"))); assert_that(cargo_home(), is_not(has_installed_exe("bar"))); } From cf58031bcc482e80d6c3a606ba2dde43a50458b9 Mon Sep 17 00:00:00 2001 From: Fredrik Larsson Date: Wed, 11 Oct 2017 21:35:30 +0200 Subject: [PATCH 4/5] Test bailout when specifying --bin during uninstall of multiple packages --- tests/install.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/install.rs b/tests/install.rs index c4c722bfb70..2353f31a38e 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -962,3 +962,11 @@ fn test_install_git_cannot_be_a_base_url() { error: invalid url `github.com:rust-lang-nursery/rustfmt.git`: cannot-be-a-base-URLs are not supported ")); } + +#[test] +fn uninstall_multiple_and_specifying_bin() { + assert_that(cargo_process("uninstall").args(&["foo", "bar"]).arg("--bin").arg("baz"), + execs().with_status(101).with_stderr("\ +error: A binary can only be associated with a single installed package, specifying multiple specs with --bin is redundant. +")); +} From 05224d67cf1d6a1c4aa08ac09a8a9e1dbb95f57a Mon Sep 17 00:00:00 2001 From: Fredrik Larsson Date: Sat, 28 Oct 2017 16:36:49 +0200 Subject: [PATCH 5/5] Test partially successful uninstallation --- tests/install.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/install.rs b/tests/install.rs index 2353f31a38e..0b43acc44b7 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -970,3 +970,24 @@ fn uninstall_multiple_and_specifying_bin() { error: A binary can only be associated with a single installed package, specifying multiple specs with --bin is redundant. ")); } + +#[test] +fn uninstall_multiple_and_some_pkg_does_not_exist() { + pkg("foo", "0.0.1"); + + assert_that(cargo_process("install").arg("foo"), + execs().with_status(0)); + + assert_that(cargo_process("uninstall").args(&["foo", "bar"]), + execs().with_status(101).with_stderr(&format!("\ +[REMOVING] {home}[..]bin[..]foo[..] +error: package id specification `bar` matched no packages + +Summary: Successfully uninstalled foo! Failed to uninstall bar (see error(s) above). +error: some packages failed to uninstall +", + home = cargo_home().display()))); + + assert_that(cargo_home(), is_not(has_installed_exe("foo"))); + assert_that(cargo_home(), is_not(has_installed_exe("bar"))); +}