Skip to content

Commit

Permalink
feat(spec): Allow partial versions when unambigious
Browse files Browse the repository at this point in the history 
This was proposed in #12425 to help improve usability of the existing
`cargo update` when dealing with the added workflows.
  • Loading branch information
@epage
epage committed Sep 1, 2023
1 parent a4ad934 commit 68d99fc
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 34 deletions.
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/remove.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
65 changes: 47 additions & 18 deletions src/cargo/core/package_id_spec.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
///
Expand All @@ -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>,
version: Option<PartialVersion>,
url: Option<Url>,
}

Expand Down Expand Up @@ -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::<PartialVersion>()?),
None => None,
};
validate_package_name(name, "pkgid", "")?;
Expand All @@ -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()),
}
}
Expand Down Expand Up @@ -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::<PartialVersion>()?;
(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::<PartialVersion>()?;
(InternedString::new(path_name), Some(version))
}
}
Expand All @@ -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<Version> {
self.version.as_ref().and_then(|v| v.version())
}

pub fn url(&self) -> Option<&Url> {
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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]
Expand All @@ -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#[email protected]",
Expand All @@ -369,11 +380,20 @@ mod tests {
"https://crates.io/foo#[email protected]",
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#[email protected]",
);
ok(
"https://crates.io/foo#[email protected]",
PackageIdSpec {
name: InternedString::new("bar"),
version: Some("1.2".parse().unwrap()),
url: Some(Url::parse("https://crates.io/foo").unwrap()),
},
"https://crates.io/foo#[email protected]",
);
ok(
"foo",
PackageIdSpec {
Expand All @@ -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,
},
"[email protected]",
Expand All @@ -396,21 +416,29 @@ mod tests {
"[email protected]",
PackageIdSpec {
name: InternedString::new("foo"),
version: Some("1.2.3".to_semver().unwrap()),
version: Some("1.2.3".parse().unwrap()),
url: None,
},
"[email protected]",
);
ok(
"[email protected]",
PackageIdSpec {
name: InternedString::new("foo"),
version: Some("1.2".parse().unwrap()),
url: None,
},
"[email protected]",
);
}

#[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("[email protected]").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());
}
Expand All @@ -428,5 +456,6 @@ mod tests {
assert!(!PackageIdSpec::parse("foo:1.2.2").unwrap().matches(foo));
assert!(PackageIdSpec::parse("[email protected]").unwrap().matches(foo));
assert!(!PackageIdSpec::parse("[email protected]").unwrap().matches(foo));
assert!(PackageIdSpec::parse("[email protected]").unwrap().matches(foo));
}
}
2 changes: 1 addition & 1 deletion src/cargo/util/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down
22 changes: 22 additions & 0 deletions src/cargo/util/semver_ext.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,16 @@ pub struct PartialVersion {
}

impl PartialVersion {
pub fn version(&self) -> Option<Version> {
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 {
Expand All @@ -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<semver::Version> for PartialVersion {
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/util/toml/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
19 changes: 7 additions & 12 deletions tests/testsuite/pkgid.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,25 +151,20 @@ fn multiple_versions() {
.with_status(101)
.with_stderr(
"\
error: invalid package ID specification: `two-ver@0`
<tab>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 <spec>` where `<spec>` is one of the following:
[email protected]
[email protected]
",
)
.run();

// Incomplete version.
p.cargo("pkgid [email protected]")
.with_status(101)
.with_stderr(
.with_status(0)
.with_stdout(
"\
error: invalid package ID specification: `[email protected]`
Caused by:
cannot parse '0.2' as a SemVer version
https://github.com/rust-lang/crates.io-index#[email protected]
",
)
.run();
Expand Down

0 comments on commit 68d99fc

Please sign in to comment.