From 68d99fc694272107f5717e8f974de4ad755cc7b3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Sep 2023 13:30:58 -0500 Subject: [PATCH] feat(spec): Allow partial versions when unambigious This was proposed in #12425 to help improve usability of the existing `cargo update` when dealing with the added workflows. --- src/bin/cargo/commands/remove.rs | 2 +- src/cargo/core/package_id_spec.rs | 65 ++++++++++++++++++++++--------- src/cargo/util/mod.rs | 2 +- src/cargo/util/semver_ext.rs | 22 +++++++++++ src/cargo/util/toml/mod.rs | 4 +- tests/testsuite/pkgid.rs | 19 ++++----- 6 files changed, 80 insertions(+), 34 deletions(-) diff --git a/src/bin/cargo/commands/remove.rs b/src/bin/cargo/commands/remove.rs index 798e6fff669b..ce078dd8d512 100644 --- a/src/bin/cargo/commands/remove.rs +++ b/src/bin/cargo/commands/remove.rs @@ -280,7 +280,7 @@ fn spec_has_match( } let version_matches = match (spec.version(), dep.version()) { - (Some(v), Some(vq)) => semver::VersionReq::parse(vq)?.matches(v), + (Some(v), Some(vq)) => semver::VersionReq::parse(vq)?.matches(&v), (Some(_), None) => false, (None, None | Some(_)) => true, }; diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 267ea3ffde20..18d8d5efd828 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -10,7 +10,8 @@ use crate::core::PackageId; use crate::util::edit_distance; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; -use crate::util::{validate_package_name, IntoUrl, ToSemver}; +use crate::util::PartialVersion; +use crate::util::{validate_package_name, IntoUrl}; /// Some or all of the data required to identify a package: /// @@ -24,7 +25,7 @@ use crate::util::{validate_package_name, IntoUrl, ToSemver}; #[derive(Clone, PartialEq, Eq, Debug, Hash, Ord, PartialOrd)] pub struct PackageIdSpec { name: InternedString, - version: Option, + version: Option, url: Option, } @@ -70,7 +71,7 @@ impl PackageIdSpec { let mut parts = spec.splitn(2, [':', '@']); let name = parts.next().unwrap(); let version = match parts.next() { - Some(version) => Some(version.to_semver()?), + Some(version) => Some(version.parse::()?), None => None, }; validate_package_name(name, "pkgid", "")?; @@ -94,12 +95,12 @@ impl PackageIdSpec { spec.query(i) } - /// Convert a `PackageId` to a `PackageIdSpec`, which will have both the `Version` and `Url` + /// Convert a `PackageId` to a `PackageIdSpec`, which will have both the `PartialVersion` and `Url` /// fields filled in. pub fn from_package_id(package_id: PackageId) -> PackageIdSpec { PackageIdSpec { name: package_id.name(), - version: Some(package_id.version().clone()), + version: Some(package_id.version().clone().into()), url: Some(package_id.source_id().url().clone()), } } @@ -128,14 +129,14 @@ impl PackageIdSpec { let name_or_version = parts.next().unwrap(); match parts.next() { Some(part) => { - let version = part.to_semver()?; + let version = part.parse::()?; (InternedString::new(name_or_version), Some(version)) } None => { if name_or_version.chars().next().unwrap().is_alphabetic() { (InternedString::new(name_or_version), None) } else { - let version = name_or_version.to_semver()?; + let version = name_or_version.parse::()?; (InternedString::new(path_name), Some(version)) } } @@ -155,8 +156,9 @@ impl PackageIdSpec { self.name } - pub fn version(&self) -> Option<&Version> { - self.version.as_ref() + /// Full `semver::Version`, if present + pub fn version(&self) -> Option { + self.version.as_ref().and_then(|v| v.version()) } pub fn url(&self) -> Option<&Url> { @@ -174,7 +176,8 @@ impl PackageIdSpec { } if let Some(ref v) = self.version { - if v != package_id.version() { + let req = v.exact_req(); + if !req.matches(package_id.version()) { return false; } } @@ -326,7 +329,6 @@ mod tests { use super::PackageIdSpec; use crate::core::{PackageId, SourceId}; use crate::util::interning::InternedString; - use crate::util::ToSemver; use url::Url; #[test] @@ -351,16 +353,25 @@ mod tests { "https://crates.io/foo#1.2.3", PackageIdSpec { name: InternedString::new("foo"), - version: Some("1.2.3".to_semver().unwrap()), + version: Some("1.2.3".parse().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), }, "https://crates.io/foo#1.2.3", ); + ok( + "https://crates.io/foo#1.2", + PackageIdSpec { + name: InternedString::new("foo"), + version: Some("1.2".parse().unwrap()), + url: Some(Url::parse("https://crates.io/foo").unwrap()), + }, + "https://crates.io/foo#1.2", + ); ok( "https://crates.io/foo#bar:1.2.3", PackageIdSpec { name: InternedString::new("bar"), - version: Some("1.2.3".to_semver().unwrap()), + version: Some("1.2.3".parse().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), }, "https://crates.io/foo#bar@1.2.3", @@ -369,11 +380,20 @@ mod tests { "https://crates.io/foo#bar@1.2.3", PackageIdSpec { name: InternedString::new("bar"), - version: Some("1.2.3".to_semver().unwrap()), + version: Some("1.2.3".parse().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), }, "https://crates.io/foo#bar@1.2.3", ); + ok( + "https://crates.io/foo#bar@1.2", + PackageIdSpec { + name: InternedString::new("bar"), + version: Some("1.2".parse().unwrap()), + url: Some(Url::parse("https://crates.io/foo").unwrap()), + }, + "https://crates.io/foo#bar@1.2", + ); ok( "foo", PackageIdSpec { @@ -387,7 +407,7 @@ mod tests { "foo:1.2.3", PackageIdSpec { name: InternedString::new("foo"), - version: Some("1.2.3".to_semver().unwrap()), + version: Some("1.2.3".parse().unwrap()), url: None, }, "foo@1.2.3", @@ -396,21 +416,29 @@ mod tests { "foo@1.2.3", PackageIdSpec { name: InternedString::new("foo"), - version: Some("1.2.3".to_semver().unwrap()), + version: Some("1.2.3".parse().unwrap()), url: None, }, "foo@1.2.3", ); + ok( + "foo@1.2", + PackageIdSpec { + name: InternedString::new("foo"), + version: Some("1.2".parse().unwrap()), + url: None, + }, + "foo@1.2", + ); } #[test] fn bad_parsing() { assert!(PackageIdSpec::parse("baz:").is_err()); assert!(PackageIdSpec::parse("baz:*").is_err()); - assert!(PackageIdSpec::parse("baz:1.0").is_err()); assert!(PackageIdSpec::parse("baz@").is_err()); assert!(PackageIdSpec::parse("baz@*").is_err()); - assert!(PackageIdSpec::parse("baz@1.0").is_err()); + assert!(PackageIdSpec::parse("baz@^1.0").is_err()); assert!(PackageIdSpec::parse("https://baz:1.0").is_err()); assert!(PackageIdSpec::parse("https://#baz:1.0").is_err()); } @@ -428,5 +456,6 @@ mod tests { assert!(!PackageIdSpec::parse("foo:1.2.2").unwrap().matches(foo)); assert!(PackageIdSpec::parse("foo@1.2.3").unwrap().matches(foo)); assert!(!PackageIdSpec::parse("foo@1.2.2").unwrap().matches(foo)); + assert!(PackageIdSpec::parse("foo@1.2").unwrap().matches(foo)); } } diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 74fcc96050a5..c3193a741088 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -22,7 +22,7 @@ pub use self::progress::{Progress, ProgressStyle}; pub use self::queue::Queue; pub use self::restricted_names::validate_package_name; pub use self::rustc::Rustc; -pub use self::semver_ext::{OptVersionReq, RustVersion, VersionExt, VersionReqExt}; +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::{ diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index b03473fb3fa1..1ed12fbf519f 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -164,6 +164,16 @@ pub struct PartialVersion { } impl PartialVersion { + pub fn version(&self) -> Option { + Some(Version { + major: self.major, + minor: self.minor?, + patch: self.patch?, + pre: self.pre.clone().unwrap_or_default(), + build: self.build.clone().unwrap_or_default(), + }) + } + pub fn caret_req(&self) -> VersionReq { VersionReq { comparators: vec![Comparator { @@ -175,6 +185,18 @@ impl PartialVersion { }], } } + + pub fn exact_req(&self) -> VersionReq { + VersionReq { + comparators: vec![Comparator { + op: semver::Op::Exact, + major: self.major, + minor: self.minor, + patch: self.patch, + pre: self.pre.as_ref().cloned().unwrap_or_default(), + }], + } + } } impl From for PartialVersion { diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 79f49ff47e79..de92cbf123b7 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -2653,8 +2653,8 @@ impl TomlManifest { replacement.unused_keys(), &mut cx.warnings, ); - dep.set_version_req(VersionReq::exact(version)) - .lock_version(version); + dep.set_version_req(VersionReq::exact(&version)) + .lock_version(&version); replace.push((spec, dep)); } Ok(replace) diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index 1857f43b1857..3cac7b9692ea 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -151,25 +151,20 @@ fn multiple_versions() { .with_status(101) .with_stderr( "\ -error: invalid package ID specification: `two-ver@0` - -Did you mean `two-ver`? - -Caused by: - cannot parse '0' as a SemVer version +error: There are multiple `two-ver` packages in your project, and the specification `two-ver@0` is ambiguous. +Please re-run this command with `-p ` where `` is one of the following: + two-ver@0.1.0 + two-ver@0.2.0 ", ) .run(); // Incomplete version. p.cargo("pkgid two-ver@0.2") - .with_status(101) - .with_stderr( + .with_status(0) + .with_stdout( "\ -error: invalid package ID specification: `two-ver@0.2` - -Caused by: - cannot parse '0.2' as a SemVer version +https://github.com/rust-lang/crates.io-index#two-ver@0.2.0 ", ) .run();