Auto merge of #4216 - durka:install-multi, r=alexcrichton

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.

Author: bors

src/bin/install.rs

@@ -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>,
@@ -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:
@@ -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(())
 }

src/cargo/ops/cargo_install.rs

@@ -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
@@ -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 {

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<String, SourceConfig>,
     id2name: HashMap<SourceId, String>,
@@ -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

tests/install.rs

@@ -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");