diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index 05d3483232d..8e3062253b1 100644 --- a/src/bin/cargo/commands/install.rs +++ b/src/bin/cargo/commands/install.rs @@ -1,24 +1,27 @@ use crate::command_prelude::*; use anyhow::anyhow; +use anyhow::bail; +use anyhow::format_err; use cargo::core::{GitReference, SourceId, Workspace}; use cargo::ops; use cargo::util::IntoUrl; +use cargo::util::ToSemver; +use cargo::util::VersionReqExt; +use cargo::CargoResult; +use semver::VersionReq; 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") .value_name("VERSION") + .value_parser(parse_semver_flag) .requires("crate"), ) .arg( @@ -102,11 +105,12 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { // but not `Config::reload_rooted_at` which is always cwd) let path = path.map(|p| paths::normalize_path(&p)); - let version = args.get_one::("version").map(String::as_str); + let version = args.get_one::("version"); 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 +194,86 @@ 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(parse_semver_flag(v)?); + (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)) +} + +/// Parses x.y.z as if it were =x.y.z, and gives CLI-specific error messages in the case of invalid +/// values. +fn parse_semver_flag(v: &str) -> CargoResult { + // If the version begins with character <, >, =, ^, ~ parse it as a + // version range, otherwise parse it as a specific version + let first = v + .chars() + .next() + .ok_or_else(|| format_err!("no version provided for the `--version` flag"))?; + + let is_req = "<>=^~".contains(first) || v.contains('*'); + if is_req { + match v.parse::() { + Ok(v) => Ok(v), + Err(_) => bail!( + "the `--version` provided, `{}`, is \ + not a valid semver version requirement\n\n\ + Please have a look at \ + https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html \ + for the correct format", + v + ), + } + } else { + match v.to_semver() { + Ok(v) => Ok(VersionReq::exact(&v)), + Err(e) => { + let mut msg = e.to_string(); + + // If it is not a valid version but it is a valid version + // requirement, add a note to the warning + if v.parse::().is_ok() { + msg.push_str(&format!( + "\n\n tip: if you want to specify semver range, \ + add an explicit qualifier, like '^{}'", + v + )); + } + bail!(msg); + } + } + } +} + +fn resolve_crate( + krate: String, + local_version: Option, + version: Option<&VersionReq>, +) -> crate::CargoResult { + let version = match (local_version, version) { + (Some(_), Some(_)) => { + anyhow::bail!("cannot specify both `@` and `--version `"); + } + (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 8ddfa4fab3d..83950b9617c 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -11,10 +11,10 @@ use crate::ops::{common_for_install_and_uninstall::*, FilterRule}; use crate::ops::{CompileFilter, Packages}; use crate::sources::{GitSource, PathSource, SourceConfigMap}; use crate::util::errors::CargoResult; -use crate::util::{Config, Filesystem, Rustc, ToSemver, VersionReqExt}; +use crate::util::{Config, Filesystem, Rustc}; use crate::{drop_println, ops}; -use anyhow::{bail, format_err, Context as _}; +use anyhow::{bail, Context as _}; use cargo_util::paths; use itertools::Itertools; use semver::VersionReq; @@ -38,12 +38,12 @@ impl Drop for Transaction { } } -struct InstallablePackage<'cfg, 'a> { +struct InstallablePackage<'cfg> { config: &'cfg Config, opts: ops::CompileOptions, root: Filesystem, source_id: SourceId, - vers: Option<&'a str>, + vers: Option, force: bool, no_track: bool, @@ -53,7 +53,7 @@ struct InstallablePackage<'cfg, 'a> { target: String, } -impl<'cfg, 'a> InstallablePackage<'cfg, 'a> { +impl<'cfg> InstallablePackage<'cfg> { // Returns pkg to install. None if pkg is already installed pub fn new( config: &'cfg Config, @@ -62,12 +62,12 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> { krate: Option<&str>, source_id: SourceId, from_cwd: bool, - vers: Option<&'a str>, - original_opts: &'a ops::CompileOptions, + vers: Option<&VersionReq>, + original_opts: &ops::CompileOptions, force: bool, no_track: bool, needs_update_if_source_is_index: bool, - ) -> CargoResult>> { + ) -> CargoResult> { if let Some(name) = krate { if name == "." { bail!( @@ -82,8 +82,8 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> { let pkg = { let dep = { if let Some(krate) = krate { - let vers = if let Some(vers_flag) = vers { - Some(parse_semver_flag(vers_flag)?.to_string()) + let vers = if let Some(vers) = vers { + Some(vers.to_string()) } else if source_id.is_registry() { // Avoid pre-release versions from crate.io // unless explicitly asked for @@ -234,7 +234,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> { opts, root, source_id, - vers, + vers: vers.cloned(), force, no_track, @@ -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_ref())) .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_ref(), 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 @@ -805,54 +805,6 @@ fn make_ws_rustc_target<'cfg>( Ok((ws, rustc, target)) } -/// Parses x.y.z as if it were =x.y.z, and gives CLI-specific error messages in the case of invalid -/// values. -fn parse_semver_flag(v: &str) -> CargoResult { - // If the version begins with character <, >, =, ^, ~ parse it as a - // version range, otherwise parse it as a specific version - let first = v - .chars() - .next() - .ok_or_else(|| format_err!("no version provided for the `--version` flag"))?; - - let is_req = "<>=^~".contains(first) || v.contains('*'); - if is_req { - match v.parse::() { - Ok(v) => Ok(v), - Err(_) => bail!( - "the `--version` provided, `{}`, is \ - not a valid semver version requirement\n\n\ - Please have a look at \ - https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html \ - for the correct format", - v - ), - } - } else { - match v.to_semver() { - Ok(v) => Ok(VersionReq::exact(&v)), - Err(e) => { - let mut msg = format!( - "the `--version` provided, `{}`, is \ - not a valid semver version: {}\n", - v, e - ); - - // If it is not a valid version but it is a valid version - // requirement, add a note to the warning - if v.parse::().is_ok() { - msg.push_str(&format!( - "\nif you want to specify semver range, \ - add an explicit qualifier, like ^{}", - v - )); - } - bail!(msg); - } - } - } -} - /// Display a list of installed binaries. pub fn install_list(dst: Option<&str>, config: &Config) -> CargoResult<()> { let root = resolve_root(dst, config)?; diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 0c7fc5037e4..a3d456ace0d 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 `@` and `--version `") .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(); } diff --git a/tests/testsuite/install_upgrade.rs b/tests/testsuite/install_upgrade.rs index ae641ba98d2..100fe1fde11 100644 --- a/tests/testsuite/install_upgrade.rs +++ b/tests/testsuite/install_upgrade.rs @@ -230,12 +230,14 @@ fn ambiguous_version_no_longer_allowed() { cargo_process("install foo --version=1.0") .with_stderr( "\ -[ERROR] the `--version` provided, `1.0`, is not a valid semver version: cannot parse '1.0' as a semver +[ERROR] invalid value '1.0' for '--version ': cannot parse '1.0' as a semver -if you want to specify semver range, add an explicit qualifier, like ^1.0 + tip: if you want to specify semver range, add an explicit qualifier, like '^1.0' + +For more information, try '--help'. ", ) - .with_status(101) + .with_status(1) .run(); }