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

fix(config): warn if host is incompatible with the toolchain in rustup default #3980

Merged
merged 12 commits into from
Aug 5, 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
38 changes: 28 additions & 10 deletions src/cli/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,18 @@ use git_testament::{git_testament, render_testament};
use tracing::{debug, error, info, trace, warn};

use super::self_update;
use crate::cli::download_tracker::DownloadTracker;
use crate::dist::{
manifest::ComponentStatus, notifications as dist_notifications, TargetTriple, ToolchainDesc,
use crate::{
rami3l marked this conversation as resolved.
Show resolved Hide resolved
cli::download_tracker::DownloadTracker,
config::Cfg,
dist::{
manifest::ComponentStatus, notifications as dist_notifications, TargetTriple, ToolchainDesc,
},
install::UpdateStatus,
notifications::Notification,
process::{terminalsource, Process},
toolchain::{DistributableToolchain, LocalToolchainName, Toolchain, ToolchainName},
utils::{notifications as util_notifications, notify::NotificationLevel, utils},
};
use crate::install::UpdateStatus;
use crate::process::{terminalsource, Process};
use crate::toolchain::{DistributableToolchain, LocalToolchainName, Toolchain, ToolchainName};
use crate::utils::notifications as util_notifications;
use crate::utils::notify::NotificationLevel;
use crate::utils::utils;
use crate::{config::Cfg, notifications::Notification};

pub(crate) const WARN_COMPLETE_PROFILE: &str = "downloading with complete profile isn't recommended unless you are a developer of the rust language";

Expand Down Expand Up @@ -633,6 +634,23 @@ 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,
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(())
}

