diff --git a/Cargo.lock b/Cargo.lock index dc7536767..b85960cf5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2886,9 +2886,9 @@ dependencies = [ [[package]] name = "semver" -version = "1.0.19" +version = "1.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ad977052201c6de01a8ef2aa3378c4bd23217a056337d1d6da40468d267a4fb0" +checksum = "836fa6a3e1e547f9a2c4040802ec865b5d85f4014efe00555d7090a3dcaa1090" dependencies = [ "serde", ] diff --git a/Cargo.toml b/Cargo.toml index 5f2ca15e2..9c08d3d59 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,7 +76,7 @@ rand = "0.8.5" rustfix = "0.6.1" same-file = "1.0.6" security-framework = "2.9.2" -semver = { version = "1.0.19", features = ["serde"] } +semver = { version = "1.0.20", features = ["serde"] } serde = "1.0.188" serde-untagged = "0.1.1" serde-value = "0.7.0" diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index 456e79cfb..92234da57 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -29,8 +29,7 @@ struct PackageIdInner { source_id: SourceId, } -// Custom equality that uses full equality of SourceId, rather than its custom equality, -// and Version, which usually ignores `build` metadata. +// Custom equality that uses full equality of SourceId, rather than its custom equality. // // The `build` part of the version is usually ignored (like a "comment"). // However, there are some cases where it is important. The download path from @@ -40,11 +39,7 @@ struct PackageIdInner { impl PartialEq for PackageIdInner { fn eq(&self, other: &Self) -> bool { self.name == other.name - && self.version.major == other.version.major - && self.version.minor == other.version.minor - && self.version.patch == other.version.patch - && self.version.pre == other.version.pre - && self.version.build == other.version.build + && self.version == other.version && self.source_id.full_eq(other.source_id) } } @@ -53,11 +48,7 @@ impl PartialEq for PackageIdInner { impl Hash for PackageIdInner { fn hash(&self, into: &mut S) { self.name.hash(into); - self.version.major.hash(into); - self.version.minor.hash(into); - self.version.patch.hash(into); - self.version.pre.hash(into); - self.version.build.hash(into); + self.version.hash(into); self.source_id.full_hash(into); } } @@ -237,6 +228,16 @@ impl fmt::Debug for PackageId { } } +impl fmt::Debug for PackageIdInner { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + f.debug_struct("PackageIdInner") + .field("name", &self.name) + .field("version", &self.version.to_string()) + .field("source", &self.source_id.to_string()) + .finish() + } +} + #[cfg(test)] mod tests { use super::PackageId; @@ -261,4 +262,14 @@ mod tests { let pkg_id = PackageId::new("foo", "1.0.0", SourceId::for_registry(&loc).unwrap()).unwrap(); assert_eq!("foo v1.0.0", pkg_id.to_string()); } + + #[test] + fn unequal_build_metadata() { + let loc = CRATES_IO_INDEX.into_url().unwrap(); + let repo = SourceId::for_registry(&loc).unwrap(); + let first = PackageId::new("foo", "0.0.1+first", repo).unwrap(); + let second = PackageId::new("foo", "0.0.1+second", repo).unwrap(); + assert_ne!(first, second); + assert_ne!(first.inner, second.inner); + } } diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index 832261a41..cbacfd8a9 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -8,6 +8,7 @@ use crate::util::config::Config; use crate::util::style; use crate::util::CargoResult; use anstyle::Style; +use std::cmp::Ordering; use std::collections::{BTreeMap, HashSet}; use tracing::debug; @@ -152,7 +153,11 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes format!("{} -> v{}", removed[0], added[0].version()) }; - if removed[0].version() > added[0].version() { + // If versions differ only in build metadata, we call it an "update" + // regardless of whether the build metadata has gone up or down. + // This metadata is often stuff like git commit hashes, which are + // not meaningfully ordered. + if removed[0].version().cmp_precedence(added[0].version()) == Ordering::Greater { print_change("Downgrading", msg, &style::WARN)?; } else { print_change("Updating", msg, &style::GOOD)?; diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 7a9ba0e15..617f0b09c 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -98,6 +98,7 @@ use cargo_util::{paths, registry::make_dep_path}; use semver::Version; use serde::Deserialize; use std::borrow::Cow; +use std::cmp::Ordering; use std::collections::BTreeMap; use std::collections::{HashMap, HashSet}; use std::fs; @@ -674,11 +675,8 @@ impl<'cfg> RegistryIndex<'cfg> { (true, true) => s_vers == requested, (true, false) => false, (false, true) => { - // Strip out the metadata. - s_vers.major == requested.major - && s_vers.minor == requested.minor - && s_vers.patch == requested.patch - && s_vers.pre == requested.pre + // Compare disregarding the metadata. + s_vers.cmp_precedence(requested) == Ordering::Equal } (false, false) => s_vers == requested, } diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index bee3b2da3..0be5bca68 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -1,5 +1,6 @@ use semver::{Comparator, Op, Version, VersionReq}; use serde_untagged::UntaggedEnumVisitor; +use std::cmp::Ordering; use std::fmt::{self, Display}; #[derive(PartialEq, Eq, Hash, Clone, Debug)] @@ -83,12 +84,7 @@ impl OptVersionReq { match self { OptVersionReq::Any => true, OptVersionReq::Req(req) => req.matches(version), - OptVersionReq::Locked(v, _) => { - v.major == version.major - && v.minor == version.minor - && v.patch == version.patch - && v.pre == version.pre - } + OptVersionReq::Locked(v, _) => v.cmp_precedence(version) == Ordering::Equal, } } } diff --git a/tests/testsuite/update.rs b/tests/testsuite/update.rs index fe1d86bd7..8a58e4c80 100644 --- a/tests/testsuite/update.rs +++ b/tests/testsuite/update.rs @@ -391,6 +391,50 @@ fn update_precise() { .run(); } +#[cargo_test] +fn update_precise_build_metadata() { + Package::new("serde", "0.0.1+first").publish(); + Package::new("serde", "0.0.1+second").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.0" + + [dependencies] + serde = "0.0.1" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("generate-lockfile").run(); + p.cargo("update serde --precise 0.0.1+first").run(); + + p.cargo("update serde --precise 0.0.1+second") + .with_stderr( + "\ +[UPDATING] `[..]` index +[UPDATING] serde v0.0.1+first -> v0.0.1+second +", + ) + .run(); + + // This is not considered "Downgrading". Build metadata are not assumed to + // be ordered. + p.cargo("update serde --precise 0.0.1+first") + .with_stderr( + "\ +[UPDATING] `[..]` index +[UPDATING] serde v0.0.1+second -> v0.0.1+first +", + ) + .run(); +} + #[cargo_test] fn update_precise_do_not_force_update_deps() { Package::new("log", "0.1.0").publish();