From 6fac2ab6d9362036b6b5ddba3ac0206fd57a267c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 19 Sep 2024 13:53:42 -0500 Subject: [PATCH] fix(resolve): Improve multi-MSRV workspaces 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 --- src/cargo/core/resolver/version_prefs.rs | 44 +++++++++++++----------- src/cargo/ops/resolve.rs | 17 +++++---- tests/testsuite/rust_version.rs | 4 +-- 3 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/cargo/core/resolver/version_prefs.rs b/src/cargo/core/resolver/version_prefs.rs index 016adaa1beea..61f17404936b 100644 --- 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, prefer_patch_deps: HashMap>, version_ordering: VersionOrdering, - max_rust_version: Option, + rust_versions: Vec, } #[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) { - self.max_rust_version = ver; + pub fn rust_versions(&mut self, vers: Vec) { + 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 index ddd2334d9af7..223833ac8c3d 100644 --- 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 index bd9395a4ab26..c36416e64af8 100644 --- 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