Skip to content

Commit

Permalink
Auto merge of #12950 - epage:msrv, r=Eh2406
Browse files Browse the repository at this point in the history
fix(resolver): Prefer MSRV, rather than ignore incompatible

### What does this PR try to resolve?

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)

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

### Additional information

Note: `--ignore-rust-version` is not yet implemented for the resolver.

This builds on #12930
  • Loading branch information
bors committed Nov 14, 2023
2 parents 87ee3e9 + 0d29d3f commit 0fcc90d
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 37 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?
81 changes: 57 additions & 24 deletions tests/testsuite/rust_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,49 +245,81 @@ fn dependency_rust_version_newer_than_package() {
.file("src/main.rs", "fn main(){}")
.build();

p.cargo("check")
.arg("-Zmsrv-policy")
.masquerade_as_nightly_cargo(&["msrv-policy"])
.run();
p.cargo("check --ignore-rust-version")
.arg("-Zmsrv-policy")
.masquerade_as_nightly_cargo(&["msrv-policy"])
// This shouldn't fail
.with_status(101)
.run();
}

#[cargo_test]
fn dependency_rust_version_older_and_newer_than_package() {
Package::new("bar", "1.5.0")
.rust_version("1.55.0")
.file("src/lib.rs", "fn other_stuff() {}")
.publish();
Package::new("bar", "1.6.0")
.rust_version("1.65.0")
.file("src/lib.rs", "fn other_stuff() {}")
.publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
authors = []
rust-version = "1.60.0"
[dependencies]
bar = "1.0.0"
"#,
)
.file("src/main.rs", "fn main(){}")
.build();

p.cargo("check --ignore-rust-version")
.arg("-Zmsrv-policy")
.masquerade_as_nightly_cargo(&["msrv-policy"])
// This should pick 1.6.0
.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?
[DOWNLOADING] crates ...
[DOWNLOADED] bar v1.5.0 (registry `dummy-registry`)
[CHECKING] bar v1.5.0
[CHECKING] [..]
[FINISHED] [..]
",
)
.run();
p.cargo("check")
.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?
[FINISHED] [..]
",
)
.run();
}

#[cargo_test]
fn dependency_rust_version_older_and_newer_than_package() {
Package::new("bar", "1.5.0")
.rust_version("1.55.0")
fn dependency_rust_version_backtracking() {
Package::new("has-rust-version", "1.6.0")
.rust_version("1.65.0")
.file("src/lib.rs", "fn other_stuff() {}")
.publish();
Package::new("bar", "1.6.0")
.rust_version("1.65.0")
Package::new("no-rust-version", "2.1.0")
.file("src/lib.rs", "fn other_stuff() {}")
.publish();
Package::new("no-rust-version", "2.2.0")
.file("src/lib.rs", "fn other_stuff() {}")
.dep("has-rust-version", "1.6.0")
.publish();

let p = project()
Expand All @@ -300,7 +332,7 @@ fn dependency_rust_version_older_and_newer_than_package() {
authors = []
rust-version = "1.60.0"
[dependencies]
bar = "1.0.0"
no-rust-version = "2"
"#,
)
.file("src/main.rs", "fn main(){}")
Expand All @@ -309,13 +341,14 @@ fn dependency_rust_version_older_and_newer_than_package() {
p.cargo("check --ignore-rust-version")
.arg("-Zmsrv-policy")
.masquerade_as_nightly_cargo(&["msrv-policy"])
// This should pick 1.6.0
.with_stderr(
"\
[UPDATING] `dummy-registry` index
[DOWNLOADING] crates ...
[DOWNLOADED] bar v1.5.0 (registry `dummy-registry`)
[CHECKING] bar v1.5.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 0fcc90d

Please sign in to comment.