Skip to content

Improved error message for versions prefixed with v #15484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 4, 2025
6 changes: 6 additions & 0 deletions src/bin/cargo/commands/install.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks nice.

Would you mind following the atomic commit pattern the contributor guide suggests? Which we add a new test first than captures the current behavior, followed by the next commit containing both the fix and test change. That makes the behavior change more prominent.

Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,12 @@ fn parse_semver_flag(v: &str) -> CargoResult<VersionReq> {
.next()
.ok_or_else(|| format_err!("no version provided for the `--version` flag"))?;

if let Some(stripped) = v.strip_prefix("v") {
bail!(
"the version provided, `{v}` is not a valid SemVer requirement\n\n\
help: try changing the version to `{stripped}`",
)
}
let is_req = "<>=^~".contains(first) || v.contains('*');
if is_req {
match v.parse::<VersionReq>() {
Expand Down
15 changes: 15 additions & 0 deletions src/bin/cargo/commands/yank.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::command_prelude::*;

use anyhow::Context;
use cargo::ops;
use cargo_credential::Secret;

Expand Down Expand Up @@ -60,5 +61,19 @@ fn resolve_crate<'k>(
krate = Some(k);
version = Some(v);
}

if let Some(version) = version {
semver::Version::parse(version).with_context(|| {
if let Some(stripped) = version.strip_prefix("v") {
return format!(
"the version provided, `{version}` is not a \
valid SemVer version\n\n\
help: try changing the version to `{stripped}`",
);
}
format!("invalid version `{version}`")
})?;
}

Ok((krate, version))
}
12 changes: 10 additions & 2 deletions src/cargo/core/source_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,16 @@ impl SourceId {
version: semver::Version,
precise: &str,
) -> CargoResult<SourceId> {
let precise = semver::Version::parse(precise)
.with_context(|| format!("invalid version format for precise version `{precise}`"))?;
let precise = semver::Version::parse(precise).with_context(|| {
if let Some(stripped) = precise.strip_prefix("v") {
return format!(
"the version provided, `{precise}` is not a \
valid SemVer version\n\n\
help: try changing the version to `{stripped}`",
);
}
format!("invalid version format for precise version `{precise}`")
})?;

Ok(SourceId::wrap(SourceIdInner {
precise: Some(Precise::Updated {
Expand Down
12 changes: 10 additions & 2 deletions src/cargo/ops/cargo_add/crate_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,16 @@ impl CrateSpec {
package_name?;

if let Some(version) = version {
semver::VersionReq::parse(version)
.with_context(|| format!("invalid version requirement `{version}`"))?;
semver::VersionReq::parse(version).with_context(|| {
if let Some(stripped) = version.strip_prefix("v") {
return format!(
"the version provided, `{version}` is not a \
valid SemVer requirement\n\n\
help: changing the package to `{name}@{stripped}`",
);
}
format!("invalid version requirement `{version}`")
})?;
}

let id = Self {
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ mod path_base_unstable;
mod path_dev;
mod path_inferred_name;
mod path_inferred_name_conflicts_full_feature;
mod prefixed_v_in_version;
mod preserve_dep_std_table;
mod preserve_features_sorted;
mod preserve_features_table;
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/cargo_add/prefixed_v_in_version/in
26 changes: 26 additions & 0 deletions tests/testsuite/cargo_add/prefixed_v_in_version/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::current_dir;
use cargo_test_support::file;
use cargo_test_support::prelude::*;
use cargo_test_support::str;
use cargo_test_support::Project;

#[cargo_test]
fn case() {
cargo_test_support::registry::init();

let project = Project::from_template(current_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("add")
.arg_line("[email protected]")
.current_dir(cwd)
.assert()
.code(101)
.stdout_eq(str![""])
.stderr_eq(file!["stderr.term.svg"]);

assert_ui().subset_matches(current_dir!().join("out"), &project_root);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[workspace]

[package]
name = "cargo-list-test-fixture"
version = "0.0.0"
edition = "2015"
37 changes: 37 additions & 0 deletions tests/testsuite/cargo_add/prefixed_v_in_version/stderr.term.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 16 additions & 0 deletions tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2899,3 +2899,19 @@ fn dry_run_remove_orphan() {
// Ensure server is still installed after the dry run
assert_has_installed_exe(paths::cargo_home(), "server");
}

#[cargo_test]
fn prefixed_v_in_version() {
pkg("foo", "0.0.1");
cargo_process("install [email protected]")
.with_status(1)
.with_stderr_data(str![[r#"
[ERROR] invalid value '[email protected]' for '[CRATE[@<VER>]]...': the version provided, `v0.0.1` is not a valid SemVer requirement

[HELP] try changing the version to `0.0.1`

For more information, try '--help'.

"#]])
.run();
}
38 changes: 38 additions & 0 deletions tests/testsuite/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2709,3 +2709,41 @@ fn update_breaking_pre_release_upgrade() {
"#]])
.run();
}

#[cargo_test]
fn prefixed_v_in_version() {
Package::new("bar", "1.0.0").publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
edition = "2015"
authors = []

[dependencies]
bar = "1.0.0"
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("generate-lockfile").run();

Package::new("bar", "1.0.1").publish();
p.cargo("update bar --precise v1.0.1")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] the version provided, `v1.0.1` is not a valid SemVer version

[HELP] try changing the version to `1.0.1`

Caused by:
unexpected character 'v' while parsing major version number

"#]])
.run();
}
68 changes: 68 additions & 0 deletions tests/testsuite/yank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,71 @@ fn inline_and_explicit_version() {
"#]])
.run();
}

#[cargo_test]
fn bad_version() {
let registry = registry::init();
setup("foo", "0.0.1");

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
authors = []
license = "MIT"
description = "foo"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("yank foo@bar")
.replace_crates_io(registry.index_url())
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] invalid version `bar`

Caused by:
unexpected character 'b' while parsing major version number

"#]])
.run();
}

#[cargo_test]
fn prefixed_v_in_version() {
let registry = registry::init();
setup("foo", "0.0.1");

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
authors = []
license = "MIT"
description = "foo"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("yank [email protected]")
.replace_crates_io(registry.index_url())
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] the version provided, `v0.0.1` is not a valid SemVer version

[HELP] try changing the version to `0.0.1`

Caused by:
unexpected character 'v' while parsing major version number

"#]])
.run();
}