From aeaf3f7d3f063f47af3ce51a025c8dc16d73784d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 6 Nov 2023 21:22:08 -0600 Subject: [PATCH] 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] [..] ",