Skip to content

fix(errors)!: improve error messages for RustupError::ToolchainNotInstalled #4258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 19, 2025
+79 −75
Merged

fix(errors)!: improve error messages for RustupError::ToolchainNotInstalled #4258

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bb51e2a
refactor(cfg): extract `Cfg::active_toolchain()`
rami3l Mar 19, 2025
014a6d0
fix(errors)!: improve error messages for `RustupError::ToolchainNotIn…
rami3l Mar 19, 2025
c69f8f9
refactor(config)!: rename `Cfg::find_active_toolchain()` to `maybe_en…
rami3l Mar 19, 2025
344e9f7
refactor(config)!: rename `Cfg::find_or_install_active_toolchain()` t…
rami3l Mar 19, 2025
c2ff01d
refactor(cli/rustup-mode): use `Cfg::active_toolchain()` for notes
rami3l Mar 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -675,17 +675,13 @@ impl<'a> Cfg<'a> {

// XXX: this awkwardness deals with settings file being locked already
let toolchain_name = toolchain_name.resolve(&default_host_triple)?;
match Toolchain::new(self, (&toolchain_name).into()) {
Copy link
Member Author

@rami3l rami3l Mar 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have manually inlined Toolchain::new() here and removed the statement causing an infinite recursion, because it was not needed; in fact, to allow any RustupError::ToolchainNotInstalled to happen with this call, the toolchain must not exist:

rustup/src/toolchain.rs

Lines 106 to 115 in f9edccd

pub(crate) fn new(cfg: &'a Cfg<'a>, name: LocalToolchainName) -> Result<Self, RustupError> {
let path = cfg.toolchain_path(&name);
if !Toolchain::exists(cfg, &name)? {
return Err(match name {
LocalToolchainName::Named(name) => RustupError::ToolchainNotInstalled { name },
LocalToolchainName::Path(name) => RustupError::PathToolchainNotInstalled(name),
});
}
Ok(Self { cfg, name, path })
}

Err(RustupError::ToolchainNotInstalled { .. }) => {
if matches!(toolchain_name, ToolchainName::Custom(_)) {
bail!(
"custom toolchain specified in override file '{}' is not installed",
toolchain_file.display()
)
}
}
Ok(_) => {}
Err(e) => Err(e)?,
if !Toolchain::exists(self, &(&toolchain_name).into())?
&& matches!(toolchain_name, ToolchainName::Custom(_))
{
bail!(
"custom toolchain specified in override file '{}' is not installed",
toolchain_file.display()
)
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,16 @@ pub enum RustupError {
#[error(
"toolchain '{name}' is not installed{}",
if let ToolchainName::Official(t) = name {
format!("\nhelp: run `rustup toolchain install {t}` to install it")
let t = if *is_active { "" } else { &format!(" {t}") };
format!("\nhelp: run `rustup toolchain install{t}` to install it")
} else {
String::new()
},
)]
ToolchainNotInstalled { name: ToolchainName },
ToolchainNotInstalled {
name: ToolchainName,
is_active: bool,
},
#[error("path '{0}' not found")]
PathToolchainNotInstalled(PathBasedToolchainName),
#[error(
Expand Down
6 changes: 5 additions & 1 deletion src/toolchain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ impl<'a> Toolchain<'a> {
Ok(tc) => Ok(tc),
Err(RustupError::ToolchainNotInstalled {
name: ToolchainName::Official(desc),
..
}) if install_if_missing => {
Ok(
DistributableToolchain::install(cfg, &desc, &[], &[], cfg.get_profile()?, true)
Expand Down Expand Up @@ -107,7 +108,10 @@ impl<'a> Toolchain<'a> {
let path = cfg.toolchain_path(&name);
if !Toolchain::exists(cfg, &name)? {
return Err(match name {
LocalToolchainName::Named(name) => RustupError::ToolchainNotInstalled { name },
LocalToolchainName::Named(name) => {
let is_active = matches!(cfg.active_toolchain(), Ok(Some((t, _))) if t == name);
RustupError::ToolchainNotInstalled { name, is_active }
}
LocalToolchainName::Path(name) => RustupError::PathToolchainNotInstalled(name),
});
}
Expand Down
30 changes: 30 additions & 0 deletions tests/suite/cli_exact.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use rustup::for_host;
use rustup::test::{
CROSS_ARCH1, CROSS_ARCH2, CliTestContext, MULTI_ARCH1, Scenario, this_host_triple,
};
use rustup::utils::raw;

#[tokio::test]
async fn update_once() {
Expand Down Expand Up @@ -699,6 +700,35 @@ help: run `rustup toolchain install nightly-{0}` to install it
.await;
}

// issue #4212
#[tokio::test]
async fn show_suggestion_for_missing_toolchain_with_components() {
let cx = CliTestContext::new(Scenario::SimpleV2).await;

let cwd = cx.config.current_dir();
let toolchain_file = cwd.join("rust-toolchain.toml");
raw::write_file(
&toolchain_file,
r#"
[toolchain]
channel = "stable"
components = [ "rust-src" ]
"#,
)
.unwrap();
cx.config
.expect_err_env(
&["cargo", "fmt"],
&[("RUSTUP_AUTO_INSTALL", "0")],
for_host!(
r"error: toolchain 'stable-{0}' is not installed
help: run `rustup toolchain install` to install it
"
),
)
.await;
}

// issue #927
#[tokio::test]
async fn undefined_linked_toolchain() {
Expand Down