Factor version preferences into a struct

crates/resolver-tests/src/lib.rs

@@ -10,7 +10,7 @@ use std::rc::Rc;
 use std::time::Instant;
 
 use cargo::core::dependency::DepKind;
-use cargo::core::resolver::{self, ResolveOpts};
+use cargo::core::resolver::{self, ResolveOpts, VersionPreferences};
 use cargo::core::source::{GitReference, SourceId};
 use cargo::core::Resolve;
 use cargo::core::{Dependency, PackageId, Registry, Summary};
@@ -183,8 +183,7 @@ pub fn resolve_with_config_raw(
         &[(summary, opts)],
         &[],
         &mut registry,
-        &HashSet::new(),
-        &HashMap::new(),
+        &VersionPreferences::default(),
         Some(config),
         true,
     );

src/cargo/core/resolver/dep_cache.rs

@@ -13,23 +13,22 @@ use crate::core::resolver::context::Context;
 use crate::core::resolver::errors::describe_path;
 use crate::core::resolver::types::{ConflictReason, DepInfo, FeaturesSet};
 use crate::core::resolver::{
-    ActivateError, ActivateResult, CliFeatures, RequestedFeatures, ResolveOpts,
+    ActivateError, ActivateResult, CliFeatures, RequestedFeatures, ResolveOpts, VersionOrdering,
+    VersionPreferences,
 };
 use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec, Registry, Summary};
 use crate::util::errors::CargoResult;
 use crate::util::interning::InternedString;
 
 use anyhow::Context as _;
 use log::debug;
-use std::cmp::Ordering;
 use std::collections::{BTreeSet, HashMap, HashSet};
 use std::rc::Rc;
 
 pub struct RegistryQueryer<'a> {
     pub registry: &'a mut (dyn Registry + 'a),
     replacements: &'a [(PackageIdSpec, Dependency)],
-    try_to_use: &'a HashSet<PackageId>,
-    prefer_patch_deps: &'a HashMap<InternedString, HashSet<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.
@@ -49,15 +48,13 @@ impl<'a> RegistryQueryer<'a> {
     pub fn new(
         registry: &'a mut dyn Registry,
         replacements: &'a [(PackageIdSpec, Dependency)],
-        try_to_use: &'a HashSet<PackageId>,
-        prefer_patch_deps: &'a HashMap<InternedString, HashSet<Dependency>>,
+        version_prefs: &'a VersionPreferences,
         minimal_versions: bool,
     ) -> Self {
         RegistryQueryer {
             registry,
             replacements,
-            try_to_use,
-            prefer_patch_deps,
+            version_prefs,
             minimal_versions,
             registry_cache: HashMap::new(),
             summary_cache: HashMap::new(),
@@ -168,35 +165,15 @@ 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. Currently we try prioritized summaries (those in
-        // `try_to_use` or `prefer_patch_deps`) and failing that we list everything from the
-        // maximum version to the lowest version.
-        let should_prefer = |package_id: &PackageId| {
-            self.try_to_use.contains(package_id)
-                || self
-                    .prefer_patch_deps
-                    .get(&package_id.name())
-                    .map(|deps| deps.iter().any(|d| d.matches_id(*package_id)))
-                    .unwrap_or(false)
-        };
-        ret.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());
-                    if self.minimal_versions {
-                        // Lower version ordered first.
-                        cmp
-                    } else {
-                        // Higher version ordered first.
-                        cmp.reverse()
-                    }
-                }
-                _ => previous_cmp,
-            }
-        });
+        // the "best candidates" first. VersionPreferences implements this notion.
+        self.version_prefs.sort_summaries(
+            &mut ret,
+            if self.minimal_versions {
+                VersionOrdering::MinimumVersionsFirst
+            } else {
+                VersionOrdering::MaximumVersionsFirst
+            },
+        );
 
         let out = Rc::new(ret);
 

src/cargo/core/resolver/mod.rs

@@ -58,7 +58,6 @@ use crate::core::PackageIdSpec;
 use crate::core::{Dependency, PackageId, Registry, Summary};
 use crate::util::config::Config;
 use crate::util::errors::CargoResult;
-use crate::util::interning::InternedString;
 use crate::util::profile;
 
 use self::context::Context;
@@ -73,6 +72,7 @@ pub use self::errors::{ActivateError, ActivateResult, ResolveError};
 pub use self::features::{CliFeatures, ForceAllTargets, HasDevUnits};
 pub use self::resolve::{Resolve, ResolveVersion};
 pub use self::types::{ResolveBehavior, ResolveOpts};
+pub use self::version_prefs::{VersionOrdering, VersionPreferences};
 
 mod conflict_cache;
 mod context;
@@ -82,6 +82,7 @@ mod errors;
 pub mod features;
 mod resolve;
 mod types;
+mod version_prefs;
 
 /// Builds the list of all packages required to build the first argument.
 ///
@@ -102,14 +103,8 @@ mod types;
 ///   for the same query every time). Typically this is an instance of a
 ///   `PackageRegistry`.
 ///
-/// * `try_to_use` - this is a list of package IDs which were previously found
-///   in the lock file. We heuristically prefer the ids listed in `try_to_use`
-///   when sorting candidates to activate, but otherwise this isn't used
-///   anywhere else.
-///
-/// * `prefer_patch_deps` - this is a collection of dependencies on `[patch]`es, which
-///   should be preferred much like `try_to_use`, but are in the form of Dependencies
-///   and not PackageIds.
+/// * `version_prefs` - this represents a preference for some versions over others,
+///   based on the lock file or other reasons such as `[patch]`es.
 ///
 /// * `config` - a location to print warnings and such, or `None` if no warnings
 ///   should be printed
