From defe789dfa4d31f93998eff68346b2ec0e95f699 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 28 Nov 2023 13:57:51 -0600 Subject: [PATCH 1/2] test(resolver): Add more MSRV test cases --- src/cargo/core/resolver/version_prefs.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/cargo/core/resolver/version_prefs.rs b/src/cargo/core/resolver/version_prefs.rs index 9fd13534de1..f621c62e9d5 100644 --- a/src/cargo/core/resolver/version_prefs.rs +++ b/src/cargo/core/resolver/version_prefs.rs @@ -232,8 +232,11 @@ mod test { 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.2.4", None), + summ("foo", "1.2.3", Some("1.60")), + summ("foo", "1.2.2", None), + summ("foo", "1.2.1", Some("1.50")), + summ("foo", "1.2.0", None), summ("foo", "1.1.0", Some("1.40")), summ("foo", "1.0.9", None), ]; @@ -242,14 +245,16 @@ mod test { vp.sort_summaries(&mut summaries, None); assert_eq!( describe(&summaries), - "foo/1.2.3, foo/1.1.0, foo/1.0.9, foo/1.2.4".to_string() + "foo/1.2.4, foo/1.2.2, foo/1.2.1, foo/1.2.0, foo/1.1.0, foo/1.0.9, foo/1.2.3" + .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, foo/1.2.4".to_string() + "foo/1.0.9, foo/1.1.0, foo/1.2.0, foo/1.2.1, foo/1.2.2, foo/1.2.4, foo/1.2.3" + .to_string() ); } } From 1b97a5c5c00517f338d60ca0b79920a2c6995c50 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 28 Nov 2023 14:14:03 -0600 Subject: [PATCH 2/2] fix(resolver): De-prioritize no-rust-version in MSRV resolver This is a corner case without a good answer. As such, this change leans on some happy-path entries existing and preferring those. --- src/cargo/core/resolver/version_prefs.rs | 41 +++++++++++++++++++----- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/src/cargo/core/resolver/version_prefs.rs b/src/cargo/core/resolver/version_prefs.rs index f621c62e9d5..13c23cfb066 100644 --- a/src/cargo/core/resolver/version_prefs.rs +++ b/src/cargo/core/resolver/version_prefs.rs @@ -84,12 +84,37 @@ impl VersionPreferences { return previous_cmp; } - if self.max_rust_version.is_some() { - let msrv_a = a.rust_version() <= self.max_rust_version.as_ref(); - let msrv_b = b.rust_version() <= self.max_rust_version.as_ref(); - let msrv_cmp = msrv_a.cmp(&msrv_b).reverse(); - if msrv_cmp != Ordering::Equal { - return msrv_cmp; + if let Some(max_rust_version) = &self.max_rust_version { + match (a.rust_version(), b.rust_version()) { + // Fallback + (None, None) => {} + (Some(a), Some(b)) if a == b => {} + // Primary comparison + (Some(a), Some(b)) => { + let a_is_compat = a <= max_rust_version; + let b_is_compat = b <= max_rust_version; + match (a_is_compat, b_is_compat) { + (true, true) => {} // fallback + (false, false) => {} // fallback + (true, false) => return Ordering::Less, + (false, true) => return Ordering::Greater, + } + } + // Prioritize `None` over incompatible + (None, Some(b)) => { + if b <= max_rust_version { + return Ordering::Greater; + } else { + return Ordering::Less; + } + } + (Some(a), None) => { + if a <= max_rust_version { + return Ordering::Less; + } else { + return Ordering::Greater; + } + } } } @@ -245,7 +270,7 @@ mod test { vp.sort_summaries(&mut summaries, None); assert_eq!( describe(&summaries), - "foo/1.2.4, foo/1.2.2, foo/1.2.1, foo/1.2.0, foo/1.1.0, foo/1.0.9, foo/1.2.3" + "foo/1.2.1, foo/1.1.0, foo/1.2.4, foo/1.2.2, foo/1.2.0, foo/1.0.9, foo/1.2.3" .to_string() ); @@ -253,7 +278,7 @@ mod test { vp.sort_summaries(&mut summaries, None); assert_eq!( describe(&summaries), - "foo/1.0.9, foo/1.1.0, foo/1.2.0, foo/1.2.1, foo/1.2.2, foo/1.2.4, foo/1.2.3" + "foo/1.1.0, foo/1.2.1, foo/1.0.9, foo/1.2.0, foo/1.2.2, foo/1.2.4, foo/1.2.3" .to_string() ); }