fix(resolve): Improve multi-MSRV workspaces

epage · epage · commit 6fac2ab6d936 · 2024-09-19T15:46:22.000-05:00
We do this by resolving for a package version that is compatible with the most number of MSRVs within a workspace. If a version requirement is just right, every package will get the highest MSRV-compatible dependency. If its too high, packages will get MSRV-incompatible dependency versions. This will happen regardless of what we do due to the nature of `"fallback"`. If its too low, packages with higher MSRVs will get older-than-necessary dependency versions. This is similar to the "some with and without MSRV" workspaces. When locking dependencies, we do report to users when newer MSRV/SemVer compatible dependencies are available to help guide them to upgrading if desired. Fixes #14414
diff --git a/src/cargo/core/resolver/version_prefs.rs b/src/cargo/core/resolver/version_prefs.rs
@@ -21,7 +21,7 @@ pub struct VersionPreferences {
     try_to_use: HashSet<PackageId>,
     prefer_patch_deps: HashMap<InternedString, HashSet<Dependency>>,
     version_ordering: VersionOrdering,
-    max_rust_version: Option<PartialVersion>,
+    rust_versions: Vec<PartialVersion>,
 }
 
 #[derive(Copy, Clone, Default, PartialEq, Eq, Hash, Debug)]
@@ -49,8 +49,8 @@ impl VersionPreferences {
         self.version_ordering = ordering;
     }
 
-    pub fn max_rust_version(&mut self, ver: Option<PartialVersion>) {
-        self.max_rust_version = ver;
+    pub fn rust_versions(&mut self, vers: Vec<PartialVersion>) {
+        self.rust_versions = vers;
     }
 
     /// Sort (and filter) the given vector of summaries in-place
@@ -59,7 +59,7 @@ impl VersionPreferences {
     ///
     /// Sort order:
     /// 1. Preferred packages
-    /// 2. [`VersionPreferences::max_rust_version`]
+    /// 2. Most compatible [`VersionPreferences::rust_versions`]
     /// 3. `first_version`, falling back to [`VersionPreferences::version_ordering`] when `None`
     ///
     /// Filtering:
@@ -85,20 +85,11 @@ impl VersionPreferences {
                 return previous_cmp;
             }
 
-            if let Some(max_rust_version) = &self.max_rust_version {
-                let a_is_compat = a
-                    .rust_version()
-                    .map(|a| a.is_compatible_with(max_rust_version))
-                    .unwrap_or(true);
-                let b_is_compat = b
-                    .rust_version()
-                    .map(|b| b.is_compatible_with(max_rust_version))
-                    .unwrap_or(true);
-                match (a_is_compat, b_is_compat) {
-                    (true, true) => {}   // fallback
-                    (false, false) => {} // fallback
-                    (true, false) => return Ordering::Less,
-                    (false, true) => return Ordering::Greater,
+            if !self.rust_versions.is_empty() {
+                let a_compat_count = self.msrv_compat_count(a);
+                let b_compat_count = self.msrv_compat_count(b);
+                if b_compat_count != a_compat_count {
+                    return b_compat_count.cmp(&a_compat_count);
                 }
             }
 
@@ -112,6 +103,17 @@ impl VersionPreferences {
             let _ = summaries.split_off(1);
         }
     }
+
+    fn msrv_compat_count(&self, summary: &Summary) -> usize {
+        let Some(rust_version) = summary.rust_version() else {
+            return self.rust_versions.len();
+        };
+
+        self.rust_versions
+            .iter()
+            .filter(|max| rust_version.is_compatible_with(max))
+            .count()
+    }
 }
 
 #[cfg(test)]
@@ -238,7 +240,7 @@ mod test {
     #[test]
     fn test_single_rust_version() {
         let mut vp = VersionPreferences::default();
-        vp.max_rust_version(Some("1.50".parse().unwrap()));
+        vp.rust_versions(vec!["1.50".parse().unwrap()]);
 
         let mut summaries = vec![
             summ("foo", "1.2.4", None),
@@ -270,7 +272,7 @@ mod test {
     #[test]
     fn test_multiple_rust_versions() {
         let mut vp = VersionPreferences::default();
-        vp.max_rust_version(Some("1.45".parse().unwrap()));
+        vp.rust_versions(vec!["1.45".parse().unwrap(), "1.55".parse().unwrap()]);
 
         let mut summaries = vec![
             summ("foo", "1.2.4", None),
@@ -286,7 +288,7 @@ mod test {
         vp.sort_summaries(&mut summaries, None);
         assert_eq!(
             describe(&summaries),
-            "foo/1.2.4, foo/1.2.2, foo/1.2.0, foo/1.1.0, foo/1.0.9, foo/1.2.3, foo/1.2.1"
+            "foo/1.2.4, foo/1.2.2, foo/1.2.0, foo/1.1.0, foo/1.0.9, foo/1.2.1, foo/1.2.3"
                 .to_string()
         );
 
diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs
@@ -79,6 +79,7 @@ use crate::util::errors::CargoResult;
 use crate::util::CanonicalUrl;
 use anyhow::Context as _;
 use cargo_util::paths;
+use cargo_util_schemas::core::PartialVersion;
 use std::collections::{HashMap, HashSet};
 use tracing::{debug, trace};
 
@@ -357,14 +358,16 @@ pub fn resolve_with_previous<'gctx>(
         version_prefs.version_ordering(VersionOrdering::MinimumVersionsFirst)
     }
     if ws.resolve_honors_rust_version() {
-        let rust_version = if let Some(ver) = ws.lowest_rust_version() {
-            ver.clone().into_partial()
-        } else {
+        let mut rust_versions: Vec<_> = ws
+            .members()
+            .filter_map(|p| p.rust_version().map(|rv| rv.as_partial().clone()))
+            .collect();
+        if rust_versions.is_empty() {
             let rustc = ws.gctx().load_global_rustc(Some(ws))?;
-            let rustc_version = rustc.version.clone().into();
-            rustc_version
-        };
-        version_prefs.max_rust_version(Some(rust_version));
+            let rust_version: PartialVersion = rustc.version.clone().into();
+            rust_versions.push(rust_version);
+        }
+        version_prefs.rust_versions(rust_versions);
     }
 
     let avoid_patch_ids = if register_patches {
diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs
@@ -510,7 +510,7 @@ higher v0.0.1 ([ROOT]/foo)
         .with_stderr_data(str![[r#"
 [UPDATING] `dummy-registry` index
 [LOCKING] 6 packages to latest Rust 1.50.0 compatible versions
-[ADDING] higher-newer-and-older v1.65.0 (requires Rust 1.65.0)
+[ADDING] higher-newer-and-older v1.55.0 (available: v1.65.0, requires Rust 1.65.0)
 [ADDING] higher-only-newer v1.65.0 (requires Rust 1.65.0)
 [ADDING] lower-newer-and-older v1.45.0 (available: v1.55.0, requires Rust 1.55.0)
 [ADDING] lower-only-newer v1.65.0 (requires Rust 1.65.0)
@@ -522,7 +522,7 @@ higher v0.0.1 ([ROOT]/foo)
     p.cargo("tree")
         .with_stdout_data(str![[r#"
 higher v0.0.1 ([ROOT]/foo)
-├── higher-newer-and-older v1.65.0
+├── higher-newer-and-older v1.55.0
 ├── higher-only-newer v1.65.0
 ├── shared-newer-and-older v1.45.0
 └── shared-only-newer v1.65.0
