From ac25dc7158f4a4a0b25df208a37fae6594f0cab3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 20 Apr 2022 09:43:05 -0500 Subject: [PATCH 1/3] refactor(install): Allow per-crate versions in the API --- src/bin/cargo/commands/install.rs | 4 ++-- src/cargo/ops/cargo_install.rs | 22 ++++++++-------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index c5564bfbba5..f8410e62517 100644 --- a/src/bin/cargo/commands/install.rs +++ b/src/bin/cargo/commands/install.rs @@ -97,9 +97,11 @@ 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.value_of("version"); let krates = args .values_of("crate") .unwrap_or_default() + .map(|k| (k, version)) .collect::>(); let mut from_cwd = false; @@ -129,7 +131,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { SourceId::crates_io(config)? }; - let version = args.value_of("version"); let root = args.value_of("root"); // We only provide workspace information for local crate installation from @@ -166,7 +167,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { krates, source, from_cwd, - version, &compile_opts, args.is_present("force"), args.is_present("no-track"), diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 354ea1bcfbc..0fea201b044 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -556,10 +556,9 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> { pub fn install( config: &Config, root: Option<&str>, - krates: Vec<&str>, + krates: Vec<(&str, Option<&str>)>, source_id: SourceId, from_cwd: bool, - vers: Option<&str>, opts: &ops::CompileOptions, force: bool, no_track: bool, @@ -569,18 +568,13 @@ pub fn install( let map = SourceConfigMap::new(config)?; let (installed_anything, scheduled_error) = if krates.len() <= 1 { + let (krate, vers) = krates + .into_iter() + .next() + .map(|(k, v)| (Some(k), v)) + .unwrap_or((None, None)); let installable_pkg = InstallablePackage::new( - config, - root, - map, - krates.into_iter().next(), - source_id, - from_cwd, - vers, - opts, - force, - no_track, - true, + config, root, map, krate, source_id, from_cwd, vers, opts, force, no_track, true, )?; let mut installed_anything = true; if let Some(installable_pkg) = installable_pkg { @@ -596,7 +590,7 @@ pub fn install( let pkgs_to_install: Vec<_> = krates .into_iter() - .filter_map(|krate| { + .filter_map(|(krate, vers)| { let root = root.clone(); let map = map.clone(); match InstallablePackage::new( From 07681341da584e97b9d8fe33b28d182fa9ffe503 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 20 Apr 2022 09:53:35 -0500 Subject: [PATCH 2/3] feat(install): Support `foo@version` like cargo-add In #10472, cargo-add was merged with support for an inline version syntax of `foo@version`. That also served as the change proposal for extending that syntax to `cargo install` for convinience and consistency. While both commands are specifying a version-req, `cargo-install` has an implicit-but-required `=` operand while `cargo-add` allows any operand. This doesn't use the full `pkgid` syntax because that allows syntax that is unsupported here. This doesn't use `cargo-add`s parser because that is for version reqs. I held off on reusing the parser from `cargo-yank` because they had different type system needs and the level of duplication didn't seem worth it (see Rule of Three). --- src/bin/cargo/commands/install.rs | 22 ++++++++++++-- tests/testsuite/install.rs | 49 ++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index f8410e62517..82dc31fc158 100644 --- a/src/bin/cargo/commands/install.rs +++ b/src/bin/cargo/commands/install.rs @@ -101,8 +101,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let krates = args .values_of("crate") .unwrap_or_default() - .map(|k| (k, version)) - .collect::>(); + .map(|k| resolve_crate(k, version)) + .collect::>>()?; let mut from_cwd = false; @@ -174,3 +174,21 @@ 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`"); + } + if k.is_empty() { + // by convention, arguments starting with `@` are response files + anyhow::bail!("missing crate name for `@{v}`"); + } + krate = k; + version = Some(v); + } + Ok((krate, version)) +} diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index e09e88a70b5..113e45a9c85 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -1382,7 +1382,7 @@ fn vers_precise() { } #[cargo_test] -fn version_too() { +fn version_precise() { pkg("foo", "0.1.1"); pkg("foo", "0.1.2"); @@ -1391,6 +1391,53 @@ fn version_too() { .run(); } +#[cargo_test] +fn inline_version_precise() { + pkg("foo", "0.1.1"); + pkg("foo", "0.1.2"); + + cargo_process("install foo@0.1.1") + .with_stderr_contains("[DOWNLOADED] foo v0.1.1 (registry [..])") + .run(); +} + +#[cargo_test] +fn inline_version_multiple() { + pkg("foo", "0.1.0"); + pkg("foo", "0.1.1"); + pkg("foo", "0.1.2"); + pkg("bar", "0.2.0"); + pkg("bar", "0.2.1"); + pkg("bar", "0.2.2"); + + cargo_process("install foo@0.1.1 bar@0.2.1") + .with_stderr_contains("[DOWNLOADED] foo v0.1.1 (registry [..])") + .with_stderr_contains("[DOWNLOADED] bar v0.2.1 (registry [..])") + .run(); +} + +#[cargo_test] +fn inline_version_without_name() { + pkg("foo", "0.1.1"); + pkg("foo", "0.1.2"); + + cargo_process("install @0.1.1") + .with_status(101) + .with_stderr("error: missing crate name for `@0.1.1`") + .run(); +} + +#[cargo_test] +fn inline_and_explicit_version() { + pkg("foo", "0.1.1"); + pkg("foo", "0.1.2"); + + 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`") + .run(); +} + #[cargo_test] fn not_both_vers_and_version() { pkg("foo", "0.1.1"); From f063c65c1b75ef886c8820950bf71ceef106a3e7 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 20 Apr 2022 10:31:40 -0500 Subject: [PATCH 3/3] doc(install): Tell users about `foo@version` syntax --- src/doc/man/cargo-install.md | 2 +- src/doc/man/generated_txt/cargo-install.txt | 2 +- src/doc/src/commands/cargo-install.md | 2 +- src/etc/man/cargo-install.1 | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/doc/man/cargo-install.md b/src/doc/man/cargo-install.md index 320f2bf5492..118e3efdf03 100644 --- a/src/doc/man/cargo-install.md +++ b/src/doc/man/cargo-install.md @@ -8,7 +8,7 @@ cargo-install - Build and install a Rust binary ## SYNOPSIS -`cargo install` [_options_] _crate_...\ +`cargo install` [_options_] _crate_[@_version_]...\ `cargo install` [_options_] `--path` _path_\ `cargo install` [_options_] `--git` _url_ [_crate_...]\ `cargo install` [_options_] `--list` diff --git a/src/doc/man/generated_txt/cargo-install.txt b/src/doc/man/generated_txt/cargo-install.txt index 4bcd7d9c40e..6132721e7f7 100644 --- a/src/doc/man/generated_txt/cargo-install.txt +++ b/src/doc/man/generated_txt/cargo-install.txt @@ -4,7 +4,7 @@ NAME cargo-install - Build and install a Rust binary SYNOPSIS - cargo install [options] crate... + cargo install [options] crate[@version]... cargo install [options] --path path cargo install [options] --git url [crate...] cargo install [options] --list diff --git a/src/doc/src/commands/cargo-install.md b/src/doc/src/commands/cargo-install.md index f4c2c6af0d4..68ddf93da84 100644 --- a/src/doc/src/commands/cargo-install.md +++ b/src/doc/src/commands/cargo-install.md @@ -8,7 +8,7 @@ cargo-install - Build and install a Rust binary ## SYNOPSIS -`cargo install` [_options_] _crate_...\ +`cargo install` [_options_] _crate_[@_version_]...\ `cargo install` [_options_] `--path` _path_\ `cargo install` [_options_] `--git` _url_ [_crate_...]\ `cargo install` [_options_] `--list` diff --git a/src/etc/man/cargo-install.1 b/src/etc/man/cargo-install.1 index 367fcfdd158..d4785c186a9 100644 --- a/src/etc/man/cargo-install.1 +++ b/src/etc/man/cargo-install.1 @@ -6,7 +6,7 @@ .SH "NAME" cargo\-install \- Build and install a Rust binary .SH "SYNOPSIS" -\fBcargo install\fR [\fIoptions\fR] \fIcrate\fR\&... +\fBcargo install\fR [\fIoptions\fR] \fIcrate\fR[@\fIversion\fR]\&... .br \fBcargo install\fR [\fIoptions\fR] \fB\-\-path\fR \fIpath\fR .br