From dff1874c984a5631d2e01603b13e2fe5d4a33c20 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Wed, 25 Oct 2023 14:25:06 +0000 Subject: [PATCH] move handling precise up --- src/cargo/sources/registry/index.rs | 38 ++---------------------- src/cargo/sources/registry/mod.rs | 21 +++++++++---- src/cargo/util/semver_ext.rs | 46 +++++++++++++++++++++++++---- 3 files changed, 60 insertions(+), 45 deletions(-) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index b9dbc76dd68..c235ecc46cf 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -98,7 +98,6 @@ 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; @@ -632,11 +631,7 @@ impl<'cfg> RegistryIndex<'cfg> { f: &mut dyn FnMut(IndexSummary), online: bool, ) -> Poll> { - let source_id = self.source_id; - - let summaries = ready!(self.summaries(name, req, load))?; - - let summaries = summaries + ready!(self.summaries(name, &req, load))? // First filter summaries for `--offline`. If we're online then // everything is a candidate, otherwise if we're offline we're only // going to consider candidates which are actually present on disk. @@ -657,35 +652,8 @@ impl<'cfg> RegistryIndex<'cfg> { // Next filter out all yanked packages. Some yanked packages may // leak through if they're in a whitelist (aka if they were // previously in `Cargo.lock` - .filter(|s| !s.is_yanked() || yanked_whitelist.contains(&s.package_id())); - - // Handle `cargo update --precise` here. - let precise = source_id.precise_registry_version(name.as_str()); - let summaries = summaries.filter(|s| match precise { - Some((current, requested)) => { - if req.matches(current) { - // Unfortunately crates.io allows versions to differ only - // by build metadata. This shouldn't be allowed, but since - // it is, this will honor it if requested. However, if not - // specified, then ignore it. - let s_vers = s.package_id().version(); - match (s_vers.build.is_empty(), requested.build.is_empty()) { - (true, true) => s_vers == requested, - (true, false) => false, - (false, true) => { - // Compare disregarding the metadata. - s_vers.cmp_precedence(requested) == Ordering::Equal - } - (false, false) => s_vers == requested, - } - } else { - true - } - } - None => true, - }); - - summaries.for_each(f); + .filter(|s| !s.is_yanked() || yanked_whitelist.contains(&s.package_id())) + .for_each(f); Poll::Ready(Ok(())) } diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index f5cc17b6a43..273f6ffb5c4 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -712,16 +712,27 @@ impl<'cfg> Source for RegistrySource<'cfg> { kind: QueryKind, f: &mut dyn FnMut(Summary), ) -> Poll> { - // If this is a precise dependency, then it came from a lock file and in + let mut req = dep.version_req().clone(); + + // Handle `cargo update --precise` here. + if let Some((_, requested)) = self + .source_id + .precise_registry_version(dep.package_name().as_str()) + .filter(|(c, _)| req.matches(c)) + { + req.update_precise(&requested); + } + + // If this is a locked dependency, then it came from a lock file and in // theory the registry is known to contain this version. If, however, we // come back with no summaries, then our registry may need to be // updated, so we fall back to performing a lazy update. - if kind == QueryKind::Exact && dep.source_id().has_precise() && !self.ops.is_updated() { + if kind == QueryKind::Exact && req.is_locked() && !self.ops.is_updated() { debug!("attempting query without update"); let mut called = false; ready!(self.index.query_inner( dep.package_name(), - dep.version_req(), + &req, &mut *self.ops, &self.yanked_whitelist, &mut |s| { @@ -742,7 +753,7 @@ impl<'cfg> Source for RegistrySource<'cfg> { let mut called = false; ready!(self.index.query_inner( dep.package_name(), - dep.version_req(), + &req, &mut *self.ops, &self.yanked_whitelist, &mut |s| { @@ -779,7 +790,7 @@ impl<'cfg> Source for RegistrySource<'cfg> { .index .query_inner( name_permutation, - dep.version_req(), + &req, &mut *self.ops, &self.yanked_whitelist, f, diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index faab9fbce17..5ad61f5787f 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -8,6 +8,8 @@ pub enum OptVersionReq { Req(VersionReq), /// The exact locked version and the original version requirement. Locked(Version, VersionReq), + /// The exact requested version and the original version requirement. + UpdatePrecise(Version, VersionReq), } pub trait VersionExt { @@ -53,7 +55,7 @@ impl OptVersionReq { pub fn is_exact(&self) -> bool { match self { OptVersionReq::Any => false, - OptVersionReq::Req(req) => { + OptVersionReq::Req(req) | OptVersionReq::UpdatePrecise(_, req) => { req.comparators.len() == 1 && { let cmp = &req.comparators[0]; cmp.op == Op::Exact && cmp.minor.is_some() && cmp.patch.is_some() @@ -69,8 +71,24 @@ impl OptVersionReq { let version = version.clone(); *self = match self { Any => Locked(version, VersionReq::STAR), - Req(req) => Locked(version, req.clone()), - Locked(_, req) => Locked(version, req.clone()), + Req(req) | Locked(_, req) | UpdatePrecise(_, req) => Locked(version, req.clone()), + }; + } + + pub fn update_precise(&mut self, version: &Version) { + assert!( + self.matches(version), + "cannot update_precise {} to {}", + self, + version + ); + use OptVersionReq::*; + let version = version.clone(); + *self = match self { + Any => UpdatePrecise(version, VersionReq::STAR), + Req(req) | Locked(_, req) | UpdatePrecise(_, req) => { + UpdatePrecise(version, req.clone()) + } }; } @@ -100,6 +118,23 @@ impl OptVersionReq { // we should not silently use `1.0.0+foo` even though they have the same version. v == version } + OptVersionReq::UpdatePrecise(v, _) => { + // This is used for the `--precise` field of cargo update. + // + // Unfortunately crates.io allowed versions to differ only + // by build metadata. This shouldn't be allowed, but since + // it is, this will honor it if requested. + // + // In that context we treat a requirement that does not have + // build metadata as allowing any metadata. But, if a requirement + // has build metadata, then we only allow it to match the exact + // metadata. + v.major == version.major + && v.minor == version.minor + && v.patch == version.patch + && v.pre == version.pre + && (v.build == version.build || v.build.is_empty()) + } } } } @@ -108,8 +143,9 @@ impl Display for OptVersionReq { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { OptVersionReq::Any => f.write_str("*"), - OptVersionReq::Req(req) => Display::fmt(req, f), - OptVersionReq::Locked(_, req) => Display::fmt(req, f), + OptVersionReq::Req(req) + | OptVersionReq::Locked(_, req) + | OptVersionReq::UpdatePrecise(_, req) => Display::fmt(req, f), } } }