From 699b30a5f40549e3f394ae5ae738e34c0dbf6664 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 9 Oct 2023 15:48:53 -0500 Subject: [PATCH 1/3] test(install): Verify existing top-level MSRV behavior --- tests/testsuite/install.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 0a3670e6c8b..5312354d291 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -2463,3 +2463,30 @@ For more information, try '--help'. .with_status(1) .run(); } + +#[cargo_test] +fn install_incompat_msrv() { + Package::new("foo", "0.1.0") + .file("src/main.rs", "fn main() {}") + .rust_version("1.30") + .publish(); + Package::new("foo", "0.2.0") + .file("src/main.rs", "fn main() {}") + .rust_version("1.9876.0") + .publish(); + + cargo_process("install foo") + .with_stderr("\ +[UPDATING] `dummy-registry` index +[DOWNLOADING] crates ... +[DOWNLOADED] foo v0.2.0 (registry `[..]`) +[INSTALLING] foo v0.2.0 +[ERROR] failed to compile `foo v0.2.0`, intermediate artifacts can be found at `[..]`. +To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path. + +Caused by: + package `foo v0.2.0` cannot be built because it requires rustc 1.9876.0 or newer, while the currently active rustc version is [..] + Try re-running cargo install with `--locked` +") + .with_status(101).run(); +} From 2976e2ac6634f56b1e03537ddb18f4357dcce018 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 9 Oct 2023 16:31:30 -0500 Subject: [PATCH 2/3] fix(install): Don't suggest --locked for MSRV when its root package This will also report the error without having to download the `.crate` first. If installing multiple packages, this will also report it immediately, rather than waiting for the other packages to be installed first. This also offers us more flexibility in the error we report, like suggesting more appropriate fixes. --- src/cargo/ops/cargo_install.rs | 42 +++++++++++++++++-- src/cargo/ops/cargo_uninstall.rs | 1 + .../ops/common_for_install_and_uninstall.rs | 20 +++++++-- tests/testsuite/install.rs | 10 +---- 4 files changed, 57 insertions(+), 16 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 957ab43e6e6..16027233edb 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -68,6 +68,7 @@ impl<'cfg> InstallablePackage<'cfg> { force: bool, no_track: bool, needs_update_if_source_is_index: bool, + current_rust_version: Option<&semver::Version>, ) -> CargoResult> { if let Some(name) = krate { if name == "." { @@ -105,6 +106,7 @@ impl<'cfg> InstallablePackage<'cfg> { dep, |git: &mut GitSource<'_>| git.read_packages(), config, + current_rust_version, )? } else if source_id.is_path() { let mut src = path_source(source_id, config)?; @@ -142,6 +144,7 @@ impl<'cfg> InstallablePackage<'cfg> { dep, |path: &mut PathSource<'_>| path.read_packages(), config, + current_rust_version, )? } else if let Some(dep) = dep { let mut source = map.load(source_id, &HashSet::new())?; @@ -161,7 +164,13 @@ impl<'cfg> InstallablePackage<'cfg> { config.shell().status("Ignored", &msg)?; return Ok(None); } - select_dep_pkg(&mut source, dep, config, needs_update_if_source_is_index)? + select_dep_pkg( + &mut source, + dep, + config, + needs_update_if_source_is_index, + current_rust_version, + )? } else { bail!( "must specify a crate to install from \ @@ -616,6 +625,21 @@ pub fn install( let dst = root.join("bin").into_path_unlocked(); let map = SourceConfigMap::new(config)?; + let current_rust_version = if opts.honor_rust_version { + let rustc = config.load_global_rustc(None)?; + + // Remove any pre-release identifiers for easier comparison + let current_version = &rustc.version; + let untagged_version = semver::Version::new( + current_version.major, + current_version.minor, + current_version.patch, + ); + Some(untagged_version) + } else { + None + }; + let (installed_anything, scheduled_error) = if krates.len() <= 1 { let (krate, vers) = krates .iter() @@ -623,7 +647,18 @@ pub fn install( .map(|(k, v)| (Some(k.as_str()), v.as_ref())) .unwrap_or((None, None)); let installable_pkg = InstallablePackage::new( - config, root, map, krate, source_id, from_cwd, vers, opts, force, no_track, true, + config, + root, + map, + krate, + source_id, + from_cwd, + vers, + opts, + force, + no_track, + true, + current_rust_version.as_ref(), )?; let mut installed_anything = true; if let Some(installable_pkg) = installable_pkg { @@ -654,6 +689,7 @@ pub fn install( force, no_track, !did_update, + current_rust_version.as_ref(), ) { Ok(Some(installable_pkg)) => { did_update = true; @@ -773,7 +809,7 @@ where // expensive network call in the case that the package is already installed. // If this fails, the caller will possibly do an index update and try again, this is just a // best-effort check to see if we can avoid hitting the network. - if let Ok(pkg) = select_dep_pkg(source, dep, config, false) { + if let Ok(pkg) = select_dep_pkg(source, dep, config, false, None) { let (_ws, rustc, target) = make_ws_rustc_target(config, opts, &source.source_id(), pkg.clone())?; if let Ok(true) = is_installed(&pkg, config, opts, &rustc, &target, root, dst, force) { diff --git a/src/cargo/ops/cargo_uninstall.rs b/src/cargo/ops/cargo_uninstall.rs index 3551544183b..1f22e191e80 100644 --- a/src/cargo/ops/cargo_uninstall.rs +++ b/src/cargo/ops/cargo_uninstall.rs @@ -90,6 +90,7 @@ fn uninstall_cwd(root: &Filesystem, bins: &[String], config: &Config) -> CargoRe None, |path: &mut PathSource<'_>| path.read_packages(), config, + None, )?; let pkgid = pkg.package_id(); uninstall_pkgid(root, tracker, pkgid, bins, config) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 45c74ab3c11..7b3ca0534f1 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -532,6 +532,7 @@ pub fn select_dep_pkg( dep: Dependency, config: &Config, needs_update: bool, + current_rust_version: Option<&semver::Version>, ) -> CargoResult where T: Source, @@ -551,9 +552,19 @@ where Poll::Pending => source.block_until_ready()?, } }; - match deps.iter().map(|p| p.package_id()).max() { - Some(pkgid) => { - let pkg = Box::new(source).download_now(pkgid, config)?; + match deps.iter().max_by_key(|p| p.package_id()) { + Some(summary) => { + if let (Some(current), Some(msrv)) = (current_rust_version, summary.rust_version()) { + let msrv_req = msrv.caret_req(); + if !msrv_req.matches(current) { + let name = summary.name(); + let ver = summary.version(); + bail!("\ +cannot install package `{name} {ver}`, it requires rustc {msrv} or newer, while the currently active rustc version is {current}" +) + } + } + let pkg = Box::new(source).download_now(summary.package_id(), config)?; Ok(pkg) } None => { @@ -599,6 +610,7 @@ pub fn select_pkg( dep: Option, mut list_all: F, config: &Config, + current_rust_version: Option<&semver::Version>, ) -> CargoResult where T: Source, @@ -612,7 +624,7 @@ where source.invalidate_cache(); return if let Some(dep) = dep { - select_dep_pkg(source, dep, config, false) + select_dep_pkg(source, dep, config, false, current_rust_version) } else { let candidates = list_all(source)?; let binaries = candidates diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 5312354d291..cb1539eb8db 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -2478,15 +2478,7 @@ fn install_incompat_msrv() { cargo_process("install foo") .with_stderr("\ [UPDATING] `dummy-registry` index -[DOWNLOADING] crates ... -[DOWNLOADED] foo v0.2.0 (registry `[..]`) -[INSTALLING] foo v0.2.0 -[ERROR] failed to compile `foo v0.2.0`, intermediate artifacts can be found at `[..]`. -To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path. - -Caused by: - package `foo v0.2.0` cannot be built because it requires rustc 1.9876.0 or newer, while the currently active rustc version is [..] - Try re-running cargo install with `--locked` +[ERROR] cannot install package `foo 0.2.0`, it requires rustc 1.9876.0 or newer, while the currently active rustc version is [..] ") .with_status(101).run(); } From 9b32be7f89485abf5a7ab2df40c074fdc0275956 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 9 Oct 2023 20:22:54 -0500 Subject: [PATCH 3/3] fix(install): Suggest an alternative version on MSRV failure The next step would be to also automatically install an MSRV compatible version if compatible with the version req (#10903). This improved error message will still be useful if the MSRV compatible version is outside of the version req. I did this as the first step - Helps people now, not needing to wait on `-Zmsrv-policy` to be stabilized - Has fewer questions on how it should be done (or if it should be) --- .../ops/common_for_install_and_uninstall.rs | 39 ++++++++++++++++++- tests/testsuite/install.rs | 1 + 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 7b3ca0534f1..9dc823bbc38 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -559,8 +559,45 @@ where if !msrv_req.matches(current) { let name = summary.name(); let ver = summary.version(); + let extra = if dep.source_id().is_registry() { + // Match any version, not just the selected + let msrv_dep = + Dependency::parse(dep.package_name(), None, dep.source_id())?; + let msrv_deps = loop { + match source.query_vec(&msrv_dep, QueryKind::Exact)? { + Poll::Ready(deps) => break deps, + Poll::Pending => source.block_until_ready()?, + } + }; + if let Some(alt) = msrv_deps + .iter() + .filter(|summary| { + summary + .rust_version() + .map(|msrv| msrv.caret_req().matches(current)) + .unwrap_or(true) + }) + .max_by_key(|s| s.package_id()) + { + if let Some(rust_version) = alt.rust_version() { + format!( + "\n`{name} {}` supports rustc {rust_version}", + alt.version() + ) + } else { + format!( + "\n`{name} {}` has an unspecified minimum rustc version", + alt.version() + ) + } + } else { + String::new() + } + } else { + String::new() + }; bail!("\ -cannot install package `{name} {ver}`, it requires rustc {msrv} or newer, while the currently active rustc version is {current}" +cannot install package `{name} {ver}`, it requires rustc {msrv} or newer, while the currently active rustc version is {current}{extra}" ) } } diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index cb1539eb8db..3f0720faa2e 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -2479,6 +2479,7 @@ fn install_incompat_msrv() { .with_stderr("\ [UPDATING] `dummy-registry` index [ERROR] cannot install package `foo 0.2.0`, it requires rustc 1.9876.0 or newer, while the currently active rustc version is [..] +`foo 0.1.0` supports rustc 1.30 ") .with_status(101).run(); }