Skip to content

Commit

Permalink
fix: Report more detailed semver errors
Browse files Browse the repository at this point in the history
For `cargo install` we'll now show a more specific parse error for
semver, much like other parts of cargo.

This came out of my work on rust-lang#12801.  I was looking at what might be
appropriate to put in a `cargo-util-semver` crate and realized we have
the `ToSemver` trait that exists but doesn't do much, so I dropped it.
  • Loading branch information
epage committed Nov 6, 2023
1 parent a9f6393 commit 2d99bdd
Show file tree
Hide file tree
Showing 11 changed files with 15 additions and 58 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion crates/xtask-bump-check/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ cargo.workspace = true
cargo-util.workspace = true
clap.workspace = true
git2.workspace = true
tracing.workspace = true
semver.workspace = true
tracing-subscriber.workspace = true
tracing.workspace = true
3 changes: 1 addition & 2 deletions crates/xtask-bump-check/src/xtask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use cargo::core::Workspace;
use cargo::sources::source::QueryKind;
use cargo::util::cache_lock::CacheLockMode;
use cargo::util::command_prelude::*;
use cargo::util::ToSemver;
use cargo::CargoResult;
use cargo_util::ProcessBuilder;

Expand Down Expand Up @@ -277,7 +276,7 @@ fn beta_and_stable_branch(repo: &git2::Repository) -> CargoResult<[git2::Branch<
tracing::trace!("branch `{name}` is not in the format of `<remote>/rust-<semver>`");
continue;
};
let Ok(version) = version.to_semver() else {
let Ok(version) = version.parse::<semver::Version>() else {
tracing::trace!("branch `{name}` is not a valid semver: `{version}`");
continue;
};
Expand Down
3 changes: 1 addition & 2 deletions src/bin/cargo/commands/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ 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 itertools::Itertools;
Expand Down Expand Up @@ -264,7 +263,7 @@ fn parse_semver_flag(v: &str) -> CargoResult<VersionReq> {
),
}
} else {
match v.to_semver() {
match v.trim().parse() {
Ok(v) => Ok(VersionReq::exact(&v)),
Err(e) => {
let mut msg = e.to_string();
Expand Down
10 changes: 5 additions & 5 deletions src/cargo/core/package_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use serde::ser;

use crate::core::SourceId;
use crate::util::interning::InternedString;
use crate::util::{CargoResult, ToSemver};
use crate::util::CargoResult;

static PACKAGE_ID_CACHE: OnceLock<Mutex<HashSet<&'static PackageIdInner>>> = OnceLock::new();

Expand Down Expand Up @@ -82,7 +82,7 @@ impl<'de> de::Deserialize<'de> for PackageId {
let (field, rest) = rest
.split_once(' ')
.ok_or_else(|| de::Error::custom("invalid serialized PackageId"))?;
let version = field.to_semver().map_err(de::Error::custom)?;
let version = field.parse().map_err(de::Error::custom)?;

let url =
strip_parens(rest).ok_or_else(|| de::Error::custom("invalid serialized PackageId"))?;
Expand Down Expand Up @@ -123,12 +123,12 @@ impl Hash for PackageId {
}

impl PackageId {
pub fn new<T: ToSemver>(
pub fn new(
name: impl Into<InternedString>,
version: T,
version: &str,
sid: SourceId,
) -> CargoResult<PackageId> {
let v = version.to_semver()?;
let v = version.parse()?;
Ok(PackageId::pure(name.into(), v, sid))
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/sources/registry/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,7 @@ impl IndexSummary {
} = serde_json::from_slice(line)?;
let v = v.unwrap_or(1);
tracing::trace!("json parsed registry {}/{}", name, vers);
let pkgid = PackageId::new(name, &vers, source_id)?;
let pkgid = PackageId::pure(name.into(), vers.clone(), source_id);
let deps = deps
.into_iter()
.map(|dep| dep.into_dep(source_id))
Expand Down
2 changes: 0 additions & 2 deletions src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ pub use self::queue::Queue;
pub use self::restricted_names::validate_package_name;
pub use self::rustc::Rustc;
pub use self::semver_ext::{OptVersionReq, PartialVersion, RustVersion, VersionExt, VersionReqExt};
pub use self::to_semver::ToSemver;
pub use self::vcs::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo};
pub use self::workspace::{
add_path_args, path_args, print_available_benches, print_available_binaries,
Expand Down Expand Up @@ -62,7 +61,6 @@ pub mod restricted_names;
pub mod rustc;
mod semver_ext;
pub mod style;
pub mod to_semver;
pub mod toml;
pub mod toml_mut;
mod vcs;
Expand Down
36 changes: 0 additions & 36 deletions src/cargo/util/to_semver.rs

This file was deleted.

10 changes: 3 additions & 7 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ impl schema::TomlManifest {
version
.clone()
.unwrap_or_else(|| semver::Version::new(0, 0, 0)),
)?;
);

let edition = if let Some(edition) = package.edition.clone() {
let edition: Edition = edition
Expand Down Expand Up @@ -1596,12 +1596,8 @@ impl schema::InheritableFields {
}

impl schema::TomlPackage {
pub fn to_package_id(
&self,
source_id: SourceId,
version: semver::Version,
) -> CargoResult<PackageId> {
PackageId::new(&self.name, version, source_id)
pub fn to_package_id(&self, source_id: SourceId, version: semver::Version) -> PackageId {
PackageId::pure(self.name.as_str().into(), version, source_id)
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/install_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ fn ambiguous_version_no_longer_allowed() {
cargo_process("install foo --version=1.0")
.with_stderr(
"\
[ERROR] invalid value '1.0' for '--version <VERSION>': cannot parse '1.0' as a SemVer version
[ERROR] invalid value '1.0' for '--version <VERSION>': unexpected end of input while parsing minor version number
tip: if you want to specify SemVer range, add an explicit qualifier, like '^1.0'
Expand Down
1 change: 0 additions & 1 deletion triagebot.toml
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ trigger_files = ["src/cargo/util/auth/"]
trigger_files = [
"crates/semver-check",
"src/cargo/util/semver_ext.rs",
"src/cargo/util/to_semver.rs",
]

[autolabel."A-source-replacement"]
Expand Down

0 comments on commit 2d99bdd

Please sign in to comment.