Skip to content

Commit

Permalink
fix(install): Don't suggest --locked for MSRV when its root package
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
epage committed Oct 9, 2023
1 parent 699b30a commit cc2d88c
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 16 deletions.
1 change: 1 addition & 0 deletions src/bin/cargo/commands/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
&compile_opts,
args.flag("force"),
args.flag("no-track"),
!args.flag("ignore-rust-version"),
)?;
}
Ok(())
Expand Down
43 changes: 40 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 @@ -611,19 +620,46 @@ pub fn install(
opts: &ops::CompileOptions,
force: bool,
no_track: bool,
honor_rust_version: bool,
) -> CargoResult<()> {
let root = resolve_root(root, config)?;
let dst = root.join("bin").into_path_unlocked();
let map = SourceConfigMap::new(config)?;

let current_rust_version = if 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 +690,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 +810,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
20 changes: 16 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,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 => {
Expand Down Expand Up @@ -599,6 +610,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 +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
Expand Down
10 changes: 1 addition & 9 deletions tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

0 comments on commit cc2d88c

Please sign in to comment.