From 66168acd47ed5567a72f742faf89d1aa20b73e07 Mon Sep 17 00:00:00 2001 From: Paul Woolcock Date: Thu, 21 Apr 2016 12:23:33 -0400 Subject: [PATCH 01/11] old commit adding some support for multi-install --- src/bin/install.rs | 10 +++++----- tests/install.rs | 40 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/bin/install.rs b/src/bin/install.rs index 8d532e6bc1c..7aaca4d484e 100644 --- a/src/bin/install.rs +++ b/src/bin/install.rs @@ -22,7 +22,7 @@ pub struct Options { flag_frozen: bool, flag_locked: bool, - arg_crate: Option, + arg_crate: Vec, flag_vers: Option, flag_git: Option, @@ -37,7 +37,7 @@ pub const USAGE: &'static str = " Install a Rust binary Usage: - cargo install [options] [] + cargo install [options] [...] cargo install [options] --list Specifying what crate to install: @@ -139,20 +139,20 @@ pub fn execute(options: Options, config: &Config) -> CliResult { SourceId::for_git(&url, gitref) } else if let Some(path) = options.flag_path { SourceId::for_path(&config.cwd().join(path))? - } else if options.arg_crate == None { + } else if options.arg_crate.is_empty() { SourceId::for_path(&config.cwd())? } else { SourceId::crates_io(config)? }; - let krate = options.arg_crate.as_ref().map(|s| &s[..]); + let krates = options.arg_crate.iter().map(|s| &s[..]).collect::>(); let vers = options.flag_vers.as_ref().map(|s| &s[..]); let root = options.flag_root.as_ref().map(|s| &s[..]); if options.flag_list { ops::install_list(root, config)?; } else { - ops::install(root, krate, &source, vers, &compile_opts, options.flag_force)?; + ops::install(root, krates, &source, vers, &compile_opts, options.flag_force)?; } Ok(()) } diff --git a/tests/install.rs b/tests/install.rs index 22f9af060bb..7a61b716e18 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -55,7 +55,45 @@ warning: be sure to add `[..]` to your PATH to be able to run the installed bina } #[test] -fn pick_max_version() { +test!(multiple_pkgs { + pkg("foo", "0.0.1"); + pkg("bar", "0.0.1"); + + assert_that(cargo_process("install").arg("foo").arg("bar"), + execs().with_status(0).with_stdout(&format!("\ +{updating} registry `[..]` +{downloading} foo v0.0.1 (registry file://[..]) +{compiling} foo v0.0.1 (registry file://[..]) +{installing} {home}[..]bin[..]foo[..] +{downloading} bar v0.0.1 (registry file://[..]) +{compiling} bar v0.0.1 (registry file://[..]) +{installing} {home}[..]bin[..]bar[..] +", + updating = UPDATING, + downloading = DOWNLOADING, + compiling = COMPILING, + installing = INSTALLING, + home = cargo_home().display()))); + assert_that(cargo_home(), has_installed_exe("foo")); + assert_that(cargo_home(), has_installed_exe("bar")); + + assert_that(cargo_process("uninstall").arg("foo"), + execs().with_status(0).with_stdout(&format!("\ +{removing} {home}[..]bin[..]foo[..] +", + removing = REMOVING, + home = cargo_home().display()))); + assert_that(cargo_process("uninstall").arg("bar"), + execs().with_status(0).with_stdout(&format!("\ +{removing} {home}[..]bin[..]bar[..] +", + removing = REMOVING, + home = cargo_home().display()))); + assert_that(cargo_home(), is_not(has_installed_exe("foo"))); + assert_that(cargo_home(), is_not(has_installed_exe("bar"))); +}); + +test!(pick_max_version { pkg("foo", "0.0.1"); pkg("foo", "0.0.2"); From 40226f5d3919dcdc04e0b4b97ff37760b337567f Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Fri, 23 Jun 2017 19:30:05 -0400 Subject: [PATCH 02/11] update test --- tests/install.rs | 51 ++++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/tests/install.rs b/tests/install.rs index 7a61b716e18..b135922987c 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -55,45 +55,54 @@ warning: be sure to add `[..]` to your PATH to be able to run the installed bina } #[test] -test!(multiple_pkgs { +fn multiple_pkgs() { pkg("foo", "0.0.1"); pkg("bar", "0.0.1"); assert_that(cargo_process("install").arg("foo").arg("bar"), - execs().with_status(0).with_stdout(&format!("\ -{updating} registry `[..]` -{downloading} foo v0.0.1 (registry file://[..]) -{compiling} foo v0.0.1 (registry file://[..]) -{installing} {home}[..]bin[..]foo[..] -{downloading} bar v0.0.1 (registry file://[..]) -{compiling} bar v0.0.1 (registry file://[..]) -{installing} {home}[..]bin[..]bar[..] + execs().with_status(0).with_stderr(&format!("\ +[UPDATING] registry `[..]` +[DOWNLOADING] foo v0.0.1 (registry file://[..]) +[INSTALLING] foo v0.0.1 +[COMPILING] foo v0.0.1 +[FINISHED] release [optimized] target(s) in [..] +[INSTALLING] {home}[..]bin[..]foo[..] +warning: be sure to add `[..]` to your PATH to be able to run the installed binaries +[DOWNLOADING] bar v0.0.1 (registry file://[..]) +[INSTALLING] bar v0.0.1 +[COMPILING] bar v0.0.1 +[FINISHED] release [optimized] target(s) in [..] +[INSTALLING] {home}[..]bin[..]bar[..] +warning: be sure to add `[..]` to your PATH to be able to run the installed binaries + + +SUMMARY + +Successfully installed: foo, bar + +Errors: + ", - updating = UPDATING, - downloading = DOWNLOADING, - compiling = COMPILING, - installing = INSTALLING, home = cargo_home().display()))); assert_that(cargo_home(), has_installed_exe("foo")); assert_that(cargo_home(), has_installed_exe("bar")); assert_that(cargo_process("uninstall").arg("foo"), - execs().with_status(0).with_stdout(&format!("\ -{removing} {home}[..]bin[..]foo[..] + execs().with_status(0).with_stderr(&format!("\ +[REMOVING] {home}[..]bin[..]foo[..] ", - removing = REMOVING, home = cargo_home().display()))); assert_that(cargo_process("uninstall").arg("bar"), - execs().with_status(0).with_stdout(&format!("\ -{removing} {home}[..]bin[..]bar[..] + execs().with_status(0).with_stderr(&format!("\ +[REMOVING] {home}[..]bin[..]bar[..] ", - removing = REMOVING, home = cargo_home().display()))); assert_that(cargo_home(), is_not(has_installed_exe("foo"))); assert_that(cargo_home(), is_not(has_installed_exe("bar"))); -}); +} -test!(pick_max_version { +#[test] +fn pick_max_version() { pkg("foo", "0.0.1"); pkg("foo", "0.0.2"); From f2084b72593c116e58d406350d1347ce6f281104 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Fri, 23 Jun 2017 19:30:25 -0400 Subject: [PATCH 03/11] make SourceConfigMap cloneable --- src/cargo/sources/config.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cargo/sources/config.rs b/src/cargo/sources/config.rs index 8799f0c30e8..faae80c2a0d 100644 --- a/src/cargo/sources/config.rs +++ b/src/cargo/sources/config.rs @@ -15,6 +15,7 @@ use util::{Config, ToUrl}; use util::config::ConfigValue; use util::errors::{CargoError, CargoResult, CargoResultExt}; +#[derive(Clone)] pub struct SourceConfigMap<'cfg> { cfgs: HashMap, id2name: HashMap, @@ -28,6 +29,7 @@ pub struct SourceConfigMap<'cfg> { /// registry = 'https://github.com/rust-lang/crates.io-index' /// replace-with = 'foo' # optional /// ``` +#[derive(Clone)] struct SourceConfig { // id this source corresponds to, inferred from the various defined keys in // the configuration From 497c2975c05b96e72fac77f01caa09a15b910b13 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Fri, 23 Jun 2017 19:30:46 -0400 Subject: [PATCH 04/11] support installing multiple crates --- src/cargo/ops/cargo_install.rs | 62 +++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 5421ebbcee3..ef2756ea7ee 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -4,9 +4,10 @@ use std::env; use std::ffi::OsString; use std::fs::{self, File}; use std::io::prelude::*; -use std::io::SeekFrom; +use std::io::{self, SeekFrom}; use std::path::{Path, PathBuf}; use std::sync::Arc; +use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering}; use semver::Version; use tempdir::TempDir; @@ -55,20 +56,57 @@ impl Drop for Transaction { } pub fn install(root: Option<&str>, + krates: Vec<&str>, + source_id: &SourceId, + vers: Option<&str>, + opts: &ops::CompileOptions, + force: bool) -> CargoResult<()> { + let root = resolve_root(root, opts.config)?; + let map = SourceConfigMap::new(opts.config)?; + + if krates.is_empty() { + install_one(root, map, None, source_id, vers, opts, force) + } else { + let mut success = vec![]; + let mut errors = vec![]; + for krate in krates { + let root = root.clone(); + let map = map.clone(); + match install_one(root, map, Some(krate), source_id, vers, opts, force) { + Ok(()) => success.push(krate), + Err(e) => errors.push(format!("{}: {}", krate, e)) + } + } + + writeln!(io::stderr(), + "\n\nSUMMARY\n\nSuccessfully installed: {}\n\nErrors:\n\t{}", + success.join(", "), + errors.join("\n\t"))?; + + Ok(()) + } +} + +fn install_one(root: Filesystem, + map: SourceConfigMap, krate: Option<&str>, source_id: &SourceId, vers: Option<&str>, opts: &ops::CompileOptions, force: bool) -> CargoResult<()> { + + static ALREADY_UPDATED: AtomicBool = ATOMIC_BOOL_INIT; + let needs_update = !ALREADY_UPDATED.load(Ordering::SeqCst); + let config = opts.config; - let root = resolve_root(root, config)?; - let map = SourceConfigMap::new(config)?; + let (pkg, source) = if source_id.is_git() { select_pkg(GitSource::new(source_id, config), - krate, vers, config, &mut |git| git.read_packages())? + krate, vers, config, needs_update, + &mut |git| git.read_packages())? } else if source_id.is_path() { - let path = source_id.url().to_file_path().ok() - .expect("path sources must have a valid path"); + let path = source_id.url().to_file_path() + .map_err(|()| CargoError::from("path sources must have a valid path"))?; let mut src = PathSource::new(&path, source_id, config); src.update().chain_err(|| { format!("`{}` is not a crate root; specify a crate to \ @@ -76,15 +114,17 @@ pub fn install(root: Option<&str>, specify an alternate source", path.display()) })?; select_pkg(PathSource::new(&path, source_id, config), - krate, vers, config, &mut |path| path.read_packages())? + krate, vers, config, needs_update, + &mut |path| path.read_packages())? } else { select_pkg(map.load(source_id)?, - krate, vers, config, + krate, vers, config, needs_update, &mut |_| Err("must specify a crate to install from \ crates.io, or use --path or --git to \ specify alternate source".into()))? }; + ALREADY_UPDATED.store(true, Ordering::SeqCst); let mut td_opt = None; let overidden_target_dir = if source_id.is_path() { @@ -267,11 +307,15 @@ fn select_pkg<'a, T>(mut source: T, name: Option<&str>, vers: Option<&str>, config: &Config, + needs_update: bool, list_all: &mut FnMut(&mut T) -> CargoResult>) -> CargoResult<(Package, Box)> where T: Source + 'a { - source.update()?; + if needs_update { + source.update()?; + } + match name { Some(name) => { let vers = match vers { From a65fad08cc61273d2fc86fd3d50157c674a7d6f0 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Fri, 23 Jun 2017 23:27:46 -0400 Subject: [PATCH 05/11] don't print summary of single crate install --- src/cargo/ops/cargo_install.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index ef2756ea7ee..b8e94270e3d 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -64,8 +64,8 @@ pub fn install(root: Option<&str>, let root = resolve_root(root, opts.config)?; let map = SourceConfigMap::new(opts.config)?; - if krates.is_empty() { - install_one(root, map, None, source_id, vers, opts, force) + if krates.len() <= 1 { + install_one(root, map, krates.into_iter().next(), source_id, vers, opts, force) } else { let mut success = vec![]; let mut errors = vec![]; @@ -97,6 +97,7 @@ fn install_one(root: Filesystem, static ALREADY_UPDATED: AtomicBool = ATOMIC_BOOL_INIT; let needs_update = !ALREADY_UPDATED.load(Ordering::SeqCst); + ALREADY_UPDATED.store(true, Ordering::SeqCst); let config = opts.config; @@ -124,8 +125,6 @@ fn install_one(root: Filesystem, specify alternate source".into()))? }; - ALREADY_UPDATED.store(true, Ordering::SeqCst); - let mut td_opt = None; let overidden_target_dir = if source_id.is_path() { None From daf4eabb775d99fa7bb6da847a512771e6f85bd6 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Wed, 12 Jul 2017 14:57:47 -0400 Subject: [PATCH 06/11] only print PATH warning once --- src/cargo/ops/cargo_install.rs | 49 +++++++++++++++++----------------- tests/install.rs | 3 +-- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index b8e94270e3d..2ca35802884 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -7,7 +7,6 @@ use std::io::prelude::*; use std::io::{self, SeekFrom}; use std::path::{Path, PathBuf}; use std::sync::Arc; -use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering}; use semver::Version; use tempdir::TempDir; @@ -65,26 +64,43 @@ pub fn install(root: Option<&str>, let map = SourceConfigMap::new(opts.config)?; if krates.len() <= 1 { - install_one(root, map, krates.into_iter().next(), source_id, vers, opts, force) + install_one(root.clone(), map, krates.into_iter().next(), source_id, vers, opts, + force, true)?; } else { let mut success = vec![]; let mut errors = vec![]; + let mut first = true; for krate in krates { let root = root.clone(); let map = map.clone(); - match install_one(root, map, Some(krate), source_id, vers, opts, force) { + match install_one(root, map, Some(krate), source_id, vers, opts, force, first) { Ok(()) => success.push(krate), Err(e) => errors.push(format!("{}: {}", krate, e)) } + first = false; } writeln!(io::stderr(), "\n\nSUMMARY\n\nSuccessfully installed: {}\n\nErrors:\n\t{}", success.join(", "), errors.join("\n\t"))?; + } - Ok(()) + // Print a warning that if this directory isn't in PATH that they won't be + // able to run these commands. + let dst = metadata(opts.config, &root)?.parent().join("bin"); + let path = env::var_os("PATH").unwrap_or(OsString::new()); + for path in env::split_paths(&path) { + if path == dst { + return Ok(()) + } } + + opts.config.shell().warn(&format!("be sure to add `{}` to your PATH to be \ + able to run the installed binaries", + dst.display()))?; + + Ok(()) } fn install_one(root: Filesystem, @@ -93,17 +109,14 @@ fn install_one(root: Filesystem, source_id: &SourceId, vers: Option<&str>, opts: &ops::CompileOptions, - force: bool) -> CargoResult<()> { - - static ALREADY_UPDATED: AtomicBool = ATOMIC_BOOL_INIT; - let needs_update = !ALREADY_UPDATED.load(Ordering::SeqCst); - ALREADY_UPDATED.store(true, Ordering::SeqCst); + force: bool, + is_first_install: bool) -> CargoResult<()> { let config = opts.config; let (pkg, source) = if source_id.is_git() { select_pkg(GitSource::new(source_id, config), - krate, vers, config, needs_update, + krate, vers, config, is_first_install, &mut |git| git.read_packages())? } else if source_id.is_path() { let path = source_id.url().to_file_path() @@ -115,11 +128,11 @@ fn install_one(root: Filesystem, specify an alternate source", path.display()) })?; select_pkg(PathSource::new(&path, source_id, config), - krate, vers, config, needs_update, + krate, vers, config, is_first_install, &mut |path| path.read_packages())? } else { select_pkg(map.load(source_id)?, - krate, vers, config, needs_update, + krate, vers, config, is_first_install, &mut |_| Err("must specify a crate to install from \ crates.io, or use --path or --git to \ specify alternate source".into()))? @@ -287,18 +300,6 @@ fn install_one(root: Filesystem, fs::remove_dir_all(&target_dir)?; } - // Print a warning that if this directory isn't in PATH that they won't be - // able to run these commands. - let path = env::var_os("PATH").unwrap_or(OsString::new()); - for path in env::split_paths(&path) { - if path == dst { - return Ok(()) - } - } - - config.shell().warn(&format!("be sure to add `{}` to your PATH to be \ - able to run the installed binaries", - dst.display()))?; Ok(()) } diff --git a/tests/install.rs b/tests/install.rs index b135922987c..ac4ec9b823b 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -67,13 +67,11 @@ fn multiple_pkgs() { [COMPILING] foo v0.0.1 [FINISHED] release [optimized] target(s) in [..] [INSTALLING] {home}[..]bin[..]foo[..] -warning: be sure to add `[..]` to your PATH to be able to run the installed binaries [DOWNLOADING] bar v0.0.1 (registry file://[..]) [INSTALLING] bar v0.0.1 [COMPILING] bar v0.0.1 [FINISHED] release [optimized] target(s) in [..] [INSTALLING] {home}[..]bin[..]bar[..] -warning: be sure to add `[..]` to your PATH to be able to run the installed binaries SUMMARY @@ -82,6 +80,7 @@ Successfully installed: foo, bar Errors: +warning: be sure to add `[..]` to your PATH to be able to run the installed binaries ", home = cargo_home().display()))); assert_that(cargo_home(), has_installed_exe("foo")); From aa201ab749ba2b4925d664de6606a54dc60de869 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Wed, 12 Jul 2017 15:31:30 -0400 Subject: [PATCH 07/11] use shell helpers to condense output --- src/cargo/ops/cargo_install.rs | 24 +++++++++++++++--------- tests/install.rs | 9 +-------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 2ca35802884..afde6edde1b 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -4,12 +4,13 @@ use std::env; use std::ffi::OsString; use std::fs::{self, File}; use std::io::prelude::*; -use std::io::{self, SeekFrom}; +use std::io::SeekFrom; use std::path::{Path, PathBuf}; use std::sync::Arc; use semver::Version; use tempdir::TempDir; +use termcolor::Color::Red; use toml; use core::{SourceId, Source, Package, Dependency, PackageIdSpec}; @@ -67,23 +68,28 @@ pub fn install(root: Option<&str>, install_one(root.clone(), map, krates.into_iter().next(), source_id, vers, opts, force, true)?; } else { - let mut success = vec![]; - let mut errors = vec![]; + let mut succeeded = vec![]; + let mut failed = vec![]; let mut first = true; for krate in krates { let root = root.clone(); let map = map.clone(); match install_one(root, map, Some(krate), source_id, vers, opts, force, first) { - Ok(()) => success.push(krate), - Err(e) => errors.push(format!("{}: {}", krate, e)) + Ok(()) => succeeded.push(krate), + Err(e) => { + opts.config.shell().error(e)?; + failed.push(krate) + } } first = false; } - writeln!(io::stderr(), - "\n\nSUMMARY\n\nSuccessfully installed: {}\n\nErrors:\n\t{}", - success.join(", "), - errors.join("\n\t"))?; + if !succeeded.is_empty() { + opts.config.shell().status("Successfully installed", succeeded.join(", "))?; + } + if !failed.is_empty() { + opts.config.shell().status_with_color("Failed to install", failed.join(", "), Red)?; + } } // Print a warning that if this directory isn't in PATH that they won't be diff --git a/tests/install.rs b/tests/install.rs index ac4ec9b823b..93dd71fc137 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -72,14 +72,7 @@ fn multiple_pkgs() { [COMPILING] bar v0.0.1 [FINISHED] release [optimized] target(s) in [..] [INSTALLING] {home}[..]bin[..]bar[..] - - -SUMMARY - -Successfully installed: foo, bar - -Errors: - +Successfully installed foo, bar warning: be sure to add `[..]` to your PATH to be able to run the installed binaries ", home = cargo_home().display()))); From f78fc7c266b028ac90442d2d085ba3e0d566d275 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Wed, 12 Jul 2017 16:28:33 -0400 Subject: [PATCH 08/11] add test for failing package --- src/cargo/ops/cargo_install.rs | 41 ++++++++++++++++++++-------------- tests/install.rs | 14 +++++++----- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index afde6edde1b..7d91d032d96 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -10,7 +10,6 @@ use std::sync::Arc; use semver::Version; use tempdir::TempDir; -use termcolor::Color::Red; use toml; use core::{SourceId, Source, Package, Dependency, PackageIdSpec}; @@ -64,9 +63,9 @@ pub fn install(root: Option<&str>, let root = resolve_root(root, opts.config)?; let map = SourceConfigMap::new(opts.config)?; - if krates.len() <= 1 { + let installed_anything = if krates.len() <= 1 { install_one(root.clone(), map, krates.into_iter().next(), source_id, vers, opts, - force, true)?; + force, true).is_ok() } else { let mut succeeded = vec![]; let mut failed = vec![]; @@ -84,27 +83,35 @@ pub fn install(root: Option<&str>, first = false; } + let mut summary = vec![]; if !succeeded.is_empty() { - opts.config.shell().status("Successfully installed", succeeded.join(", "))?; + summary.push(format!("Successfully installed {}!", succeeded.join(", "))); } if !failed.is_empty() { - opts.config.shell().status_with_color("Failed to install", failed.join(", "), Red)?; + summary.push(format!("Failed to install {} (see error(s) above).", failed.join(", "))); } - } + if !succeeded.is_empty() || !failed.is_empty() { + opts.config.shell().status("\nSummary:", summary.join(" "))?; + } + + !succeeded.is_empty() + }; - // Print a warning that if this directory isn't in PATH that they won't be - // able to run these commands. - let dst = metadata(opts.config, &root)?.parent().join("bin"); - let path = env::var_os("PATH").unwrap_or(OsString::new()); - for path in env::split_paths(&path) { - if path == dst { - return Ok(()) + if installed_anything { + // Print a warning that if this directory isn't in PATH that they won't be + // able to run these commands. + let dst = metadata(opts.config, &root)?.parent().join("bin"); + let path = env::var_os("PATH").unwrap_or(OsString::new()); + for path in env::split_paths(&path) { + if path == dst { + return Ok(()) + } } - } - opts.config.shell().warn(&format!("be sure to add `{}` to your PATH to be \ - able to run the installed binaries", - dst.display()))?; + opts.config.shell().warn(&format!("be sure to add `{}` to your PATH to be \ + able to run the installed binaries", + dst.display()))?; + } Ok(()) } diff --git a/tests/install.rs b/tests/install.rs index 93dd71fc137..bd4b20a63fc 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -57,9 +57,9 @@ warning: be sure to add `[..]` to your PATH to be able to run the installed bina #[test] fn multiple_pkgs() { pkg("foo", "0.0.1"); - pkg("bar", "0.0.1"); + pkg("bar", "0.0.2"); - assert_that(cargo_process("install").arg("foo").arg("bar"), + assert_that(cargo_process("install").args(&["foo", "bar", "baz"]), execs().with_status(0).with_stderr(&format!("\ [UPDATING] registry `[..]` [DOWNLOADING] foo v0.0.1 (registry file://[..]) @@ -67,12 +67,14 @@ fn multiple_pkgs() { [COMPILING] foo v0.0.1 [FINISHED] release [optimized] target(s) in [..] [INSTALLING] {home}[..]bin[..]foo[..] -[DOWNLOADING] bar v0.0.1 (registry file://[..]) -[INSTALLING] bar v0.0.1 -[COMPILING] bar v0.0.1 +[DOWNLOADING] bar v0.0.2 (registry file://[..]) +[INSTALLING] bar v0.0.2 +[COMPILING] bar v0.0.2 [FINISHED] release [optimized] target(s) in [..] [INSTALLING] {home}[..]bin[..]bar[..] -Successfully installed foo, bar +error: could not find `baz` in `registry [..]` + +Summary: Successfully installed foo, bar! Failed to install baz (see error(s) above). warning: be sure to add `[..]` to your PATH to be able to run the installed binaries ", home = cargo_home().display()))); From 1800c43cf6b949df4a0f73e0eaf0472e68a8dbc2 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Mon, 24 Jul 2017 19:25:46 -0400 Subject: [PATCH 09/11] don't continue if single install errors --- src/cargo/ops/cargo_install.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 7d91d032d96..74e294e8882 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -65,7 +65,8 @@ pub fn install(root: Option<&str>, let installed_anything = if krates.len() <= 1 { install_one(root.clone(), map, krates.into_iter().next(), source_id, vers, opts, - force, true).is_ok() + force, true)?; + true } else { let mut succeeded = vec![]; let mut failed = vec![]; From 1c7c88a854e7c3e4087b5212d6df641e6202be2e Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Tue, 25 Jul 2017 15:36:01 -0400 Subject: [PATCH 10/11] use top level err handling --- src/cargo/ops/cargo_install.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 74e294e8882..96a739ac68d 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -77,7 +77,7 @@ pub fn install(root: Option<&str>, match install_one(root, map, Some(krate), source_id, vers, opts, force, first) { Ok(()) => succeeded.push(krate), Err(e) => { - opts.config.shell().error(e)?; + ::handle_error(e, &mut opts.config.shell()); failed.push(krate) } } From ce2d69d2057a8aa10025c6be89c57f05fb5c4265 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Tue, 25 Jul 2017 15:36:42 -0400 Subject: [PATCH 11/11] nonzero exit code if some crates didn't install --- src/cargo/ops/cargo_install.rs | 10 +++++++--- tests/install.rs | 3 ++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 96a739ac68d..d8c8711b376 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -63,10 +63,10 @@ pub fn install(root: Option<&str>, let root = resolve_root(root, opts.config)?; let map = SourceConfigMap::new(opts.config)?; - let installed_anything = if krates.len() <= 1 { + let (installed_anything, scheduled_error) = if krates.len() <= 1 { install_one(root.clone(), map, krates.into_iter().next(), source_id, vers, opts, force, true)?; - true + (true, false) } else { let mut succeeded = vec![]; let mut failed = vec![]; @@ -95,7 +95,7 @@ pub fn install(root: Option<&str>, opts.config.shell().status("\nSummary:", summary.join(" "))?; } - !succeeded.is_empty() + (!succeeded.is_empty(), !failed.is_empty()) }; if installed_anything { @@ -114,6 +114,10 @@ pub fn install(root: Option<&str>, dst.display()))?; } + if scheduled_error { + bail!("some crates failed to install"); + } + Ok(()) } diff --git a/tests/install.rs b/tests/install.rs index bd4b20a63fc..9d385551b73 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -60,7 +60,7 @@ fn multiple_pkgs() { pkg("bar", "0.0.2"); assert_that(cargo_process("install").args(&["foo", "bar", "baz"]), - execs().with_status(0).with_stderr(&format!("\ + execs().with_status(101).with_stderr(&format!("\ [UPDATING] registry `[..]` [DOWNLOADING] foo v0.0.1 (registry file://[..]) [INSTALLING] foo v0.0.1 @@ -76,6 +76,7 @@ error: could not find `baz` in `registry [..]` Summary: Successfully installed foo, bar! Failed to install baz (see error(s) above). warning: be sure to add `[..]` to your PATH to be able to run the installed binaries +error: some crates failed to install ", home = cargo_home().display()))); assert_that(cargo_home(), has_installed_exe("foo"));