Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cargo install multiple crates #4216

Merged
merged 11 commits into from
Jul 28, 2017
10 changes: 5 additions & 5 deletions src/bin/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub struct Options {
flag_frozen: bool,
flag_locked: bool,

arg_crate: Option<String>,
arg_crate: Vec<String>,
flag_vers: Option<String>,

flag_git: Option<String>,
Expand All @@ -37,7 +37,7 @@ pub const USAGE: &'static str = "
Install a Rust binary
Usage:
cargo install [options] [<crate>]
cargo install [options] [<crate>...]
cargo install [options] --list
Specifying what crate to install:
Expand Down Expand Up @@ -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::<Vec<_>>();
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(())
}
102 changes: 80 additions & 22 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,37 +55,103 @@ impl Drop for Transaction {
}

pub fn install(root: Option<&str>,
krate: 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)?;

let installed_anything = if krates.len() <= 1 {
install_one(root.clone(), map, krates.into_iter().next(), source_id, vers, opts,
force, true)?;
true
} else {
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(()) => succeeded.push(krate),
Err(e) => {
opts.config.shell().error(e)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this'll want to use the top-level error handling functions in Cargo in the root src/cargo/lib.rs, there's a lot of contextual information in this error that the raw Display impl doesn't print out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use ::handle_error, I assume that's what you meant?

failed.push(krate)
}
}
first = false;
}

let mut summary = vec![];
if !succeeded.is_empty() {
summary.push(format!("Successfully installed {}!", succeeded.join(", ")));
}
if !failed.is_empty() {
summary.push(format!("Failed to install {} (see error(s) above).", failed.join(", ")));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this schedule an error to be returned at the top level, to ensure that cargo returns a nonzero exit status?

Copy link
Contributor Author

@durka durka Jul 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now if there are failures it looks like this:

$ target/debug/cargo install asdfasdf fdasfdsa
    Updating registry `https://github.com/rust-lang/crates.io-index`
error: could not find `asdfasdf` in `registry https://github.com/rust-lang/crates.io-index`
error: could not find `fdasfdsa` in `registry https://github.com/rust-lang/crates.io-index`

Summary: Failed to install asdfasdf, fdasfdsa (see error(s) above).
error: some crates failed to install

and exits with 101.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}
if !succeeded.is_empty() || !failed.is_empty() {
opts.config.shell().status("\nSummary:", summary.join(" "))?;
}

!succeeded.is_empty()
};

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()))?;
}

Ok(())
}

fn install_one(root: Filesystem,
map: SourceConfigMap,
krate: Option<&str>,
source_id: &SourceId,
vers: Option<&str>,
opts: &ops::CompileOptions,
force: bool,
is_first_install: bool) -> CargoResult<()> {

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, is_first_install,
&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 \
install from crates.io, or use --path or --git to \
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, is_first_install,
&mut |path| path.read_packages())?
} else {
select_pkg(map.load(source_id)?,
krate, vers, config,
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()))?
};


let mut td_opt = None;
let overidden_target_dir = if source_id.is_path() {
None
Expand Down Expand Up @@ -248,30 +314,22 @@ pub fn install(root: Option<&str>,
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(())
}

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<Vec<Package>>)
-> CargoResult<(Package, Box<Source + 'a>)>
where T: Source + 'a
{
source.update()?;
if needs_update {
source.update()?;
}

match name {
Some(name) => {
let vers = match vers {
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/sources/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, SourceConfig>,
id2name: HashMap<SourceId, String>,
Expand All @@ -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
Expand Down
41 changes: 41 additions & 0 deletions tests/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,47 @@ warning: be sure to add `[..]` to your PATH to be able to run the installed bina
assert_that(cargo_home(), is_not(has_installed_exe("foo")));
}

#[test]
fn multiple_pkgs() {
pkg("foo", "0.0.1");
pkg("bar", "0.0.2");

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://[..])
[INSTALLING] foo v0.0.1
[COMPILING] foo v0.0.1
[FINISHED] release [optimized] target(s) in [..]
[INSTALLING] {home}[..]bin[..]foo[..]
[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[..]
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())));
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_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[..]
",
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]
fn pick_max_version() {
pkg("foo", "0.0.1");
Expand Down