diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index 05d348323..f07af83ea 100644 --- a/src/bin/cargo/commands/install.rs +++ b/src/bin/cargo/commands/install.rs @@ -10,11 +10,7 @@ use cargo_util::paths; pub fn cli() -> Command { subcommand("install") .about("Install a Rust binary. Default location is $HOME/.cargo/bin") - .arg( - Arg::new("crate") - .value_parser(clap::builder::NonEmptyStringValueParser::new()) - .num_args(0..), - ) + .arg(Arg::new("crate").value_parser(parse_crate).num_args(0..)) .arg( opt("version", "Specify a version to install") .alias("vers") @@ -104,9 +100,10 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let version = args.get_one::("version").map(String::as_str); let krates = args - .get_many::("crate") + .get_many::("crate") .unwrap_or_default() - .map(|k| resolve_crate(k, version)) + .cloned() + .map(|(krate, local_version)| resolve_crate(krate, local_version, version)) .collect::>>()?; for (crate_name, _) in krates.iter() { @@ -190,20 +187,42 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { Ok(()) } -fn resolve_crate<'k>( - mut krate: &'k str, - mut version: Option<&'k str>, -) -> crate::CargoResult<(&'k str, Option<&'k str>)> { - if let Some((k, v)) = krate.split_once('@') { - if version.is_some() { - anyhow::bail!("cannot specify both `@{v}` and `--version`"); - } +type CrateVersion = (String, Option); + +fn parse_crate(krate: &str) -> crate::CargoResult { + let (krate, version) = if let Some((k, v)) = krate.split_once('@') { if k.is_empty() { // by convention, arguments starting with `@` are response files - anyhow::bail!("missing crate name for `@{v}`"); + anyhow::bail!("missing crate name before '@'"); } - krate = k; - version = Some(v); + let krate = k.to_owned(); + let version = Some(v.to_owned()); + (krate, version) + } else { + let krate = krate.to_owned(); + let version = None; + (krate, version) + }; + + if krate.is_empty() { + anyhow::bail!("crate name is empty"); } + + Ok((krate, version)) +} + +fn resolve_crate( + krate: String, + local_version: Option, + version: Option<&str>, +) -> crate::CargoResult { + let version = match (local_version, version) { + (Some(l), Some(g)) => { + anyhow::bail!("cannot specify both `@{l}` and `--version {g}`"); + } + (Some(l), None) => Some(l), + (None, Some(g)) => Some(g.to_owned()), + (None, None) => None, + }; Ok((krate, version)) } diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 8ddfa4fab..b9eb5afca 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -604,7 +604,7 @@ Consider enabling some of the needed features by passing, e.g., `--features=\"{e pub fn install( config: &Config, root: Option<&str>, - krates: Vec<(&str, Option<&str>)>, + krates: Vec<(String, Option)>, source_id: SourceId, from_cwd: bool, opts: &ops::CompileOptions, @@ -617,9 +617,9 @@ pub fn install( let (installed_anything, scheduled_error) = if krates.len() <= 1 { let (krate, vers) = krates - .into_iter() + .iter() .next() - .map(|(k, v)| (Some(k), v)) + .map(|(k, v)| (Some(k.as_str()), v.as_deref())) .unwrap_or((None, None)); let installable_pkg = InstallablePackage::new( config, root, map, krate, source_id, from_cwd, vers, opts, force, no_track, true, @@ -637,7 +637,7 @@ pub fn install( let mut did_update = false; let pkgs_to_install: Vec<_> = krates - .into_iter() + .iter() .filter_map(|(krate, vers)| { let root = root.clone(); let map = map.clone(); @@ -645,10 +645,10 @@ pub fn install( config, root, map, - Some(krate), + Some(krate.as_str()), source_id, from_cwd, - vers, + vers.as_deref(), opts, force, no_track, @@ -660,12 +660,12 @@ pub fn install( } Ok(None) => { // Already installed - succeeded.push(krate); + succeeded.push(krate.as_str()); None } Err(e) => { crate::display_error(&e, &mut config.shell()); - failed.push(krate); + failed.push(krate.as_str()); // We assume an update was performed if we got an error. did_update = true; None diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 0c7fc5037..c3482ee61 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -1600,8 +1600,13 @@ fn inline_version_without_name() { pkg("foo", "0.1.2"); cargo_process("install @0.1.1") - .with_status(101) - .with_stderr("error: missing crate name for `@0.1.1`") + .with_status(1) + .with_stderr( + "error: invalid value '@0.1.1' for '[crate]...': missing crate name before '@' + +For more information, try '--help'. +", + ) .run(); } @@ -1612,7 +1617,7 @@ fn inline_and_explicit_version() { cargo_process("install foo@0.1.1 --version 0.1.1") .with_status(101) - .with_stderr("error: cannot specify both `@0.1.1` and `--version`") + .with_stderr("error: cannot specify both `@0.1.1` and `--version 0.1.1`") .run(); } @@ -1827,7 +1832,7 @@ fn install_empty_argument() { cargo_process("install") .arg("") .with_status(1) - .with_stderr_contains("[ERROR] a value is required for '[crate]...' but none was supplied") + .with_stderr_contains("[ERROR] invalid value '' for '[crate]...': crate name is empty") .run(); }