Skip to content

Commit 7573248

Browse files
authored
Warn when installing with a non-default toolchain (#16131)
This PR implements the cargo changes for #11036. The rustup changes were implemented in rust-lang/rustup#4518. The PR is currently organized as four commits: - The first expands the rustup tests to set `RUSTUP_TOOLCHAIN_SOURCE`. This change is not strictly necessary, but it improves the rustup tests' fidelity. - The second moves the `pkg` function to a location where the next commit can use it. - The third implements a test to check the fourth commit's fix. - The fourth warns when `cargo install` is invoked with a non-default toolchain, as indicated by `RUSTUP_TOOLCHAIN_SOURCE`. In the third commit, two significant changes were made to `simulated_rustup_environment`: - Previously, the constructed proxy would call an always-panicking executable whenever it was asked to run cargo. Now, the proxy can be made to run the cargo under test. - The proxy can now be made to set additional environment variables (e.g., `RUSTUP_TOOLCHAIN_SOURCE`) before calling the proxied tool. The PR is currently marked as draft because rust-lang/rustup#4518 has not yet been released. Technically, this PR could be merged before then. But testing it with a fixed rustup would seem to make sense. Nits are welcome. cc: @epage
2 parents 3e9f677 + 8f464a5 commit 7573248

File tree

2 files changed

+381
-59
lines changed

2 files changed

+381
-59
lines changed

src/cargo/ops/cargo_install.rs

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
22
use std::path::{Path, PathBuf};
33
use std::sync::Arc;
4-
use std::{env, fs};
4+
use std::{env, fmt, fs};
55

66
use crate::core::compiler::{CompileKind, DefaultExecutor, Executor, UnitOutput};
77
use crate::core::{Dependency, Edition, Package, PackageId, SourceId, Target, Workspace};
@@ -14,12 +14,14 @@ use crate::util::errors::CargoResult;
1414
use crate::util::{Filesystem, GlobalContext, Rustc};
1515
use crate::{drop_println, ops};
1616

17+
use annotate_snippets::Level;
1718
use anyhow::{Context as _, bail};
1819
use cargo_util::paths;
1920
use cargo_util_schemas::core::PartialVersion;
2021
use itertools::Itertools;
2122
use semver::VersionReq;
2223
use tempfile::Builder as TempFileBuilder;
24+
use tracing::debug;
2325

2426
struct Transaction {
2527
bins: Vec<PathBuf>,
@@ -39,6 +41,42 @@ impl Drop for Transaction {
3941
}
4042
}
4143

44+
enum RustupToolchainSource {
45+
Default,
46+
Environment,
47+
CommandLine,
48+
OverrideDB,
49+
ToolchainFile,
50+
Other(String),
51+
}
52+
53+
#[allow(dead_code)]
54+
impl RustupToolchainSource {
55+
fn is_implicit_override(&self) -> Option<bool> {
56+
match self {
57+
Self::Default => Some(false),
58+
Self::Environment => Some(true),
59+
Self::CommandLine => Some(false),
60+
Self::OverrideDB => Some(true),
61+
Self::ToolchainFile => Some(true),
62+
Self::Other(_) => None,
63+
}
64+
}
65+
}
66+
67+
impl fmt::Display for RustupToolchainSource {
68+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
69+
f.write_str(match self {
70+
Self::Default => "default",
71+
Self::Environment => "environment variable",
72+
Self::CommandLine => "command line",
73+
Self::OverrideDB => "rustup directory override",
74+
Self::ToolchainFile => "rustup toolchain file",
75+
Self::Other(other) => other,
76+
})
77+
}
78+
}
79+
4280
struct InstallablePackage<'gctx> {
4381
gctx: &'gctx GlobalContext,
4482
opts: ops::CompileOptions,
@@ -309,6 +347,31 @@ impl<'gctx> InstallablePackage<'gctx> {
309347
fn install_one(mut self, dry_run: bool) -> CargoResult<bool> {
310348
self.gctx.shell().status("Installing", &self.pkg)?;
311349

350+
if let Some(source) = get_rustup_toolchain_source()
351+
&& source.is_implicit_override().unwrap_or_else(|| {
352+
debug!("ignoring unrecognized rustup toolchain source `{source}`");
353+
false
354+
})
355+
{
356+
#[expect(clippy::disallowed_methods, reason = "consistency with rustup")]
357+
let maybe_toolchain = env::var("RUSTUP_TOOLCHAIN")
358+
.ok()
359+
.map(|toolchain| format!(" with `{toolchain}`"))
360+
.unwrap_or_default();
361+
let report = &[Level::WARNING
362+
.secondary_title(format!(
363+
"default toolchain implicitly overridden{maybe_toolchain} by {source}"
364+
))
365+
.element(Level::HELP.message(format!(
366+
"use `cargo +stable install` if you meant to use the stable toolchain"
367+
)))
368+
.element(Level::NOTE.message(format!(
369+
"rustup selects the toolchain based on the parent environment and not the \
370+
environment of the package being installed"
371+
)))];
372+
self.gctx.shell().print_report(report, false)?;
373+
}
374+
312375
// Normalize to absolute path for consistency throughout.
313376
// See: https://github.com/rust-lang/cargo/issues/16023
314377
let dst = self.root.join("bin").into_path_unlocked();
@@ -604,6 +667,20 @@ impl<'gctx> InstallablePackage<'gctx> {
604667
}
605668
}
606669

670+
fn get_rustup_toolchain_source() -> Option<RustupToolchainSource> {
671+
#[expect(clippy::disallowed_methods, reason = "consistency with rustup")]
672+
let source = std::env::var("RUSTUP_TOOLCHAIN_SOURCE").ok()?;
673+
let source = match source.as_str() {
674+
"default" => RustupToolchainSource::Default,
675+
"env" => RustupToolchainSource::Environment,
676+
"cli" => RustupToolchainSource::CommandLine,
677+
"path-override" => RustupToolchainSource::OverrideDB,
678+
"toolchain-file" => RustupToolchainSource::ToolchainFile,
679+
other => RustupToolchainSource::Other(other.to_owned()),
680+
};
681+
Some(source)
682+
}
683+
607684
fn make_warning_about_missing_features(binaries: &[&Target]) -> String {
608685
let max_targets_listed = 7;
609686
let target_features_message = binaries

0 commit comments

Comments
 (0)