Skip to content

Commit

Permalink
Auto merge of rust-lang#12798 - epage:msrv-install, r=ehuss
Browse files Browse the repository at this point in the history
fix(install): Suggest an alternative version on MSRV failure

### What does this PR try to resolve?

Moves users from a bad error message, suggesting `--locked` which won't do anything, to suggesting a version of a package to use instead.

The side benefit is errors get reported sooner
- Before downloading the `.crate`
- When installing multiple packages, before building the first

This comes at the cost of an extra `rustc` invocation.

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

Per-commit this builds it up, from tests to the final design.

### Additional information

This is also written in a way to align fairly well with how we'd likely implement rust-lang#10903.
This improved error message will still be useful after that issue is resolved when the MSRV compatible version is outside of the version req.
  • Loading branch information
bors committed Oct 10, 2023
2 parents e4fe8f0 + 9b32be7 commit 8ba1c31
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 7 deletions.
42 changes: 39 additions & 3 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<Self>> {
if let Some(name) = krate {
if name == "." {
Expand Down Expand Up @@ -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)?;
Expand Down Expand Up @@ -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())?;
Expand All @@ -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 \
Expand Down Expand Up @@ -616,14 +625,40 @@ 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()
.next()
.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 {
Expand Down Expand Up @@ -654,6 +689,7 @@ pub fn install(
force,
no_track,
!did_update,
current_rust_version.as_ref(),
) {
Ok(Some(installable_pkg)) => {
did_update = true;
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
57 changes: 53 additions & 4 deletions src/cargo/ops/common_for_install_and_uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,7 @@ pub fn select_dep_pkg<T>(
dep: Dependency,
config: &Config,
needs_update: bool,
current_rust_version: Option<&semver::Version>,
) -> CargoResult<Package>
where
T: Source,
Expand All @@ -551,9 +552,56 @@ 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();
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}{extra}"
)
}
}
let pkg = Box::new(source).download_now(summary.package_id(), config)?;
Ok(pkg)
}
None => {
Expand Down Expand Up @@ -599,6 +647,7 @@ pub fn select_pkg<T, F>(
dep: Option<Dependency>,
mut list_all: F,
config: &Config,
current_rust_version: Option<&semver::Version>,
) -> CargoResult<Package>
where
T: Source,
Expand All @@ -612,7 +661,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
Expand Down
20 changes: 20 additions & 0 deletions tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2463,3 +2463,23 @@ 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
[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();
}

0 comments on commit 8ba1c31

Please sign in to comment.