From 0bc5d22285b3ec4c4cd1547d67cec0265d937ab8 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 14 Nov 2023 14:56:53 -0600 Subject: [PATCH 1/2] test(resolver): Verify backtracking for MSRV resolver --- tests/testsuite/rust_version.rs | 55 +++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs index 21321b7c579..51dd9ef307f 100644 --- a/tests/testsuite/rust_version.rs +++ b/tests/testsuite/rust_version.rs @@ -332,6 +332,61 @@ fn dependency_rust_version_older_and_newer_than_package() { .run(); } +#[cargo_test] +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("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() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + rust-version = "1.60.0" + [dependencies] + no-rust-version = "2" + "#, + ) + .file("src/main.rs", "fn main(){}") + .build(); + + p.cargo("check --ignore-rust-version") + .arg("-Zmsrv-policy") + .masquerade_as_nightly_cargo(&["msrv-policy"]) + .with_stderr( + "\ +[UPDATING] `dummy-registry` index +[DOWNLOADING] crates ... +[DOWNLOADED] no-rust-version v2.1.0 (registry `dummy-registry`) +[CHECKING] no-rust-version v2.1.0 +[CHECKING] [..] +[FINISHED] [..] +", + ) + .run(); + p.cargo("check") + .arg("-Zmsrv-policy") + .masquerade_as_nightly_cargo(&["msrv-policy"]) + .with_stderr( + "\ +[FINISHED] [..] +", + ) + .run(); +} + #[cargo_test] fn workspace_with_mixed_rust_version() { Package::new("bar", "1.4.0") From 0d29d3f71b2229d8f8749c566bd83dd862c13e67 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 6 Nov 2023 21:22:08 -0600 Subject: [PATCH 2/2] fix(resolver): Prefer MSRV, rather than ignore incompatible 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 --- src/cargo/core/resolver/version_prefs.rs | 20 +++++++---- .../cargo_add/rust_version_ignore/mod.rs | 2 +- .../cargo_add/rust_version_ignore/stderr.log | 5 --- tests/testsuite/rust_version.rs | 34 ++++--------------- 4 files changed, 20 insertions(+), 41 deletions(-) diff --git a/src/cargo/core/resolver/version_prefs.rs b/src/cargo/core/resolver/version_prefs.rs index 0deef55654a..9fd13534de1 100644 --- a/src/cargo/core/resolver/version_prefs.rs +++ b/src/cargo/core/resolver/version_prefs.rs @@ -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, @@ -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()); @@ -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(), @@ -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() ); } } diff --git a/tests/testsuite/cargo_add/rust_version_ignore/mod.rs b/tests/testsuite/cargo_add/rust_version_ignore/mod.rs index f8aac0ad83f..0404d12b4ba 100644 --- a/tests/testsuite/cargo_add/rust_version_ignore/mod.rs +++ b/tests/testsuite/cargo_add/rust_version_ignore/mod.rs @@ -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")); diff --git a/tests/testsuite/cargo_add/rust_version_ignore/stderr.log b/tests/testsuite/cargo_add/rust_version_ignore/stderr.log index 96bcbddc2a8..430abe31b47 100644 --- a/tests/testsuite/cargo_add/rust_version_ignore/stderr.log +++ b/tests/testsuite/cargo_add/rust_version_ignore/stderr.log @@ -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? diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs index 51dd9ef307f..d0ea33c8377 100644 --- a/tests/testsuite/rust_version.rs +++ b/tests/testsuite/rust_version.rs @@ -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(); } @@ -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] [..] ",