diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index 9bdeb867415..e2cbcee62b6 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -12,7 +12,7 @@ use std::task::Poll; use std::time::Instant; use cargo::core::dependency::DepKind; -use cargo::core::resolver::{self, ResolveOpts, VersionPreferences}; +use cargo::core::resolver::{self, ResolveOpts, VersionOrdering, VersionPreferences}; use cargo::core::Resolve; use cargo::core::{Dependency, PackageId, Registry, Summary}; use cargo::core::{GitReference, SourceId}; @@ -190,15 +190,17 @@ pub fn resolve_with_config_raw( .unwrap(); let opts = ResolveOpts::everything(); let start = Instant::now(); - let max_rust_version = None; + let mut version_prefs = VersionPreferences::default(); + if config.cli_unstable().minimal_versions { + version_prefs.version_ordering(VersionOrdering::MinimumVersionsFirst) + } let resolve = resolver::resolve( &[(summary, opts)], &[], &mut registry, - &VersionPreferences::default(), + &version_prefs, Some(config), true, - max_rust_version, ); // The largest test in our suite takes less then 30 sec. @@ -982,14 +984,17 @@ fn meta_test_multiple_versions_strategy() { /// Assert `xs` contains `elems` #[track_caller] -pub fn assert_contains<A: PartialEq>(xs: &[A], elems: &[A]) { +pub fn assert_contains<A: PartialEq + std::fmt::Debug>(xs: &[A], elems: &[A]) { for elem in elems { - assert!(xs.contains(elem)); + assert!( + xs.contains(elem), + "missing element\nset: {xs:?}\nmissing: {elem:?}" + ); } } #[track_caller] -pub fn assert_same<A: PartialEq>(a: &[A], b: &[A]) { - assert_eq!(a.len(), b.len()); +pub fn assert_same<A: PartialEq + std::fmt::Debug>(a: &[A], b: &[A]) { + assert_eq!(a.len(), b.len(), "not equal\n{a:?}\n{b:?}"); assert_contains(b, a); } diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index 9041c5b0f9e..6c904c1483a 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -20,7 +20,6 @@ use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec, Registry, use crate::sources::source::QueryKind; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; -use crate::util::RustVersion; use anyhow::Context as _; use std::collections::{BTreeSet, HashMap, HashSet}; @@ -32,16 +31,11 @@ pub struct RegistryQueryer<'a> { pub registry: &'a mut (dyn Registry + 'a), replacements: &'a [(PackageIdSpec, Dependency)], version_prefs: &'a VersionPreferences, - /// If set the list of dependency candidates will be sorted by minimal - /// versions first. That allows `cargo update -Z minimal-versions` which will - /// specify minimum dependency versions to be used. - minimal_versions: bool, - max_rust_version: Option<RustVersion>, - /// a cache of `Candidate`s that fulfil a `Dependency` (and whether `first_minimal_version`) - registry_cache: HashMap<(Dependency, bool), Poll<Rc<Vec<Summary>>>>, + /// a cache of `Candidate`s that fulfil a `Dependency` (and whether `first_version`) + registry_cache: HashMap<(Dependency, Option<VersionOrdering>), Poll<Rc<Vec<Summary>>>>, /// a cache of `Dependency`s that are required for a `Summary` /// - /// HACK: `first_minimal_version` is not kept in the cache key is it is 1:1 with + /// HACK: `first_version` is not kept in the cache key is it is 1:1 with /// `parent.is_none()` (the first element of the cache key) as it doesn't change through /// execution. summary_cache: HashMap< @@ -57,15 +51,11 @@ impl<'a> RegistryQueryer<'a> { registry: &'a mut dyn Registry, replacements: &'a [(PackageIdSpec, Dependency)], version_prefs: &'a VersionPreferences, - minimal_versions: bool, - max_rust_version: Option<&RustVersion>, ) -> Self { RegistryQueryer { registry, replacements, version_prefs, - minimal_versions, - max_rust_version: max_rust_version.cloned(), registry_cache: HashMap::new(), summary_cache: HashMap::new(), used_replacements: HashMap::new(), @@ -106,23 +96,20 @@ impl<'a> RegistryQueryer<'a> { pub fn query( &mut self, dep: &Dependency, - first_minimal_version: bool, + first_version: Option<VersionOrdering>, ) -> Poll<CargoResult<Rc<Vec<Summary>>>> { - let registry_cache_key = (dep.clone(), first_minimal_version); + let registry_cache_key = (dep.clone(), first_version); if let Some(out) = self.registry_cache.get(®istry_cache_key).cloned() { return out.map(Result::Ok); } let mut ret = Vec::new(); let ready = self.registry.query(dep, QueryKind::Exact, &mut |s| { - if self.max_rust_version.is_none() || s.rust_version() <= self.max_rust_version.as_ref() - { - ret.push(s); - } + ret.push(s); })?; if ready.is_pending() { self.registry_cache - .insert((dep.clone(), first_minimal_version), Poll::Pending); + .insert((dep.clone(), first_version), Poll::Pending); return Poll::Pending; } for summary in ret.iter() { @@ -144,7 +131,7 @@ impl<'a> RegistryQueryer<'a> { Poll::Ready(s) => s.into_iter(), Poll::Pending => { self.registry_cache - .insert((dep.clone(), first_minimal_version), Poll::Pending); + .insert((dep.clone(), first_version), Poll::Pending); return Poll::Pending; } }; @@ -215,16 +202,8 @@ impl<'a> RegistryQueryer<'a> { } } - // When we attempt versions for a package we'll want to do so in a sorted fashion to pick - // the "best candidates" first. VersionPreferences implements this notion. - let ordering = if first_minimal_version || self.minimal_versions { - VersionOrdering::MinimumVersionsFirst - } else { - VersionOrdering::MaximumVersionsFirst - }; - let first_version = first_minimal_version; - self.version_prefs - .sort_summaries(&mut ret, ordering, first_version); + let first_version = first_version; + self.version_prefs.sort_summaries(&mut ret, first_version); let out = Poll::Ready(Rc::new(ret)); @@ -243,7 +222,7 @@ impl<'a> RegistryQueryer<'a> { parent: Option<PackageId>, candidate: &Summary, opts: &ResolveOpts, - first_minimal_version: bool, + first_version: Option<VersionOrdering>, ) -> ActivateResult<Rc<(HashSet<InternedString>, Rc<Vec<DepInfo>>)>> { // if we have calculated a result before, then we can just return it, // as it is a "pure" query of its arguments. @@ -263,24 +242,22 @@ impl<'a> RegistryQueryer<'a> { let mut all_ready = true; let mut deps = deps .into_iter() - .filter_map( - |(dep, features)| match self.query(&dep, first_minimal_version) { - Poll::Ready(Ok(candidates)) => Some(Ok((dep, candidates, features))), - Poll::Pending => { - all_ready = false; - // we can ignore Pending deps, resolve will be repeatedly called - // until there are none to ignore - None - } - Poll::Ready(Err(e)) => Some(Err(e).with_context(|| { - format!( - "failed to get `{}` as a dependency of {}", - dep.package_name(), - describe_path_in_context(cx, &candidate.package_id()), - ) - })), - }, - ) + .filter_map(|(dep, features)| match self.query(&dep, first_version) { + Poll::Ready(Ok(candidates)) => Some(Ok((dep, candidates, features))), + Poll::Pending => { + all_ready = false; + // we can ignore Pending deps, resolve will be repeatedly called + // until there are none to ignore + None + } + Poll::Ready(Err(e)) => Some(Err(e).with_context(|| { + format!( + "failed to get `{}` as a dependency of {}", + dep.package_name(), + describe_path_in_context(cx, &candidate.package_id()), + ) + })), + }) .collect::<CargoResult<Vec<DepInfo>>>()?; // Attempt to resolve dependencies with fewer candidates before trying diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 7d8e8acd406..ecb6f36e612 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -71,7 +71,6 @@ use crate::util::config::Config; use crate::util::errors::CargoResult; use crate::util::network::PollExt; use crate::util::profile; -use crate::util::RustVersion; use self::context::Context; use self::dep_cache::RegistryQueryer; @@ -139,39 +138,18 @@ pub fn resolve( version_prefs: &VersionPreferences, config: Option<&Config>, check_public_visible_dependencies: bool, - mut max_rust_version: Option<&RustVersion>, ) -> CargoResult<Resolve> { let _p = profile::start("resolving"); - let minimal_versions = match config { - Some(config) => config.cli_unstable().minimal_versions, - None => false, - }; - let direct_minimal_versions = match config { - Some(config) => config.cli_unstable().direct_minimal_versions, - None => false, + let first_version = match config { + Some(config) if config.cli_unstable().direct_minimal_versions => { + Some(VersionOrdering::MinimumVersionsFirst) + } + _ => None, }; - if !config - .map(|c| c.cli_unstable().msrv_policy) - .unwrap_or(false) - { - max_rust_version = None; - } - let mut registry = RegistryQueryer::new( - registry, - replacements, - version_prefs, - minimal_versions, - max_rust_version, - ); + let mut registry = RegistryQueryer::new(registry, replacements, version_prefs); let cx = loop { let cx = Context::new(check_public_visible_dependencies); - let cx = activate_deps_loop( - cx, - &mut registry, - summaries, - direct_minimal_versions, - config, - )?; + let cx = activate_deps_loop(cx, &mut registry, summaries, first_version, config)?; if registry.reset_pending() { break cx; } else { @@ -223,7 +201,7 @@ fn activate_deps_loop( mut cx: Context, registry: &mut RegistryQueryer<'_>, summaries: &[(Summary, ResolveOpts)], - direct_minimal_versions: bool, + first_version: Option<VersionOrdering>, config: Option<&Config>, ) -> CargoResult<Context> { let mut backtrack_stack = Vec::new(); @@ -241,7 +219,7 @@ fn activate_deps_loop( registry, None, summary.clone(), - direct_minimal_versions, + first_version, opts, ); match res { @@ -441,13 +419,13 @@ fn activate_deps_loop( dep.package_name(), candidate.version() ); - let direct_minimal_version = false; // this is an indirect dependency + let first_version = None; // this is an indirect dependency let res = activate( &mut cx, registry, Some((&parent, &dep)), candidate, - direct_minimal_version, + first_version, &opts, ); @@ -659,7 +637,7 @@ fn activate( registry: &mut RegistryQueryer<'_>, parent: Option<(&Summary, &Dependency)>, candidate: Summary, - first_minimal_version: bool, + first_version: Option<VersionOrdering>, opts: &ResolveOpts, ) -> ActivateResult<Option<(DepsFrame, Duration)>> { let candidate_pid = candidate.package_id(); @@ -716,7 +694,7 @@ fn activate( parent.map(|p| p.0.package_id()), &candidate, opts, - first_minimal_version, + first_version, )?; // Record what list of features is active for this package. @@ -905,14 +883,14 @@ fn generalize_conflicting( }) { for critical_parents_dep in critical_parents_deps.iter() { - // We only want `first_minimal_version=true` for direct dependencies of workspace + // We only want `first_version.is_some()` for direct dependencies of workspace // members which isn't the case here as this has a `parent` - let first_minimal_version = false; + let first_version = None; // A dep is equivalent to one of the things it can resolve to. // Thus, if all the things it can resolve to have already ben determined // to be conflicting, then we can just say that we conflict with the parent. if let Some(others) = registry - .query(critical_parents_dep, first_minimal_version) + .query(critical_parents_dep, first_version) .expect("an already used dep now error!?") .expect("an already used dep now pending!?") .iter() diff --git a/src/cargo/core/resolver/version_prefs.rs b/src/cargo/core/resolver/version_prefs.rs index 28de77f118a..0deef55654a 100644 --- a/src/cargo/core/resolver/version_prefs.rs +++ b/src/cargo/core/resolver/version_prefs.rs @@ -6,6 +6,7 @@ use std::collections::{HashMap, HashSet}; use crate::core::{Dependency, PackageId, Summary}; use crate::util::interning::InternedString; +use crate::util::RustVersion; /// A collection of preferences for particular package versions. /// @@ -18,9 +19,13 @@ use crate::util::interning::InternedString; pub struct VersionPreferences { try_to_use: HashSet<PackageId>, prefer_patch_deps: HashMap<InternedString, HashSet<Dependency>>, + version_ordering: VersionOrdering, + max_rust_version: Option<RustVersion>, } +#[derive(Copy, Clone, Default, PartialEq, Eq, Hash, Debug)] pub enum VersionOrdering { + #[default] MaximumVersionsFirst, MinimumVersionsFirst, } @@ -39,14 +44,29 @@ impl VersionPreferences { .insert(dep); } - /// Sort the given vector of summaries in-place, with all summaries presumed to be for - /// the same package. Preferred versions appear first in the result, sorted by - /// `version_ordering`, followed by non-preferred versions sorted the same way. + pub fn version_ordering(&mut self, ordering: VersionOrdering) { + self.version_ordering = ordering; + } + + pub fn max_rust_version(&mut self, ver: Option<RustVersion>) { + self.max_rust_version = ver; + } + + /// Sort (and filter) the given vector of summaries in-place + /// + /// Note: all summaries presumed to be for the same package. + /// + /// Sort order: + /// 1. Preferred packages + /// 2. `first_version`, falling back to [`VersionPreferences::version_ordering`] when `None` + /// + /// Filtering: + /// - [`VersionPreferences::max_rust_version`] + /// - `first_version` pub fn sort_summaries( &self, summaries: &mut Vec<Summary>, - version_ordering: VersionOrdering, - first_version: bool, + first_version: Option<VersionOrdering>, ) { let should_prefer = |pkg_id: &PackageId| { self.try_to_use.contains(pkg_id) @@ -56,22 +76,24 @@ impl VersionPreferences { .map(|deps| deps.iter().any(|d| d.matches_id(*pkg_id))) .unwrap_or(false) }; + if self.max_rust_version.is_some() { + summaries.retain(|s| s.rust_version() <= self.max_rust_version.as_ref()); + } summaries.sort_unstable_by(|a, b| { let prefer_a = should_prefer(&a.package_id()); let prefer_b = should_prefer(&b.package_id()); let previous_cmp = prefer_a.cmp(&prefer_b).reverse(); - match previous_cmp { - Ordering::Equal => { - let cmp = a.version().cmp(b.version()); - match version_ordering { - VersionOrdering::MaximumVersionsFirst => cmp.reverse(), - VersionOrdering::MinimumVersionsFirst => cmp, - } - } - _ => previous_cmp, + if previous_cmp != Ordering::Equal { + return previous_cmp; + } + + let cmp = a.version().cmp(b.version()); + match first_version.unwrap_or(self.version_ordering) { + VersionOrdering::MaximumVersionsFirst => cmp.reverse(), + VersionOrdering::MinimumVersionsFirst => cmp, } }); - if first_version { + if first_version.is_some() { let _ = summaries.split_off(1); } } @@ -81,7 +103,6 @@ impl VersionPreferences { mod test { use super::*; use crate::core::SourceId; - use crate::util::RustVersion; use std::collections::BTreeMap; fn pkgid(name: &str, version: &str) -> PackageId { @@ -96,7 +117,7 @@ mod test { Dependency::parse(name, Some(version), src_id).unwrap() } - fn summ(name: &str, version: &str) -> Summary { + fn summ(name: &str, version: &str, msrv: Option<&str>) -> Summary { let pkg_id = pkgid(name, version); let features = BTreeMap::new(); Summary::new( @@ -104,7 +125,7 @@ mod test { Vec::new(), &features, None::<&String>, - None::<RustVersion>, + msrv.map(|m| m.parse().unwrap()), ) .unwrap() } @@ -123,19 +144,21 @@ mod test { vp.prefer_package_id(pkgid("foo", "1.2.3")); let mut summaries = vec![ - summ("foo", "1.2.4"), - summ("foo", "1.2.3"), - summ("foo", "1.1.0"), - summ("foo", "1.0.9"), + summ("foo", "1.2.4", None), + summ("foo", "1.2.3", None), + summ("foo", "1.1.0", None), + summ("foo", "1.0.9", None), ]; - vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst, false); + vp.version_ordering(VersionOrdering::MaximumVersionsFirst); + vp.sort_summaries(&mut summaries, None); assert_eq!( describe(&summaries), "foo/1.2.3, foo/1.2.4, foo/1.1.0, foo/1.0.9".to_string() ); - vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst, false); + vp.version_ordering(VersionOrdering::MinimumVersionsFirst); + vp.sort_summaries(&mut summaries, None); assert_eq!( describe(&summaries), "foo/1.2.3, foo/1.0.9, foo/1.1.0, foo/1.2.4".to_string() @@ -148,19 +171,21 @@ mod test { vp.prefer_dependency(dep("foo", "=1.2.3")); let mut summaries = vec![ - summ("foo", "1.2.4"), - summ("foo", "1.2.3"), - summ("foo", "1.1.0"), - summ("foo", "1.0.9"), + summ("foo", "1.2.4", None), + summ("foo", "1.2.3", None), + summ("foo", "1.1.0", None), + summ("foo", "1.0.9", None), ]; - vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst, false); + vp.version_ordering(VersionOrdering::MaximumVersionsFirst); + vp.sort_summaries(&mut summaries, None); assert_eq!( describe(&summaries), "foo/1.2.3, foo/1.2.4, foo/1.1.0, foo/1.0.9".to_string() ); - vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst, false); + vp.version_ordering(VersionOrdering::MinimumVersionsFirst); + vp.sort_summaries(&mut summaries, None); assert_eq!( describe(&summaries), "foo/1.2.3, foo/1.0.9, foo/1.1.0, foo/1.2.4".to_string() @@ -174,22 +199,51 @@ mod test { vp.prefer_dependency(dep("foo", "=1.1.0")); let mut summaries = vec![ - summ("foo", "1.2.4"), - summ("foo", "1.2.3"), - summ("foo", "1.1.0"), - summ("foo", "1.0.9"), + summ("foo", "1.2.4", None), + summ("foo", "1.2.3", None), + summ("foo", "1.1.0", None), + summ("foo", "1.0.9", None), ]; - vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst, false); + vp.version_ordering(VersionOrdering::MaximumVersionsFirst); + vp.sort_summaries(&mut summaries, None); assert_eq!( describe(&summaries), "foo/1.2.3, foo/1.1.0, foo/1.2.4, foo/1.0.9".to_string() ); - vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst, false); + vp.version_ordering(VersionOrdering::MinimumVersionsFirst); + vp.sort_summaries(&mut summaries, None); assert_eq!( describe(&summaries), "foo/1.1.0, foo/1.2.3, foo/1.0.9, foo/1.2.4".to_string() ); } + + #[test] + fn test_max_rust_version() { + let mut vp = VersionPreferences::default(); + vp.max_rust_version(Some("1.50".parse().unwrap())); + + let mut summaries = vec![ + summ("foo", "1.2.4", Some("1.60")), + summ("foo", "1.2.3", Some("1.50")), + summ("foo", "1.1.0", Some("1.40")), + summ("foo", "1.0.9", None), + ]; + + vp.version_ordering(VersionOrdering::MaximumVersionsFirst); + vp.sort_summaries(&mut summaries, None); + assert_eq!( + describe(&summaries), + "foo/1.2.3, foo/1.1.0, foo/1.0.9".to_string() + ); + + vp.version_ordering(VersionOrdering::MinimumVersionsFirst); + vp.sort_summaries(&mut summaries, None); + assert_eq!( + describe(&summaries), + "foo/1.0.9, foo/1.1.0, foo/1.2.3".to_string() + ); + } } diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 6d91ef67d01..8ca72f77c6a 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -61,7 +61,7 @@ use crate::core::resolver::features::{ CliFeatures, FeatureOpts, FeatureResolver, ForceAllTargets, RequestedFeatures, ResolvedFeatures, }; use crate::core::resolver::{ - self, HasDevUnits, Resolve, ResolveOpts, ResolveVersion, VersionPreferences, + self, HasDevUnits, Resolve, ResolveOpts, ResolveVersion, VersionOrdering, VersionPreferences, }; use crate::core::summary::Summary; use crate::core::Feature; @@ -321,6 +321,12 @@ pub fn resolve_with_previous<'cfg>( // While registering patches, we will record preferences for particular versions // of various packages. let mut version_prefs = VersionPreferences::default(); + if ws.config().cli_unstable().minimal_versions { + version_prefs.version_ordering(VersionOrdering::MinimumVersionsFirst) + } + if ws.config().cli_unstable().msrv_policy { + version_prefs.max_rust_version(max_rust_version.cloned()); + } // This is a set of PackageIds of `[patch]` entries, and some related locked PackageIds, for // which locking should be avoided (but which will be preferred when searching dependencies, @@ -509,7 +515,6 @@ pub fn resolve_with_previous<'cfg>( ws.unstable_features() .require(Feature::public_dependency()) .is_ok(), - max_rust_version, )?; let patches: Vec<_> = registry .patches()