Skip to content

Commit

Permalink
Auto merge of #4216 - durka:install-multi, r=alexcrichton
Browse files Browse the repository at this point in the history
cargo install multiple crates

rust-lang/rustup#986 for `cargo install`

Revives #2601 @pwoolcoc, replaces #3075 @esclear, closes #2585 @kindlychung @cyplo

Avoids the sticking point of the previous two PRs (multiple registry updates) by threading through a first-run boolean flag to decide whether `select_pkg` needs to call `source.update()`.

There is still the issue that flags such as `--git` and `--vers` are "global" to the multiple packages you may be installing. The workaround is just to run `cargo install` separately. In the future we could add syntax like `cargo install foo=1.0 bar=2.5 quux=git://github.com/durka/quux#dev-branch` or something.
  • Loading branch information
bors committed Jul 28, 2017
2 parents 76148e9 ce2d69d commit 3751e68
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 27 deletions.
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(", ")));
}
if !succeeded.is_empty() || !failed.is_empty() {
opts.config.shell().status("\nSummary:", summary.join(" "))?;
}

(!succeeded.is_empty(), !failed.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()))?;
}

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

0 comments on commit 3751e68

Please sign in to comment.