refactor(resolver): Track minimal-versions in VersionPreferences

This had repurcussions on direct-minimal-versions as it before relied on
the ordering parameter to `sort_sumarries`.

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};
@@ -191,11 +191,15 @@ pub fn resolve_with_config_raw(
     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,

src/cargo/core/resolver/dep_cache.rs

@@ -32,16 +32,12 @@ 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`)
+    /// a cache of `Candidate`s that fulfil a `Dependency` (and whether `first_version`)
-    registry_cache: HashMap<(Dependency, bool), Poll<Rc<Vec<Summary>>>>,
+    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,14 +53,12 @@ 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(),
@@ -106,9 +100,9 @@ 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(&registry_cache_key).cloned() {
             return out.map(Result::Ok);
         }
@@ -122,7 +116,7 @@ impl<'a> RegistryQueryer<'a> {
         })?;
         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 +138,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 +209,8 @@ impl<'a> RegistryQueryer<'a> {
             }
         }
 
-        // When we attempt versions for a package we'll want to do so in a sorted fashion to pick
+        let first_version = first_version;
-        // the "best candidates" first. VersionPreferences implements this notion.
+        self.version_prefs.sort_summaries(&mut ret, first_version);
-        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 out = Poll::Ready(Rc::new(ret));
 
@@ -243,7 +229,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 +249,22 @@ impl<'a> RegistryQueryer<'a> {
         let mut all_ready = true;
         let mut deps = deps
             .into_iter()
-            .filter_map(
+            .filter_map(|(dep, features)| match self.query(&dep, first_version) {
-                |(dep, features)| match self.query(&dep, first_minimal_version) {
+                Poll::Ready(Ok(candidates)) => Some(Ok((dep, candidates, features))),
-                    Poll::Ready(Ok(candidates)) => Some(Ok((dep, candidates, features))),
+                Poll::Pending => {
-                    Poll::Pending => {
+                    all_ready = false;
-                        all_ready = false;
+                    // we can ignore Pending deps, resolve will be repeatedly called
-                        // we can ignore Pending deps, resolve will be repeatedly called
+                    // until there are none to ignore
-                        // until there are none to ignore
+                    None
-                        None
+                }
-                    }
+                Poll::Ready(Err(e)) => Some(Err(e).with_context(|| {
-                    Poll::Ready(Err(e)) => Some(Err(e).with_context(|| {
+                    format!(
-                        format!(
+                        "failed to get `{}` as a dependency of {}",
-                            "failed to get `{}` as a dependency of {}",
+                        dep.package_name(),
-                            dep.package_name(),
+                        describe_path_in_context(cx, &candidate.package_id()),
-                            describe_path_in_context(cx, &candidate.package_id()),
+                    )
-                        )
+                })),
-                    })),
+            })
-                },
-            )
             .collect::<CargoResult<Vec<DepInfo>>>()?;
 
         // Attempt to resolve dependencies with fewer candidates before trying

src/cargo/core/resolver/mod.rs

@@ -142,13 +142,11 @@ pub fn resolve(
     mut max_rust_version: Option<&RustVersion>,
 ) -> CargoResult<Resolve> {
     let _p = profile::start("resolving");
-    let minimal_versions = match config {
+    let first_version = match config {
-        Some(config) => config.cli_unstable().minimal_versions,
+        Some(config) if config.cli_unstable().direct_minimal_versions => {
-        None => false,
+            Some(VersionOrdering::MinimumVersionsFirst)
-    };
+        }
-    let direct_minimal_versions = match config {
+        _ => None,
-        Some(config) => config.cli_unstable().direct_minimal_versions,
-        None => false,
     };
     if !config
         .map(|c| c.cli_unstable().msrv_policy)
@@ -156,22 +154,11 @@ pub fn resolve(
     {
         max_rust_version = None;
     }
-    let mut registry = RegistryQueryer::new(
+    let mut registry =
-        registry,
+        RegistryQueryer::new(registry, replacements, version_prefs, max_rust_version);
-        replacements,
-        version_prefs,
-        minimal_versions,
-        max_rust_version,
-    );
     let cx = loop {
         let cx = Context::new(check_public_visible_dependencies);
-        let cx = activate_deps_loop(
+        let cx = activate_deps_loop(cx, &mut registry, summaries, first_version, config)?;
-            cx,
-            &mut registry,
-            summaries,
-            direct_minimal_versions,
-            config,
-        )?;
         if registry.reset_pending() {
             break cx;
         } else {
@@ -223,7 +210,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 +228,7 @@ fn activate_deps_loop(
             registry,
             None,
             summary.clone(),
-            direct_minimal_versions,
+            first_version,
             opts,
         );
         match res {
@@ -441,13 +428,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 +646,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 +703,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 +892,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()

src/cargo/core/resolver/version_prefs.rs

@@ -18,9 +18,12 @@ use crate::util::interning::InternedString;
 pub struct VersionPreferences {
     try_to_use: HashSet<PackageId>,
     prefer_patch_deps: HashMap<InternedString, HashSet<Dependency>>,
+    version_ordering: VersionOrdering,
 }
 
+#[derive(Copy, Clone, Default, PartialEq, Eq, Hash, Debug)]
 pub enum VersionOrdering {
+    #[default]
     MaximumVersionsFirst,
     MinimumVersionsFirst,
 }
@@ -39,14 +42,17 @@ impl VersionPreferences {
             .insert(dep);
     }
 
+    pub fn version_ordering(&mut self, ordering: VersionOrdering) {
+        self.version_ordering = ordering;
+    }
+
     /// 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 sort_summaries(
         &self,
         summaries: &mut Vec<Summary>,
-        version_ordering: VersionOrdering,
+        first_version: Option<VersionOrdering>,
-        first_version: bool,
     ) {
         let should_prefer = |pkg_id: &PackageId| {
             self.try_to_use.contains(pkg_id)
@@ -63,7 +69,7 @@ impl VersionPreferences {
             match previous_cmp {
                 Ordering::Equal => {
                     let cmp = a.version().cmp(b.version());
-                    match version_ordering {
+                    match first_version.unwrap_or(self.version_ordering) {
                         VersionOrdering::MaximumVersionsFirst => cmp.reverse(),
                         VersionOrdering::MinimumVersionsFirst => cmp,
                     }
@@ -71,7 +77,7 @@ impl VersionPreferences {
                 _ => previous_cmp,
             }
         });
-        if first_version {
+        if first_version.is_some() {
             let _ = summaries.split_off(1);
         }
     }
@@ -129,13 +135,15 @@ mod test {
             summ("foo", "1.0.9"),
         ];
 
-        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()
@@ -154,13 +162,15 @@ mod test {
             summ("foo", "1.0.9"),
         ];
 
-        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()
@@ -180,13 +190,15 @@ mod test {
             summ("foo", "1.0.9"),
         ];
 
-        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()

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,9 @@ 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)
+    }
 
     // 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,