From c5e820abd989129e80fdf384a720b91a57210715 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Mar 2024 12:16:38 -0600 Subject: [PATCH] fix(msrv): Report all incompatible packages, not just a random one In #9930, it recommended improving the error for incompatible packages so people can better get to the root of the problem. For example, you might get an error about `clap_lex` but resolving the error for the higher level `clap` could make the problem with `clap_lex` go away. Because I generally saw earlier packages in the graph reported, I assumed we were reporting these errors bottom up. It turns out, we are reporting them in the `UnitGraph`s order, which is non-deterministic because it is built on a `HashMap`. So this adds determinism and shows all incompatible dependencies (not just the bottom or the root). This is a first step. We might find that we still want to only include the shallowest units, rather than all. At that point, we can add the complexity to address this by walking the unit graph. We could also further improve this by querying the index to suggest compatible versions of packages. --- src/cargo/ops/cargo_compile/mod.rs | 59 ++++++++++++++++------------ tests/testsuite/rust_version.rs | 63 +++++++++++++++++++++++++++--- 2 files changed, 91 insertions(+), 31 deletions(-) diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index b2a26ee0a8a0..081e9825369a 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -487,6 +487,8 @@ pub fn create_bcx<'a, 'gctx>( current_version.patch, ); + let mut incompatible = Vec::new(); + let mut local_incompatible = false; for unit in unit_graph.keys() { let Some(version) = unit.pkg.rust_version() else { continue; @@ -497,35 +499,40 @@ pub fn create_bcx<'a, 'gctx>( continue; } - let guidance = if ws.is_ephemeral() { + local_incompatible |= unit.is_local(); + incompatible.push((unit, version)); + } + if !incompatible.is_empty() { + use std::fmt::Write as _; + + let plural = if incompatible.len() == 1 { "" } else { "s" }; + let mut message = format!( + "rustc {current_version} is not supported by the following package{plural}:\n" + ); + incompatible.sort_by_key(|(unit, _)| (unit.pkg.name(), unit.pkg.version())); + for (unit, msrv) in incompatible { + let name = &unit.pkg.name(); + let version = &unit.pkg.version(); + writeln!(&mut message, " {name}@{version} requires rustc {msrv}").unwrap(); + } + if ws.is_ephemeral() { if ws.ignore_lock() { - "Try re-running cargo install with `--locked`".to_string() - } else { - String::new() + writeln!( + &mut message, + "Try re-running `cargo install` with `--locked`" + ) + .unwrap(); } - } else if !unit.is_local() { - format!( - "Either upgrade to rustc {} or newer, or use\n\ - cargo update {}@{} --precise ver\n\ - where `ver` is the latest version of `{}` supporting rustc {}", - version, - unit.pkg.name(), - unit.pkg.version(), - unit.pkg.name(), - current_version, + } else if !local_incompatible { + writeln!( + &mut message, + "Either upgrade rustc or select compatible dependency versions with +`cargo update @ --precise ` +where `` is the latest version supporting rustc {current_version}", ) - } else { - String::new() - }; - - anyhow::bail!( - "package `{}` cannot be built because it requires rustc {} or newer, \ - while the currently active rustc version is {}\n{}", - unit.pkg, - version, - current_version, - guidance, - ); + .unwrap(); + } + return Err(anyhow::Error::msg(message)); } } diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs index 6c2d01708c4b..fd269ff36a95 100644 --- a/tests/testsuite/rust_version.rs +++ b/tests/testsuite/rust_version.rs @@ -180,7 +180,8 @@ fn rust_version_too_high() { .with_status(101) .with_stderr( "\ -[ERROR] package `foo v0.0.1 ([..])` cannot be built because it requires rustc 1.9876.0 or newer, while the currently active rustc version is [..] +[ERROR] rustc [..] is not supported by the following package: + foo@0.0.1 requires rustc 1.9876.0 ", ) @@ -218,10 +219,62 @@ fn dependency_rust_version_newer_than_rustc() { [UPDATING] `[..]` index [DOWNLOADING] crates ... [DOWNLOADED] bar v0.0.1 (registry `[..]`) -[ERROR] package `bar v0.0.1` cannot be built because it requires rustc 1.2345.0 or newer, while the currently active rustc version is [..] -Either upgrade to rustc 1.2345.0 or newer, or use -cargo update bar@0.0.1 --precise ver -where `ver` is the latest version of `bar` supporting rustc [..]", +[ERROR] rustc [..] is not supported by the following package: + bar@0.0.1 requires rustc 1.2345.0 +Either upgrade rustc or select compatible dependency versions with +`cargo update @ --precise ` +where `` is the latest version supporting rustc [..] + +", + ) + .run(); + p.cargo("check --ignore-rust-version").run(); +} + +#[cargo_test] +fn dependency_tree_rust_version_newer_than_rustc() { + Package::new("baz", "0.0.1") + .dep("bar", "0.0.1") + .rust_version("1.2345.0") + .file("src/lib.rs", "fn other_stuff() {}") + .publish(); + Package::new("bar", "0.0.1") + .rust_version("1.2345.0") + .file("src/lib.rs", "fn other_stuff() {}") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + authors = [] + [dependencies] + baz = "0.0.1" + "#, + ) + .file("src/main.rs", "fn main(){}") + .build(); + + p.cargo("check") + .with_status(101) + .with_stderr( + "\ +[UPDATING] `[..]` index +[DOWNLOADING] crates ... +[DOWNLOADED] baz v0.0.1 (registry `[..]`) +[DOWNLOADED] bar v0.0.1 (registry `[..]`) +[ERROR] rustc [..] is not supported by the following packages: + bar@0.0.1 requires rustc 1.2345.0 + baz@0.0.1 requires rustc 1.2345.0 +Either upgrade rustc or select compatible dependency versions with +`cargo update @ --precise ` +where `` is the latest version supporting rustc [..] + +", ) .run(); p.cargo("check --ignore-rust-version").run();