Skip to content

Commit

Permalink
fix(resolver): Prefer MSRV, rather than ignore incompatible
Browse files Browse the repository at this point in the history
This is another experiment for #9930.

Comparing preferring over exclusively using MSRV compatible:

Benefits
- Better error messages
- `--ignore-rust-version` is implicitly sticky

Downsides
- Can't backtrack for MSRV compatible version
- Still requires workspace-wide MSRV (compared to our desired end state of declaring MSRV as yet another dependency)

This builds on #12930
  • Loading branch information
epage committed Nov 14, 2023
1 parent 0bc5d22 commit 0d29d3f
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 41 deletions.
20 changes: 13 additions & 7 deletions src/cargo/core/resolver/version_prefs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ impl VersionPreferences {
///
/// Sort order:
/// 1. Preferred packages
/// 2. `first_version`, falling back to [`VersionPreferences::version_ordering`] when `None`
/// 2. [`VersionPreferences::max_rust_version`]
/// 3. `first_version`, falling back to [`VersionPreferences::version_ordering`] when `None`
///
/// Filtering:
/// - [`VersionPreferences::max_rust_version`]
/// - `first_version`
pub fn sort_summaries(
&self,
Expand All @@ -76,9 +76,6 @@ impl VersionPreferences {
.map(|deps| deps.iter().any(|d| d.matches_id(*pkg_id)))
.unwrap_or(false)
};
if self.max_rust_version.is_some() {
summaries.retain(|s| s.rust_version() <= self.max_rust_version.as_ref());
}
summaries.sort_unstable_by(|a, b| {
let prefer_a = should_prefer(&a.package_id());
let prefer_b = should_prefer(&b.package_id());
Expand All @@ -87,6 +84,15 @@ 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;
}
}

let cmp = a.version().cmp(b.version());
match first_version.unwrap_or(self.version_ordering) {
VersionOrdering::MaximumVersionsFirst => cmp.reverse(),
Expand Down Expand Up @@ -236,14 +242,14 @@ mod test {
vp.sort_summaries(&mut summaries, None);
assert_eq!(
describe(&summaries),
"foo/1.2.3, foo/1.1.0, foo/1.0.9".to_string()
"foo/1.2.3, foo/1.1.0, foo/1.0.9, foo/1.2.4".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".to_string()
"foo/1.0.9, foo/1.1.0, foo/1.2.3, foo/1.2.4".to_string()
);
}
}
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_add/rust_version_ignore/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ fn case() {
.current_dir(cwd)
.masquerade_as_nightly_cargo(&["msrv-policy"])
.assert()
.code(101)
.code(0)
.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

Expand Down
5 changes: 0 additions & 5 deletions tests/testsuite/cargo_add/rust_version_ignore/stderr.log
Original file line number Diff line number Diff line change
@@ -1,7 +1,2 @@
Updating `dummy-registry` index
Adding rust-version-user v0.2.1 to dependencies.
error: failed to select a version for the requirement `rust-version-user = "^0.2.1"`
candidate versions found which didn't match: 0.2.1, 0.1.0
location searched: `dummy-registry` index (which is replacing registry `crates-io`)
required by package `cargo-list-test-fixture v0.0.0 ([ROOT]/case)`
perhaps a crate was updated and forgotten to be re-vendored?
34 changes: 6 additions & 28 deletions tests/testsuite/rust_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,37 +245,13 @@ fn dependency_rust_version_newer_than_package() {
.file("src/main.rs", "fn main(){}")
.build();

p.cargo("check --ignore-rust-version")
p.cargo("check")
.arg("-Zmsrv-policy")
.masquerade_as_nightly_cargo(&["msrv-policy"])
// This shouldn't fail
.with_status(101)
.with_stderr(
"\
[UPDATING] `dummy-registry` index
[ERROR] failed to select a version for the requirement `bar = \"^1.0.0\"`
candidate versions found which didn't match: 1.6.0
location searched: `dummy-registry` index (which is replacing registry `crates-io`)
required by package `foo v0.0.1 ([CWD])`
perhaps a crate was updated and forgotten to be re-vendored?
",
)
.run();
p.cargo("check")
p.cargo("check --ignore-rust-version")
.arg("-Zmsrv-policy")
.masquerade_as_nightly_cargo(&["msrv-policy"])
.with_status(101)
// This should have a better error message
.with_stderr(
"\
[UPDATING] `dummy-registry` index
[ERROR] failed to select a version for the requirement `bar = \"^1.0.0\"`
candidate versions found which didn't match: 1.6.0
location searched: `dummy-registry` index (which is replacing registry `crates-io`)
required by package `foo v0.0.1 ([CWD])`
perhaps a crate was updated and forgotten to be re-vendored?
",
)
.run();
}

Expand Down Expand Up @@ -369,8 +345,10 @@ fn dependency_rust_version_backtracking() {
"\
[UPDATING] `dummy-registry` index
[DOWNLOADING] crates ...
[DOWNLOADED] no-rust-version v2.1.0 (registry `dummy-registry`)
[CHECKING] no-rust-version v2.1.0
[DOWNLOADED] no-rust-version v2.2.0 (registry `dummy-registry`)
[DOWNLOADED] has-rust-version v1.6.0 (registry `dummy-registry`)
[CHECKING] has-rust-version v1.6.0
[CHECKING] no-rust-version v2.2.0
[CHECKING] [..]
[FINISHED] [..]
",
Expand Down

0 comments on commit 0d29d3f

Please sign in to comment.