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(())
}
106 changes: 84 additions & 22 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,37 +55,107 @@ 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, scheduled_error) = if krates.len() <= 1 {
install_one(root.clone(), map, krates.into_iter().next(), source_id, vers, opts,
force, true)?;
(true, false)
} 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) => {
::handle_error(e, &mut opts.config.shell());
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(), !failed.is_empty())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Starting to accumulate a slightly worrying number of boolean flags here. :/

};

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

if scheduled_error {
bail!("some crates failed to install");
}

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 +318,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
42 changes: 42 additions & 0 deletions tests/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,48 @@ 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(101).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
error: some crates failed to install
",
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