Skip to content

Commit

Permalink
Auto merge of #13066 - epage:msrv, r=Eh2406
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Nov 28, 2023
2 parents 26333c7 + 1b97a5c commit 5dfe9bf
Showing 1 changed file with 40 additions and 10 deletions.
50 changes: 40 additions & 10 deletions src/cargo/core/resolver/version_prefs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
}

Expand Down Expand Up @@ -232,8 +257,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),
];
Expand All @@ -242,14 +270,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.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()
);

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.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()
);
}
}

0 comments on commit 5dfe9bf

Please sign in to comment.