/// Warns if rustup is running under emulation, such as macOS Rosetta
pub(crate) fn warn_if_host_is_emulated(process: &Process) {
if TargetTriple::is_host_emulated() {
Expand Down
17 changes: 6 additions & 11 deletions src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use anyhow::{anyhow, Error, Result};
use clap::{builder::PossibleValue, Args, CommandFactory, Parser, Subcommand, ValueEnum};
use clap_complete::Shell;
use itertools::Itertools;
use tracing::{error, info, trace, warn};
use tracing::{info, trace, warn};

use crate::{
cli::{
Expand Down Expand Up @@ -717,7 +717,10 @@ async fn default_(
}
MaybeResolvableToolchainName::Some(ResolvableToolchainName::Official(toolchain)) => {
let desc = toolchain.resolve(&cfg.get_default_host_triple()?)?;
let status = DistributableToolchain::install_if_not_installed(cfg, &desc).await?;
let status = cfg
.ensure_installed(&desc, vec![], vec![], None, true)
.await?
.0;

cfg.set_default(Some(&(&desc).into()))?;

Expand Down Expand Up @@ -814,16 +817,8 @@ async fn update(cfg: &mut Cfg<'_>, opts: UpdateOpts) -> Result<utils::ExitCode>
// This needs another pass to fix it all up
if name.has_triple() {
let host_arch = TargetTriple::from_host_or_build(cfg.process);

let target_triple = name.clone().resolve(&host_arch)?.target;
if !forced && !host_arch.can_run(&target_triple)? {
error!("DEPRECATED: future versions of rustup will require --force-non-host to install a non-host toolchain.");
warn!("toolchain '{name}' 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 {}` instead?",
target_triple.to_string()
);
}
common::warn_if_host_is_incompatible(&name, &host_arch, &target_triple, forced)?;
}
let desc = name.resolve(&cfg.get_default_host_triple()?)?;

Expand Down
83 changes: 52 additions & 31 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ use tokio_stream::StreamExt;
use tracing::trace;

use crate::{
cli::self_update::SelfUpdateMode,
dist::{self, download::DownloadCfg, temp, PartialToolchainDesc, Profile, ToolchainDesc},
cli::{common, self_update::SelfUpdateMode},
dist::{
self, download::DownloadCfg, temp, PartialToolchainDesc, Profile, TargetTriple,
ToolchainDesc,
},
errors::RustupError,
fallback_settings::FallbackSettings,
install::UpdateStatus,
Expand Down Expand Up @@ -120,7 +123,7 @@ enum OverrideCfg {
toolchain: ToolchainDesc,
components: Vec<String>,
targets: Vec<String>,
profile: Option<dist::Profile>,
profile: Option<Profile>,
},
}

Expand Down Expand Up @@ -172,7 +175,7 @@ impl OverrideCfg {
.toolchain
.profile
.as_deref()
.map(dist::Profile::from_str)
.map(Profile::from_str)
.transpose()?,
}
}
Expand Down Expand Up @@ -227,7 +230,7 @@ impl From<LocalToolchainName> for OverrideCfg {
pub(crate) const UNIX_FALLBACK_SETTINGS: &str = "/etc/rustup/settings.toml";

pub(crate) struct Cfg<'a> {
profile_override: Option<dist::Profile>,
profile_override: Option<Profile>,
pub rustup_dir: PathBuf,
pub settings_file: SettingsFile,
pub fallback_settings: Option<FallbackSettings>,
Expand Down Expand Up @@ -345,7 +348,7 @@ impl<'a> Cfg<'a> {
}
}

pub(crate) fn set_profile_override(&mut self, profile: dist::Profile) {
pub(crate) fn set_profile_override(&mut self, profile: Profile) {
self.profile_override = Some(profile);
}

Expand Down Expand Up @@ -388,7 +391,7 @@ impl<'a> Cfg<'a> {
// if there is no profile in the settings file. The last variant happens when
// a user upgrades from a version of Rustup without profiles to a version of
// Rustup with profiles.
pub(crate) fn get_profile(&self) -> Result<dist::Profile> {
pub(crate) fn get_profile(&self) -> Result<Profile> {
if let Some(p) = self.profile_override {
return Ok(p);
}
Expand Down Expand Up @@ -752,8 +755,9 @@ impl<'a> Cfg<'a> {
profile,
} => {
let toolchain = self
.ensure_installed(toolchain, components, targets, profile)
.await?;
.ensure_installed(&toolchain, components, targets, profile, false)
.await?
.1;
Ok((toolchain, reason))
}
},
Expand All @@ -767,8 +771,9 @@ impl<'a> Cfg<'a> {
Some(ToolchainName::Official(toolchain_desc)) => {
let reason = ActiveReason::Default;
let toolchain = self
.ensure_installed(toolchain_desc, vec![], vec![], None)
.await?;
.ensure_installed(&toolchain_desc, vec![], vec![], None, false)
.await?
.1;
Ok((toolchain, reason))
}
},
Expand All @@ -777,40 +782,56 @@ impl<'a> Cfg<'a> {

// Returns a Toolchain matching the given ToolchainDesc, installing it and
// the given components and targets if they aren't already installed.
async fn ensure_installed(
#[tracing::instrument(level = "trace", err(level = "trace"), skip_all)]
pub(crate) async fn ensure_installed(
&self,
toolchain: ToolchainDesc,
toolchain: &ToolchainDesc,
components: Vec<String>,
targets: Vec<String>,
profile: Option<Profile>,
) -> Result<Toolchain<'_>> {
verbose: bool,
) -> Result<(UpdateStatus, Toolchain<'_>)> {
common::warn_if_host_is_incompatible(
toolchain,
&TargetTriple::from_host_or_build(self.process),
&toolchain.target,
false,
)?;
if verbose {
rami3l marked this conversation as resolved.
Show resolved Hide resolved
(self.notify_handler)(Notification::LookingForToolchain(toolchain));
}
let components: Vec<_> = components.iter().map(AsRef::as_ref).collect();
let targets: Vec<_> = targets.iter().map(AsRef::as_ref).collect();
let toolchain = match DistributableToolchain::new(self, toolchain.clone()) {
let profile = match profile {
Some(profile) => profile,
None => self.get_profile()?,
};
let (status, toolchain) = match DistributableToolchain::new(self, toolchain.clone()) {
Err(RustupError::ToolchainNotInstalled(_)) => {
DistributableToolchain::install(
self,
&toolchain,
toolchain,
&components,
&targets,
profile.unwrap_or(Profile::Default),
profile,
false,
)
.await?
.1
}
Ok(mut distributable) => {
if !distributable.components_exist(&components, &targets)? {
distributable
.update(&components, &targets, profile.unwrap_or(Profile::Default))
.await?;
if verbose {
(self.notify_handler)(Notification::UsingExistingToolchain(toolchain));
}
distributable
let status = if !distributable.components_exist(&components, &targets)? {
distributable.update(&components, &targets, profile).await?
} else {
UpdateStatus::Unchanged
};
(status, distributable)
}
Err(e) => return Err(e.into()),
}
.into();
Ok(toolchain)
};
Ok((status, toolchain.into()))
}

/// Get the configured default toolchain.
Expand Down Expand Up @@ -913,15 +934,15 @@ impl<'a> Cfg<'a> {
// against the 'stable' toolchain. This provides early errors
// if the supplied triple is insufficient / bad.
dist::PartialToolchainDesc::from_str("stable")?
.resolve(&dist::TargetTriple::new(host_triple.clone()))?;
.resolve(&TargetTriple::new(host_triple.clone()))?;
self.settings_file.with_mut(|s| {
s.default_host_triple = Some(host_triple);
Ok(())
})
}

#[tracing::instrument(level = "trace", skip_all)]
pub(crate) fn get_default_host_triple(&self) -> Result<dist::TargetTriple> {
pub(crate) fn get_default_host_triple(&self) -> Result<TargetTriple> {
self.settings_file
.with(|s| Ok(get_default_host_triple(s, self.process)))
}
Expand Down Expand Up @@ -989,11 +1010,11 @@ impl<'a> Debug for Cfg<'a> {
}
}

fn get_default_host_triple(s: &Settings, process: &Process) -> dist::TargetTriple {
fn get_default_host_triple(s: &Settings, process: &Process) -> TargetTriple {
s.default_host_triple
.as_ref()
.map(dist::TargetTriple::new)
.unwrap_or_else(|| dist::TargetTriple::from_host_or_build(process))
.map(TargetTriple::new)
.unwrap_or_else(|| TargetTriple::from_host_or_build(process))
}

fn non_empty_env_var(name: &str, process: &Process) -> anyhow::Result<Option<String>> {
Expand Down
19 changes: 0 additions & 19 deletions src/toolchain/distributable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use crate::{
DistOptions, PartialToolchainDesc, Profile, ToolchainDesc,
},
install::{InstallMethod, UpdateStatus},
notifications::Notification,
RustupError,
};

Expand Down Expand Up @@ -358,24 +357,6 @@ impl<'a> DistributableToolchain<'a> {
Ok((status, Self::new(cfg, toolchain.clone())?))
}

#[tracing::instrument(level = "trace", err(level = "trace"), skip_all)]
pub async fn install_if_not_installed(
cfg: &'a Cfg<'a>,
desc: &ToolchainDesc,
) -> anyhow::Result<UpdateStatus> {
(cfg.notify_handler)(Notification::LookingForToolchain(desc));
if Toolchain::exists(cfg, &desc.into())? {
(cfg.notify_handler)(Notification::UsingExistingToolchain(desc));
Ok(UpdateStatus::Unchanged)
} else {
Ok(
Self::install(cfg, desc, &[], &[], cfg.get_profile()?, false)
.await?
.0,
)
}
}

#[tracing::instrument(level = "trace", err(level = "trace"), skip_all)]
pub(crate) async fn update(
&mut self,
Expand Down
13 changes: 13 additions & 0 deletions tests/suite/cli_rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2725,6 +2725,19 @@ warn: If you meant to build software to target that platform, perhaps try `rustu
).await;
}

#[tokio::test]
async fn warn_on_unmatch_build_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;
}

#[tokio::test]
async fn dont_warn_on_partial_build() {
let cx = CliTestContext::new(Scenario::SimpleV2).await;
Expand Down
Loading