Skip to content

Commit

Permalink
fix(resolver): Treat unset MSRV as compatible
Browse files Browse the repository at this point in the history
We last tweaked this logic in rust-lang#13066.
However, we noticed this was inconsistent with `cargo add` in
automatically selecting version requirements.

It looks like this is a revert of rust-lang#13066, taking us back to the behavior
in rust-lang#12950.
In rust-lang#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>`.
  • Loading branch information
epage committed Apr 23, 2024
1 parent e31c27d commit 89ead6f
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 89ead6f

Please sign in to comment.