Skip to content

Commit

Permalink
Auto merge of #13791 - epage:msrv-unset, r=Eh2406
Browse files Browse the repository at this point in the history
fix(resolver): Treat unset MSRV as compatible

### What does this PR try to resolve?

Have the resolver treat no-MSRV as `rust-version = "*"`, like `cargo add` does for version-requirement selection

### How should we test and review this PR?

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>`.

See also #9930 (comment)

r? `@Eh2406`

### Additional information
  • Loading branch information
bors committed May 1, 2024
2 parents e591b0e + 89ead6f commit 82dca28
Showing 1 changed file with 15 additions and 32 deletions.
47 changes: 15 additions & 32 deletions src/cargo/core/resolver/version_prefs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -271,15 +254,15 @@ 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()
);

vp.version_ordering(VersionOrdering::MinimumVersionsFirst);
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()
);
}
Expand Down

0 comments on commit 82dca28

Please sign in to comment.