Skip to content
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.

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

refactor(cli/common)!: deny installing a host-incompatible toolchain w/o --force-non-host #4028

Merged
merged 5 commits into from
Sep 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 12 additions & 8 deletions src/cli/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use crate::{
dist::{
manifest::ComponentStatus, notifications as dist_notifications, TargetTriple, ToolchainDesc,
},
errors::RustupError,
install::UpdateStatus,
notifications::Notification,
process::{terminalsource, Process},
Expand Down Expand Up @@ -626,21 +627,24 @@ pub(crate) fn ignorable_error(
}
}

/// Warns if rustup is trying to install a toolchain that might not be
/// able to run on the host system.
pub(crate) fn warn_if_host_is_incompatible(
toolchain: impl Display,
/// Returns an error for a toolchain if both conditions are met:
/// - The toolchain has an incompatible target triple,
/// i.e. it might not be able to run on the host system.
/// - The `force_non_host` flag is set to `false`.
pub(crate) fn check_non_host_toolchain(
toolchain: String,
host_arch: &TargetTriple,
target_triple: &TargetTriple,
force_non_host: bool,
) -> Result<()> {
if force_non_host || host_arch.can_run(target_triple)? {
return Ok(());
}
error!("DEPRECATED: future versions of rustup will require --force-non-host to install a non-host toolchain.");
warn!("toolchain '{toolchain}' may not be able to run on this system.");
warn!("If you meant to build software to target that platform, perhaps try `rustup target add {target_triple}` instead?");
Ok(())
Err(RustupError::ToolchainIncompatible {
toolchain,
target_triple: target_triple.clone(),
}
.into())
}

/// Warns if rustup is running under emulation, such as macOS Rosetta
Expand Down
25 changes: 20 additions & 5 deletions src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ enum RustupSubcmd {
Default {
#[arg(help = MAYBE_RESOLVABLE_TOOLCHAIN_ARG_HELP)]
toolchain: Option<MaybeResolvableToolchainName>,

/// Install toolchains that require an emulator. See https://github.com/rust-lang/rustup/wiki/Non-host-toolchains
#[arg(long)]
force_non_host: bool,
},

/// Modify or query the installed toolchains
Expand Down Expand Up @@ -631,7 +635,10 @@ pub async fn main(
ToolchainSubcmd::Uninstall { opts } => toolchain_remove(cfg, opts),
},
RustupSubcmd::Check => check_updates(cfg).await,
RustupSubcmd::Default { toolchain } => default_(cfg, toolchain).await,
RustupSubcmd::Default {
toolchain,
force_non_host,
} => default_(cfg, toolchain, force_non_host).await,
RustupSubcmd::Target { subcmd } => match subcmd {
TargetSubcmd::List {
toolchain,
Expand Down Expand Up @@ -710,6 +717,7 @@ pub async fn main(
async fn default_(
cfg: &Cfg<'_>,
toolchain: Option<MaybeResolvableToolchainName>,
force_non_host: bool,
) -> Result<utils::ExitCode> {
common::warn_if_host_is_emulated(cfg.process);

Expand All @@ -725,7 +733,7 @@ async fn default_(
MaybeResolvableToolchainName::Some(ResolvableToolchainName::Official(toolchain)) => {
let desc = toolchain.resolve(&cfg.get_default_host_triple()?)?;
let status = cfg
.ensure_installed(&desc, vec![], vec![], None, true)
.ensure_installed(&desc, vec![], vec![], None, force_non_host, true)
.await?
.0;

Expand Down Expand Up @@ -814,7 +822,7 @@ async fn update(
let self_update = !self_update::NEVER_SELF_UPDATE
&& self_update_mode == SelfUpdateMode::Enable
&& !opts.no_self_update;
let forced = opts.force_non_host;
let force_non_host = opts.force_non_host;
if let Some(p) = opts.profile {
cfg.set_profile_override(p);
}
Expand All @@ -829,7 +837,12 @@ async fn update(
if name.has_triple() {
let host_arch = TargetTriple::from_host_or_build(cfg.process);
let target_triple = name.clone().resolve(&host_arch)?.target;
common::warn_if_host_is_incompatible(&name, &host_arch, &target_triple, forced)?;
common::check_non_host_toolchain(
name.to_string(),
&host_arch,
&target_triple,
force_non_host,
)?;
}
let desc = name.resolve(&cfg.get_default_host_triple()?)?;

Expand Down Expand Up @@ -873,7 +886,9 @@ async fn update(
exit_code &= common::self_update(|| Ok(()), cfg.process).await?;
}
} else if ensure_active_toolchain {
let (toolchain, reason) = cfg.find_or_install_active_toolchain(true).await?;
let (toolchain, reason) = cfg
.find_or_install_active_toolchain(force_non_host, true)
.await?;
info!("the active toolchain `{toolchain}` has been installed");
info!("it's active because: {reason}");
} else {
Expand Down
21 changes: 15 additions & 6 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,7 @@ impl<'a> Cfg<'a> {
#[tracing::instrument(level = "trace", skip_all)]
pub(crate) async fn find_or_install_active_toolchain(
&self,
force_non_host: bool,
verbose: bool,
) -> Result<(LocalToolchainName, ActiveReason)> {
if let Some((override_config, reason)) = self.find_override_config()? {
Expand All @@ -753,16 +754,23 @@ impl<'a> Cfg<'a> {
profile,
} = override_config
{
self.ensure_installed(&toolchain, components, targets, profile, verbose)
.await?;
self.ensure_installed(
&toolchain,
components,
targets,
profile,
force_non_host,
verbose,
)
.await?;
} else {
Toolchain::with_reason(self, toolchain.clone(), &reason)?;
}
Ok((toolchain, reason))
} else if let Some(toolchain) = self.get_default()? {
let reason = ActiveReason::Default;
if let ToolchainName::Official(desc) = &toolchain {
self.ensure_installed(desc, vec![], vec![], None, verbose)
self.ensure_installed(desc, vec![], vec![], None, force_non_host, verbose)
.await?;
} else {
Toolchain::with_reason(self, toolchain.clone().into(), &reason)?;
Expand All @@ -782,13 +790,14 @@ impl<'a> Cfg<'a> {
components: Vec<String>,
targets: Vec<String>,
profile: Option<Profile>,
force_non_host: bool,
verbose: bool,
) -> Result<(UpdateStatus, Toolchain<'_>)> {
common::warn_if_host_is_incompatible(
toolchain,
common::check_non_host_toolchain(
toolchain.to_string(),
&TargetTriple::from_host_or_build(self.process),
&toolchain.target,
false,
force_non_host,
)?;
if verbose {
(self.notify_handler)(Notification::LookingForToolchain(toolchain));
Expand Down
9 changes: 9 additions & 0 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@ pub enum RustupError {
},
#[error("command failed: '{}'", PathBuf::from(.name).display())]
RunningCommand { name: OsString },
#[error(
"toolchain '{toolchain}' may not be able to run on this system\n\
note: to build software for that platform, try `rustup target add {target_triple}` instead\n\
note: add the `--force-non-host` flag to install the toolchain anyway"
)]
ToolchainIncompatible {
toolchain: String,
target_triple: TargetTriple,
},
#[error("toolchain '{0}' is not installable")]
ToolchainNotInstallable(String),
#[error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@ stdout = """
...
Set the default toolchain

Usage: rustup[EXE] default [TOOLCHAIN]
Usage: rustup[EXE] default [OPTIONS] [TOOLCHAIN]

Arguments:
[TOOLCHAIN] 'none', a toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom toolchain
name. For more information see `rustup help toolchain`

Options:
-h, --help Print help
--force-non-host Install toolchains that require an emulator. See
https://github.com/rust-lang/rustup/wiki/Non-host-toolchains
-h, --help Print help

Discussion:
Sets the default toolchain to the one specified. If the toolchain
Expand Down
2 changes: 1 addition & 1 deletion tests/suite/cli_misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ async fn multi_host_smoke_test() {
let mut cx = CliTestContext::new(Scenario::MultiHost).await;
let toolchain = format!("nightly-{}", clitools::MULTI_ARCH1);
cx.config
.expect_ok(&["rustup", "default", &toolchain])
.expect_ok(&["rustup", "default", &toolchain, "--force-non-host"])
.await;
cx.config
.expect_stdout_ok(&["rustc", "--version"], "xxxx-nightly-2")
Expand Down
39 changes: 23 additions & 16 deletions tests/suite/cli_rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,7 @@ async fn show_multiple_targets() {
"rustup",
"default",
&format!("nightly-{}", clitools::MULTI_ARCH1),
"--force-non-host",
])
.await;
cx.config
Expand Down Expand Up @@ -801,6 +802,7 @@ async fn show_multiple_toolchains_and_targets() {
"rustup",
"default",
&format!("nightly-{}", clitools::MULTI_ARCH1),
"--force-non-host",
])
.await;
cx.config
Expand All @@ -810,6 +812,7 @@ async fn show_multiple_toolchains_and_targets() {
.expect_ok(&[
"rustup",
"update",
"--force-non-host",
&format!("stable-{}", clitools::MULTI_ARCH1),
])
.await;
Expand Down Expand Up @@ -2731,29 +2734,33 @@ async fn check_unix_settings_fallback() {
}

#[tokio::test]
async fn warn_on_unmatch_build() {
async fn deny_incompatible_toolchain_install() {
let cx = CliTestContext::new(Scenario::MultiHost).await;
let arch = clitools::MULTI_ARCH1;
cx.config.expect_stderr_ok(
&["rustup", "toolchain", "install", &format!("nightly-{arch}")],
&format!(
r"warn: toolchain 'nightly-{arch}' may not be able to run on this system.
warn: If you meant to build software to target that platform, perhaps try `rustup target add {arch}` instead?",
),
).await;
cx.config
.expect_err(
&["rustup", "toolchain", "install", &format!("nightly-{arch}")],
&format!(
"error: toolchain 'nightly-{arch}' may not be able to run on this system
note: to build software for that platform, try `rustup target add {arch}` instead",
),
)
.await;
}

#[tokio::test]
async fn warn_on_unmatch_build_default() {
async fn deny_incompatible_toolchain_default() {
let cx = CliTestContext::new(Scenario::MultiHost).await;
let arch = clitools::MULTI_ARCH1;
cx.config.expect_stderr_ok(
&["rustup", "default", &format!("nightly-{arch}")],
&format!(
r"warn: toolchain 'nightly-{arch}' may not be able to run on this system.
warn: If you meant to build software to target that platform, perhaps try `rustup target add {arch}` instead?",
),
).await;
cx.config
.expect_err(
&["rustup", "default", &format!("nightly-{arch}")],
&format!(
"error: toolchain 'nightly-{arch}' may not be able to run on this system
note: to build software for that platform, try `rustup target add {arch}` instead",
),
)
.await;
}

#[tokio::test]
Expand Down
Loading