From 89ead6f60eaf733d232eb18db0e31da04ba8b88d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 23 Apr 2024 10:21:41 -0500 Subject: [PATCH] fix(resolver): Treat unset MSRV as compatible We last tweaked this logic in #13066. However, we noticed this was inconsistent with `cargo add` in automatically selecting version requirements. It looks like this is a revert of #13066, taking us back to the behavior in #12950. In #12950 there was a concern about the proliferation of no-MSRV and whether we should de-prioritize those to make the chance of success more likely. There are no right answes here, only which wrong answer is ok enough. - Do we treat lack of rust version as `rust-version = "*"` as some people expect or do we try to be smart? - If a user adds or removes `rust-version`, how should that affect the priority? One piece of new information is that the RFC for this has us trying to fill the no-MSRV gap with `rust-version = some-value-representing-the-current-toolchain>`. --- src/cargo/core/resolver/version_prefs.rs | 47 ++++++++---------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/src/cargo/core/resolver/version_prefs.rs b/src/cargo/core/resolver/version_prefs.rs index 5e6cc230ffb..c89c13fdd4b 100644 --- a/src/cargo/core/resolver/version_prefs.rs +++ b/src/cargo/core/resolver/version_prefs.rs @@ -86,36 +86,19 @@ impl VersionPreferences { } 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.is_compatible_with(max_rust_version); - let b_is_compat = b.is_compatible_with(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.is_compatible_with(max_rust_version) { - return Ordering::Greater; - } else { - return Ordering::Less; - } - } - (Some(a), None) => { - if a.is_compatible_with(max_rust_version) { - return Ordering::Less; - } else { - return Ordering::Greater; - } - } + 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, } } @@ -271,7 +254,7 @@ mod test { vp.sort_summaries(&mut summaries, None); assert_eq!( describe(&summaries), - "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" + "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() ); @@ -279,7 +262,7 @@ mod test { vp.sort_summaries(&mut summaries, None); assert_eq!( describe(&summaries), - "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" + "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() ); }