Skip to content

Commit 5dfe9bf

Browse files
committed
Auto merge of #13066 - epage:msrv, r=Eh2406
fix(resolver): De-prioritize no-rust-version in MSRV resolver ### What does this PR try to resolve? This is a corner case without a good answer. As such, this change leans on some happy-path entries existing and preferring those. ### How should we test and review this PR? ### Additional information This was originally discussed around the time of #12950 but was held off. When working on this, I was considering other heuristics like - If a future version has an MSRV, assume that it applies also to the current version - This can be added in the future - We likely would want to consider an alternative value, like inferring the rust-version from the manifest or the rust-version used from publish - Sort no-MSRV versions of a package by minimal versions - The lower the version, the more likely it is to be compatible - This likely could apply to incompatible MSRVs (or we could reverse-sort those by rust-version) but those will error anyways without `--ignore-rust-version`, so I decided against these - I realized this was a backdoor to minimal versions for dependencies without a MSRV and that the community support isn't there for that yet to be a high enough quality of an experience
2 parents 26333c7 + 1b97a5c commit 5dfe9bf

File tree

1 file changed

+40
-10
lines changed

1 file changed

+40
-10
lines changed

src/cargo/core/resolver/version_prefs.rs

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,37 @@ impl VersionPreferences {
8484
return previous_cmp;
8585
}
8686

87-
if self.max_rust_version.is_some() {
88-
let msrv_a = a.rust_version() <= self.max_rust_version.as_ref();
89-
let msrv_b = b.rust_version() <= self.max_rust_version.as_ref();
90-
let msrv_cmp = msrv_a.cmp(&msrv_b).reverse();
91-
if msrv_cmp != Ordering::Equal {
92-
return msrv_cmp;
87+
if let Some(max_rust_version) = &self.max_rust_version {
88+
match (a.rust_version(), b.rust_version()) {
89+
// Fallback
90+
(None, None) => {}
91+
(Some(a), Some(b)) if a == b => {}
92+
// Primary comparison
93+
(Some(a), Some(b)) => {
94+
let a_is_compat = a <= max_rust_version;
95+
let b_is_compat = b <= max_rust_version;
96+
match (a_is_compat, b_is_compat) {
97+
(true, true) => {} // fallback
98+
(false, false) => {} // fallback
99+
(true, false) => return Ordering::Less,
100+
(false, true) => return Ordering::Greater,
101+
}
102+
}
103+
// Prioritize `None` over incompatible
104+
(None, Some(b)) => {
105+
if b <= max_rust_version {
106+
return Ordering::Greater;
107+
} else {
108+
return Ordering::Less;
109+
}
110+
}
111+
(Some(a), None) => {
112+
if a <= max_rust_version {
113+
return Ordering::Less;
114+
} else {
115+
return Ordering::Greater;
116+
}
117+
}
93118
}
94119
}
95120

@@ -232,8 +257,11 @@ mod test {
232257
vp.max_rust_version(Some("1.50".parse().unwrap()));
233258

234259
let mut summaries = vec![
235-
summ("foo", "1.2.4", Some("1.60")),
236-
summ("foo", "1.2.3", Some("1.50")),
260+
summ("foo", "1.2.4", None),
261+
summ("foo", "1.2.3", Some("1.60")),
262+
summ("foo", "1.2.2", None),
263+
summ("foo", "1.2.1", Some("1.50")),
264+
summ("foo", "1.2.0", None),
237265
summ("foo", "1.1.0", Some("1.40")),
238266
summ("foo", "1.0.9", None),
239267
];
@@ -242,14 +270,16 @@ mod test {
242270
vp.sort_summaries(&mut summaries, None);
243271
assert_eq!(
244272
describe(&summaries),
245-
"foo/1.2.3, foo/1.1.0, foo/1.0.9, foo/1.2.4".to_string()
273+
"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"
274+
.to_string()
246275
);
247276

248277
vp.version_ordering(VersionOrdering::MinimumVersionsFirst);
249278
vp.sort_summaries(&mut summaries, None);
250279
assert_eq!(
251280
describe(&summaries),
252-
"foo/1.0.9, foo/1.1.0, foo/1.2.3, foo/1.2.4".to_string()
281+
"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"
282+
.to_string()
253283
);
254284
}
255285
}

0 commit comments

Comments
 (0)