development/cargo
development/cargo
1
0
Fork 0
mirror of https://github.com/rust-lang/cargo synced 2024-10-12 10:42:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #12547 - epage:install, r=weihanglo

Browse source 
refactor(install): Move value parsing to clap

### What does this PR try to resolve?

Originally, I thought this was going to help some of my other work but it won't.  Instead I decided to finish and post this PR in case there was interest since `@weihanglo` expressed interest in using `Arg::value_parser` more and this demonstrates that.

### How should we test and review this PR?

A commit at a time.  There seemed to be existing tests for some of the error changes.
This commit is contained in:
bors 2023-08-24 12:45:03 +00:00
parent 2a159aaa67 937932520b
commit 35814255a1
4 changed files with 122 additions and 93 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

108
src/bin/cargo/commands/install.rs
View file

@ -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::<String>("version").map(String::as_str);
let version = args.get_one::<VersionReq>("version");
let krates = args
.get_many::<String>("crate")
.get_many::<CrateVersion>("crate")
.unwrap_or_default()
.map(|k| resolve_crate(k, version))
.cloned()
.map(|(krate, local_version)| resolve_crate(krate, local_version, version))
.collect::<crate::CargoResult<Vec<_>>>()?;
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<VersionReq>);
fn parse_crate(krate: &str) -> crate::CargoResult<CrateVersion> {
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<VersionReq> {
// 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::<VersionReq>() {
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::<VersionReq>().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<VersionReq>,
version: Option<&VersionReq>,
) -> crate::CargoResult<CrateVersion> {
let version = match (local_version, version) {
(Some(_), Some(_)) => {
anyhow::bail!("cannot specify both `@<VERSION>` and `--version <VERSION>`");
}
(Some(l), None) => Some(l),
(None, Some(g)) => Some(g.to_owned()),
(None, None) => None,
};
Ok((krate, version))
}

86
src/cargo/ops/cargo_install.rs
View file

@ -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<VersionReq>,
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<Option<InstallablePackage<'cfg, 'a>>> {
) -> CargoResult<Option<Self>> {
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<VersionReq>)>,
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<VersionReq> {
// 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::<VersionReq>() {
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::<VersionReq>().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)?;

13
tests/testsuite/install.rs
View file

@ -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 `@<VERSION>` and `--version <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();
}

8
tests/testsuite/install_upgrade.rs
View file

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