Skip to content

Commit 6cba807

Browse files
authored
Improved error message for versions prefixed with v (#15484)
### What does this PR try to resolve? - Added an error message when version in `CRATE[@<VER>]` or `--version <VER>` starts with 'v' for `install`, `add`, `yank` and `update --precise <VER>` - Check if version is valid in `cargo yank` Fixes #12331 ### How should we test and review this PR? Added tests for each subcommand
2 parents 4b96b30 + 2ba61b9 commit 6cba807

File tree

12 files changed

+234
-4
lines changed

12 files changed

+234
-4
lines changed

src/bin/cargo/commands/install.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,12 @@ fn parse_semver_flag(v: &str) -> CargoResult<VersionReq> {
286286
.next()
287287
.ok_or_else(|| format_err!("no version provided for the `--version` flag"))?;
288288

289+
if let Some(stripped) = v.strip_prefix("v") {
290+
bail!(
291+
"the version provided, `{v}` is not a valid SemVer requirement\n\n\
292+
help: try changing the version to `{stripped}`",
293+
)
294+
}
289295
let is_req = "<>=^~".contains(first) || v.contains('*');
290296
if is_req {
291297
match v.parse::<VersionReq>() {

src/bin/cargo/commands/yank.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::command_prelude::*;
22

3+
use anyhow::Context;
34
use cargo::ops;
45
use cargo_credential::Secret;
56

@@ -60,5 +61,19 @@ fn resolve_crate<'k>(
6061
krate = Some(k);
6162
version = Some(v);
6263
}
64+
65+
if let Some(version) = version {
66+
semver::Version::parse(version).with_context(|| {
67+
if let Some(stripped) = version.strip_prefix("v") {
68+
return format!(
69+
"the version provided, `{version}` is not a \
70+
valid SemVer version\n\n\
71+
help: try changing the version to `{stripped}`",
72+
);
73+
}
74+
format!("invalid version `{version}`")
75+
})?;
76+
}
77+
6378
Ok((krate, version))
6479
}

src/cargo/core/source_id.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -523,8 +523,16 @@ impl SourceId {
523523
version: semver::Version,
524524
precise: &str,
525525
) -> CargoResult<SourceId> {
526-
let precise = semver::Version::parse(precise)
527-
.with_context(|| format!("invalid version format for precise version `{precise}`"))?;
526+
let precise = semver::Version::parse(precise).with_context(|| {
527+
if let Some(stripped) = precise.strip_prefix("v") {
528+
return format!(
529+
"the version provided, `{precise}` is not a \
530+
valid SemVer version\n\n\
531+
help: try changing the version to `{stripped}`",
532+
);
533+
}
534+
format!("invalid version format for precise version `{precise}`")
535+
})?;
528536

529537
Ok(SourceId::wrap(SourceIdInner {
530538
precise: Some(Precise::Updated {

src/cargo/ops/cargo_add/crate_spec.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,16 @@ impl CrateSpec {
4747
package_name?;
4848

4949
if let Some(version) = version {
50-
semver::VersionReq::parse(version)
51-
.with_context(|| format!("invalid version requirement `{version}`"))?;
50+
semver::VersionReq::parse(version).with_context(|| {
51+
if let Some(stripped) = version.strip_prefix("v") {
52+
return format!(
53+
"the version provided, `{version}` is not a \
54+
valid SemVer requirement\n\n\
55+
help: changing the package to `{name}@{stripped}`",
56+
);
57+
}
58+
format!("invalid version requirement `{version}`")
59+
})?;
5260
}
5361

5462
let id = Self {

tests/testsuite/cargo_add/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ mod path_base_unstable;
126126
mod path_dev;
127127
mod path_inferred_name;
128128
mod path_inferred_name_conflicts_full_feature;
129+
mod prefixed_v_in_version;
129130
mod preserve_dep_std_table;
130131
mod preserve_features_sorted;
131132
mod preserve_features_table;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../add-basic.in
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
use cargo_test_support::compare::assert_ui;
2+
use cargo_test_support::current_dir;
3+
use cargo_test_support::file;
4+
use cargo_test_support::prelude::*;
5+
use cargo_test_support::str;
6+
use cargo_test_support::Project;
7+
8+
#[cargo_test]
9+
fn case() {
10+
cargo_test_support::registry::init();
11+
12+
let project = Project::from_template(current_dir!().join("in"));
13+
let project_root = project.root();
14+
let cwd = &project_root;
15+
16+
snapbox::cmd::Command::cargo_ui()
17+
.arg("add")
18+
.arg_line("foo@v0.0.1")
19+
.current_dir(cwd)
20+
.assert()
21+
.code(101)
22+
.stdout_eq(str![""])
23+
.stderr_eq(file!["stderr.term.svg"]);
24+
25+
assert_ui().subset_matches(current_dir!().join("out"), &project_root);
26+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
[workspace]
2+
3+
[package]
4+
name = "cargo-list-test-fixture"
5+
version = "0.0.0"
6+
edition = "2015"
Lines changed: 37 additions & 0 deletions
Loading

tests/testsuite/install.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2899,3 +2899,19 @@ fn dry_run_remove_orphan() {
28992899
// Ensure server is still installed after the dry run
29002900
assert_has_installed_exe(paths::cargo_home(), "server");
29012901
}
2902+
2903+
#[cargo_test]
2904+
fn prefixed_v_in_version() {
2905+
pkg("foo", "0.0.1");
2906+
cargo_process("install foo@v0.0.1")
2907+
.with_status(1)
2908+
.with_stderr_data(str![[r#"
2909+
[ERROR] invalid value 'foo@v0.0.1' for '[CRATE[@<VER>]]...': the version provided, `v0.0.1` is not a valid SemVer requirement
2910+
2911+
[HELP] try changing the version to `0.0.1`
2912+
2913+
For more information, try '--help'.
2914+
2915+
"#]])
2916+
.run();
2917+
}

0 commit comments

Comments
 (0)