@@ -128,8 +123,7 @@ pub fn resolve(
     summaries: &[(Summary, ResolveOpts)],
     replacements: &[(PackageIdSpec, Dependency)],
     registry: &mut dyn Registry,
-    try_to_use: &HashSet<PackageId>,
-    prefer_patch_deps: &HashMap<InternedString, HashSet<Dependency>>,
+    version_prefs: &VersionPreferences,
     config: Option<&Config>,
     check_public_visible_dependencies: bool,
 ) -> CargoResult<Resolve> {
@@ -139,13 +133,8 @@ pub fn resolve(
         Some(config) => config.cli_unstable().minimal_versions,
         None => false,
     };
-    let mut registry = RegistryQueryer::new(
-        registry,
-        replacements,
-        try_to_use,
-        prefer_patch_deps,
-        minimal_versions,
-    );
+    let mut registry =
+        RegistryQueryer::new(registry, replacements, version_prefs, minimal_versions);
     let cx = activate_deps_loop(cx, &mut registry, summaries, config)?;
 
     let mut cksums = HashMap::new();

src/cargo/core/resolver/version_prefs.rs

@@ -0,0 +1,181 @@
+//! This module implements support for preferring some versions of a package
+//! over other versions.
+
+use std::cmp::Ordering;
+use std::collections::{HashMap, HashSet};
+
+use crate::core::{Dependency, PackageId, Summary};
+use crate::util::interning::InternedString;
+
+/// A collection of preferences for particular package versions.
+///
+/// This is built up with [`prefer_package_id`] and [`prefer_dep`], then used to sort the set of
+/// summaries for a package during resolution via [`sort_summaries`].
+///
+/// As written, a version is either "preferred" or "not preferred".  Later extensions may
+/// introduce more granular preferences.
+#[derive(Default)]
+pub struct VersionPreferences {
+    try_to_use: HashSet<PackageId>,
+    prefer_patch_deps: HashMap<InternedString, HashSet<Dependency>>,
+}
+
+pub enum VersionOrdering {
+    MaximumVersionsFirst,
+    MinimumVersionsFirst,
+}
+
+impl VersionPreferences {
+    /// Indicate that the given package (specified as a [`PackageId`]) should be preferred.
+    pub fn prefer_package_id(&mut self, pkg_id: PackageId) {
+        self.try_to_use.insert(pkg_id);
+    }
+
+    /// Indicate that the given package (specified as a [`Dependency`])  should be preferred.
+    pub fn prefer_dependency(&mut self, dep: Dependency) {
+        self.prefer_patch_deps
+            .entry(dep.package_name())
+            .or_insert_with(HashSet::new)
+            .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 sort_summaries(&self, summaries: &mut Vec<Summary>, version_ordering: VersionOrdering) {
+        let should_prefer = |pkg_id: &PackageId| {
+            self.try_to_use.contains(pkg_id)
+                || self
+                    .prefer_patch_deps
+                    .get(&pkg_id.name())
+                    .map(|deps| deps.iter().any(|d| d.matches_id(*pkg_id)))
+                    .unwrap_or(false)
+        };
+        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,
+            }
+        });
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::core::SourceId;
+    use crate::util::Config;
+    use std::collections::BTreeMap;
+
+    fn pkgid(name: &str, version: &str) -> PackageId {
+        let src_id =
+            SourceId::from_url("registry+https://github.com/rust-lang/crates.io-index").unwrap();
+        PackageId::new(name, version, src_id).unwrap()
+    }
+
+    fn dep(name: &str, version: &str) -> Dependency {
+        let src_id =
+            SourceId::from_url("registry+https://github.com/rust-lang/crates.io-index").unwrap();
+        Dependency::parse(name, Some(version), src_id).unwrap()
+    }
+
+    fn summ(name: &str, version: &str) -> Summary {
+        let pkg_id = pkgid(name, version);
+        let config = Config::default().unwrap();
+        let features = BTreeMap::new();
+        Summary::new(&config, pkg_id, Vec::new(), &features, None::<&String>).unwrap()
+    }
+
+    fn describe(summaries: &Vec<Summary>) -> String {
+        let strs: Vec<String> = summaries
+            .iter()
+            .map(|summary| format!("{}/{}", summary.name(), summary.version()))
+            .collect();
+        strs.join(", ")
+    }
+
+    #[test]
+    fn test_prefer_package_id() {
+        let mut vp = VersionPreferences::default();
+        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"),
+        ];
+
+        vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst);
+        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);
+        assert_eq!(
+            describe(&summaries),
+            "foo/1.2.3, foo/1.0.9, foo/1.1.0, foo/1.2.4".to_string()
+        );
+    }
+
+    #[test]
+    fn test_prefer_dependency() {
+        let mut vp = VersionPreferences::default();
+        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"),
+        ];
+
+        vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst);
+        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);
+        assert_eq!(
+            describe(&summaries),
+            "foo/1.2.3, foo/1.0.9, foo/1.1.0, foo/1.2.4".to_string()
+        );
+    }
+
+    #[test]
+    fn test_prefer_both() {
+        let mut vp = VersionPreferences::default();
+        vp.prefer_package_id(pkgid("foo", "1.2.3"));
+        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"),
+        ];
+
+        vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst);
+        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);
+        assert_eq!(
+            describe(&summaries),
+            "foo/1.1.0, foo/1.2.3, foo/1.0.9, foo/1.2.4".to_string()
+        );
+    }
+}