From b46e24cb60dad4d69870e91128c0aaeaafd9cf7d Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 4 Mar 2025 15:44:19 +0100 Subject: [PATCH 1/2] config: make Config::find_active_toolchain() async --- src/cli/common.rs | 4 +-- src/cli/proxy_mode.rs | 5 +++- src/cli/rustup_mode.rs | 51 +++++++++++++++++++--------------- src/config.rs | 23 +++++++-------- src/toolchain/distributable.rs | 6 ++-- 5 files changed, 51 insertions(+), 38 deletions(-) diff --git a/src/cli/common.rs b/src/cli/common.rs index 8c37d4ac80..931b0c7dbf 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -406,7 +406,7 @@ pub(super) fn list_items( Ok(utils::ExitCode(0)) } -pub(crate) fn list_toolchains( +pub(crate) async fn list_toolchains( cfg: &Cfg<'_>, verbose: bool, quiet: bool, @@ -418,7 +418,7 @@ pub(crate) fn list_toolchains( let default_toolchain_name = cfg.get_default()?; let active_toolchain_name: Option = if let Ok(Some((LocalToolchainName::Named(toolchain), _reason))) = - cfg.find_active_toolchain() + cfg.find_active_toolchain().await { Some(toolchain) } else { diff --git a/src/cli/proxy_mode.rs b/src/cli/proxy_mode.rs index 650767fb44..b453f8f7d3 100644 --- a/src/cli/proxy_mode.rs +++ b/src/cli/proxy_mode.rs @@ -32,6 +32,9 @@ pub async fn main(arg0: &str, current_dir: PathBuf, process: &Process) -> Result .collect(); let cfg = set_globals(current_dir, true, process)?; - let cmd = cfg.resolve_local_toolchain(toolchain)?.command(arg0)?; + let cmd = cfg + .resolve_local_toolchain(toolchain) + .await? + .command(arg0)?; run_command_for_dir(cmd, arg0, &cmd_args) } diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 43a867cb1e..1ec9632596 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -560,7 +560,7 @@ pub async fn main( write!(process.stdout().lock(), "{err}")?; info!("This is the version for the rustup toolchain manager, not the rustc compiler."); let mut cfg = common::set_globals(current_dir, true, process)?; - match cfg.active_rustc_version() { + match cfg.active_rustc_version().await { Ok(Some(version)) => info!("The currently active `rustc` version is `{version}`"), Ok(None) => info!("No `rustc` is currently active"), Err(err) => trace!("Failed to display the current `rustc` version: {err}"), @@ -601,10 +601,12 @@ pub async fn main( match subcmd { RustupSubcmd::DumpTestament => common::dump_testament(process), RustupSubcmd::Install { opts } => update(cfg, opts, true).await, - RustupSubcmd::Uninstall { opts } => toolchain_remove(cfg, opts), + RustupSubcmd::Uninstall { opts } => toolchain_remove(cfg, opts).await, RustupSubcmd::Show { verbose, subcmd } => handle_epipe(match subcmd { - None => show(cfg, verbose), - Some(ShowSubcmd::ActiveToolchain { verbose }) => show_active_toolchain(cfg, verbose), + None => show(cfg, verbose).await, + Some(ShowSubcmd::ActiveToolchain { verbose }) => { + show_active_toolchain(cfg, verbose).await + } Some(ShowSubcmd::Home) => show_rustup_home(cfg), Some(ShowSubcmd::Profile) => { writeln!(process.stdout().lock(), "{}", cfg.get_profile()?)?; @@ -633,12 +635,12 @@ pub async fn main( RustupSubcmd::Toolchain { subcmd } => match subcmd { ToolchainSubcmd::Install { opts } => update(cfg, opts, true).await, ToolchainSubcmd::List { verbose, quiet } => { - handle_epipe(common::list_toolchains(cfg, verbose, quiet)) + handle_epipe(common::list_toolchains(cfg, verbose, quiet).await) } ToolchainSubcmd::Link { toolchain, path } => { toolchain_link(cfg, &toolchain, &path).await } - ToolchainSubcmd::Uninstall { opts } => toolchain_remove(cfg, opts), + ToolchainSubcmd::Uninstall { opts } => toolchain_remove(cfg, opts).await, }, RustupSubcmd::Check => check_updates(cfg).await, RustupSubcmd::Default { @@ -751,7 +753,7 @@ async fn default_( } }; - if let Some((toolchain, reason)) = cfg.find_active_toolchain()? { + if let Some((toolchain, reason)) = cfg.find_active_toolchain().await? { if !matches!(reason, ActiveReason::Default) { info!("note that the toolchain '{toolchain}' is currently in use ({reason})"); } @@ -928,7 +930,7 @@ async fn which( binary: &str, toolchain: Option, ) -> Result { - let binary_path = cfg.resolve_toolchain(toolchain)?.binary_file(binary); + let binary_path = cfg.resolve_toolchain(toolchain).await?.binary_file(binary); utils::assert_is_file(&binary_path)?; @@ -937,7 +939,7 @@ async fn which( } #[tracing::instrument(level = "trace", skip_all)] -fn show(cfg: &Cfg<'_>, verbose: bool) -> Result { +async fn show(cfg: &Cfg<'_>, verbose: bool) -> Result { common::warn_if_host_is_emulated(cfg.process); // Print host triple @@ -962,7 +964,7 @@ fn show(cfg: &Cfg<'_>, verbose: bool) -> Result { let installed_toolchains = cfg.list_toolchains()?; let active_toolchain_and_reason: Option<(ToolchainName, ActiveReason)> = if let Ok(Some((LocalToolchainName::Named(toolchain_name), reason))) = - cfg.find_active_toolchain() + cfg.find_active_toolchain().await { Some((toolchain_name, reason)) } else { @@ -1081,8 +1083,8 @@ fn show(cfg: &Cfg<'_>, verbose: bool) -> Result { } #[tracing::instrument(level = "trace", skip_all)] -fn show_active_toolchain(cfg: &Cfg<'_>, verbose: bool) -> Result { - match cfg.find_active_toolchain()? { +async fn show_active_toolchain(cfg: &Cfg<'_>, verbose: bool) -> Result { + match cfg.find_active_toolchain().await? { Some((toolchain_name, reason)) => { let toolchain = Toolchain::with_reason(cfg, toolchain_name.clone(), &reason)?; writeln!( @@ -1118,7 +1120,7 @@ async fn target_list( quiet: bool, ) -> Result { // downcasting required because the toolchain files can name any toolchain - let distributable = DistributableToolchain::from_partial(toolchain, cfg)?; + let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; common::list_items( distributable, |c| { @@ -1145,7 +1147,7 @@ async fn target_add( // isn't a feature yet. // list_components *and* add_component would both be inappropriate for // custom toolchains. - let distributable = DistributableToolchain::from_partial(toolchain, cfg)?; + let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; let components = distributable.components()?; if targets.contains(&"all".to_string()) { @@ -1189,7 +1191,7 @@ async fn target_remove( targets: Vec, toolchain: Option, ) -> Result { - let distributable = DistributableToolchain::from_partial(toolchain, cfg)?; + let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; for target in targets { let target = TargetTriple::new(target); @@ -1227,7 +1229,7 @@ async fn component_list( quiet: bool, ) -> Result { // downcasting required because the toolchain files can name any toolchain - let distributable = DistributableToolchain::from_partial(toolchain, cfg)?; + let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; common::list_items( distributable, |c| Some(&c.name), @@ -1243,7 +1245,7 @@ async fn component_add( toolchain: Option, target: Option, ) -> Result { - let distributable = DistributableToolchain::from_partial(toolchain, cfg)?; + let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; let target = get_target(target, &distributable); for component in &components { @@ -1269,7 +1271,7 @@ async fn component_remove( toolchain: Option, target: Option, ) -> Result { - let distributable = DistributableToolchain::from_partial(toolchain, cfg)?; + let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; let target = get_target(target, &distributable); for component in &components { @@ -1311,9 +1313,14 @@ async fn toolchain_link( Ok(utils::ExitCode(0)) } -fn toolchain_remove(cfg: &mut Cfg<'_>, opts: UninstallOpts) -> Result { +async fn toolchain_remove(cfg: &mut Cfg<'_>, opts: UninstallOpts) -> Result { let default_toolchain = cfg.get_default().ok().flatten(); - let active_toolchain = cfg.find_active_toolchain().ok().flatten().map(|(it, _)| it); + let active_toolchain = cfg + .find_active_toolchain() + .await + .ok() + .flatten() + .map(|(it, _)| it); for toolchain_name in &opts.toolchain { let toolchain_name = toolchain_name.resolve(&cfg.get_default_host_triple()?)?; @@ -1556,7 +1563,7 @@ async fn doc( mut topic: Option<&str>, doc_page: &DocPage, ) -> Result { - let toolchain = cfg.toolchain_from_partial(toolchain)?; + let toolchain = cfg.toolchain_from_partial(toolchain).await?; if let Ok(distributable) = DistributableToolchain::try_from(&toolchain) { if let [_] = distributable @@ -1625,7 +1632,7 @@ async fn man( command: &str, toolchain: Option, ) -> Result { - let toolchain = cfg.toolchain_from_partial(toolchain)?; + let toolchain = cfg.toolchain_from_partial(toolchain).await?; let path = toolchain.man_path(); utils::assert_is_directory(&path)?; diff --git a/src/config.rs b/src/config.rs index 8787d80981..d84ac28276 100644 --- a/src/config.rs +++ b/src/config.rs @@ -500,7 +500,7 @@ impl<'a> Cfg<'a> { .transpose()?) } - pub(crate) fn toolchain_from_partial( + pub(crate) async fn toolchain_from_partial( &self, toolchain: Option, ) -> anyhow::Result> { @@ -511,10 +511,10 @@ impl<'a> Cfg<'a> { ))) }) .transpose()?; - self.local_toolchain(toolchain) + self.local_toolchain(toolchain).await } - pub(crate) fn find_active_toolchain( + pub(crate) async fn find_active_toolchain( &self, ) -> Result> { Ok( @@ -703,43 +703,44 @@ impl<'a> Cfg<'a> { } #[tracing::instrument(level = "trace")] - pub(crate) fn active_rustc_version(&mut self) -> Result> { + pub(crate) async fn active_rustc_version(&mut self) -> Result> { if let Some(t) = self.process.args().find(|x| x.starts_with('+')) { trace!("Fetching rustc version from toolchain `{}`", t); self.set_toolchain_override(&ResolvableToolchainName::try_from(&t[1..])?); } - let Some((name, _)) = self.find_active_toolchain()? else { + let Some((name, _)) = self.find_active_toolchain().await? else { return Ok(None); }; Ok(Some(Toolchain::new(self, name)?.rustc_version())) } - pub(crate) fn resolve_toolchain( + pub(crate) async fn resolve_toolchain( &self, name: Option, ) -> Result> { let toolchain = name .map(|name| anyhow::Ok(name.resolve(&self.get_default_host_triple()?)?.into())) .transpose()?; - self.local_toolchain(toolchain) + self.local_toolchain(toolchain).await } - pub(crate) fn resolve_local_toolchain( + pub(crate) async fn resolve_local_toolchain( &self, name: Option, ) -> Result> { let local = name .map(|name| name.resolve(&self.get_default_host_triple()?)) .transpose()?; - self.local_toolchain(local) + self.local_toolchain(local).await } - fn local_toolchain(&self, name: Option) -> Result> { + async fn local_toolchain(&self, name: Option) -> Result> { let toolchain = match name { Some(tc) => tc, None => { - self.find_active_toolchain()? + self.find_active_toolchain() + .await? .ok_or_else(|| no_toolchain_error(self.process))? .0 } diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index 4d51a2fee8..48b3dd2b00 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -32,11 +32,13 @@ pub(crate) struct DistributableToolchain<'a> { } impl<'a> DistributableToolchain<'a> { - pub(crate) fn from_partial( + pub(crate) async fn from_partial( toolchain: Option, cfg: &'a Cfg<'a>, ) -> anyhow::Result { - Ok(Self::try_from(&cfg.toolchain_from_partial(toolchain)?)?) + Ok(Self::try_from( + &cfg.toolchain_from_partial(toolchain).await?, + )?) } pub(crate) fn new(cfg: &'a Cfg<'a>, desc: ToolchainDesc) -> Result { From 4045b1350461f567cab2a1606b9bf3ec265289b9 Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 4 Mar 2025 13:45:28 +0800 Subject: [PATCH 2/2] fix!(config): re-enable active toolchain installation in `Cfg::find_active_toolchain()` with optional opt-out --- src/cli/common.rs | 2 +- src/cli/rustup_mode.rs | 8 ++--- src/config.rs | 68 +++++++++++++++++++++++++++++++++------ tests/suite/cli_rustup.rs | 20 +++++------- tests/suite/cli_v1.rs | 10 ++---- tests/suite/cli_v2.rs | 15 ++++----- 6 files changed, 80 insertions(+), 43 deletions(-) diff --git a/src/cli/common.rs b/src/cli/common.rs index 931b0c7dbf..8eb531251f 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -418,7 +418,7 @@ pub(crate) async fn list_toolchains( let default_toolchain_name = cfg.get_default()?; let active_toolchain_name: Option = if let Ok(Some((LocalToolchainName::Named(toolchain), _reason))) = - cfg.find_active_toolchain().await + cfg.find_active_toolchain(None).await { Some(toolchain) } else { diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 1ec9632596..df52324c19 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -753,7 +753,7 @@ async fn default_( } }; - if let Some((toolchain, reason)) = cfg.find_active_toolchain().await? { + if let Some((toolchain, reason)) = cfg.find_active_toolchain(None).await? { if !matches!(reason, ActiveReason::Default) { info!("note that the toolchain '{toolchain}' is currently in use ({reason})"); } @@ -964,7 +964,7 @@ async fn show(cfg: &Cfg<'_>, verbose: bool) -> Result { let installed_toolchains = cfg.list_toolchains()?; let active_toolchain_and_reason: Option<(ToolchainName, ActiveReason)> = if let Ok(Some((LocalToolchainName::Named(toolchain_name), reason))) = - cfg.find_active_toolchain().await + cfg.find_active_toolchain(None).await { Some((toolchain_name, reason)) } else { @@ -1084,7 +1084,7 @@ async fn show(cfg: &Cfg<'_>, verbose: bool) -> Result { #[tracing::instrument(level = "trace", skip_all)] async fn show_active_toolchain(cfg: &Cfg<'_>, verbose: bool) -> Result { - match cfg.find_active_toolchain().await? { + match cfg.find_active_toolchain(None).await? { Some((toolchain_name, reason)) => { let toolchain = Toolchain::with_reason(cfg, toolchain_name.clone(), &reason)?; writeln!( @@ -1316,7 +1316,7 @@ async fn toolchain_link( async fn toolchain_remove(cfg: &mut Cfg<'_>, opts: UninstallOpts) -> Result { let default_toolchain = cfg.get_default().ok().flatten(); let active_toolchain = cfg - .find_active_toolchain() + .find_active_toolchain(Some(false)) .await .ok() .flatten() diff --git a/src/config.rs b/src/config.rs index d84ac28276..ba16d40272 100644 --- a/src/config.rs +++ b/src/config.rs @@ -516,15 +516,63 @@ impl<'a> Cfg<'a> { pub(crate) async fn find_active_toolchain( &self, + force_install_active: Option, ) -> Result> { - Ok( - if let Some((override_config, reason)) = self.find_override_config()? { - Some((override_config.into_local_toolchain_name(), reason)) - } else { - self.get_default()? - .map(|x| (x.into(), ActiveReason::Default)) - }, - ) + let (components, targets, profile, toolchain, reason) = match self.find_override_config()? { + Some(( + OverrideCfg::Official { + components, + targets, + profile, + toolchain, + }, + reason, + )) => (components, targets, profile, toolchain, reason), + Some((override_config, reason)) => { + return Ok(Some((override_config.into_local_toolchain_name(), reason))); + } + None => { + return Ok(self + .get_default()? + .map(|x| (x.into(), ActiveReason::Default))); + } + }; + + let should_install_active = force_install_active.unwrap_or_else(|| { + self.process + .var("RUSTUP_AUTO_INSTALL") + .map_or(true, |it| it != "0") + }); + + if !should_install_active { + return Ok(Some(((&toolchain).into(), reason))); + } + + let components = components.iter().map(AsRef::as_ref).collect::>(); + let targets = targets.iter().map(AsRef::as_ref).collect::>(); + match DistributableToolchain::new(self, toolchain.clone()) { + Err(RustupError::ToolchainNotInstalled { .. }) => { + DistributableToolchain::install( + self, + &toolchain, + &components, + &targets, + profile.unwrap_or_default(), + false, + ) + .await?; + } + Ok(mut distributable) => { + if !distributable.components_exist(&components, &targets)? { + distributable + .update(&components, &targets, profile.unwrap_or_default()) + .await?; + } + } + Err(e) => return Err(e.into()), + }; + + Ok(Some(((&toolchain).into(), reason))) } fn find_override_config(&self) -> Result> { @@ -709,7 +757,7 @@ impl<'a> Cfg<'a> { self.set_toolchain_override(&ResolvableToolchainName::try_from(&t[1..])?); } - let Some((name, _)) = self.find_active_toolchain().await? else { + let Some((name, _)) = self.find_active_toolchain(None).await? else { return Ok(None); }; Ok(Some(Toolchain::new(self, name)?.rustc_version())) @@ -739,7 +787,7 @@ impl<'a> Cfg<'a> { let toolchain = match name { Some(tc) => tc, None => { - self.find_active_toolchain() + self.find_active_toolchain(None) .await? .ok_or_else(|| no_toolchain_error(self.process))? .0 diff --git a/tests/suite/cli_rustup.rs b/tests/suite/cli_rustup.rs index b17d7e9e12..bec50d417c 100644 --- a/tests/suite/cli_rustup.rs +++ b/tests/suite/cli_rustup.rs @@ -1052,12 +1052,12 @@ async fn show_toolchain_override_not_installed() { .expect_ok(&["rustup", "toolchain", "remove", "nightly"]) .await; let out = cx.config.run("rustup", ["show"], &[]).await; - assert!(!out.ok); + assert!(out.ok); assert!( - out.stderr + !out.stderr .contains("is not installed: the directory override for") ); - assert!(!out.stderr.contains("info: installing component 'rustc'")); + assert!(out.stderr.contains("info: installing component 'rustc'")); } #[tokio::test] @@ -1137,7 +1137,7 @@ async fn show_toolchain_env_not_installed() { .run("rustup", ["show"], &[("RUSTUP_TOOLCHAIN", "nightly")]) .await; - assert!(!out.ok); + assert!(out.ok); let expected_out = for_host_and_home!( cx.config, @@ -1149,17 +1149,13 @@ installed toolchains active toolchain ---------------- +name: nightly-{0} +active because: overridden by environment variable RUSTUP_TOOLCHAIN +installed targets: + {0} " ); assert!(&out.stdout == expected_out); - assert!( - out.stderr - == format!( - "error: override toolchain 'nightly-{}' is not installed: \ - the RUSTUP_TOOLCHAIN environment variable specifies an uninstalled toolchain\n", - this_host_triple() - ) - ); } #[tokio::test] diff --git a/tests/suite/cli_v1.rs b/tests/suite/cli_v1.rs index 1cae52965e..5ec9e8b069 100644 --- a/tests/suite/cli_v1.rs +++ b/tests/suite/cli_v1.rs @@ -182,14 +182,10 @@ async fn remove_override_toolchain_err_handling() { .expect_ok(&["rustup", "toolchain", "remove", "beta"]) .await; cx.config - .expect_err_ex( + .expect_ok_contains( &["rustc", "--version"], - "", - for_host!( - r"error: toolchain 'beta-{0}' is not installed -help: run `rustup toolchain install beta-{0}` to install it -" - ), + "1.2.0 (hash-beta-1.2.0)", + "info: downloading component 'rust'", ) .await; } diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs index 624f56a89e..f1bbd84ecb 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -327,14 +327,10 @@ async fn remove_override_toolchain_err_handling() { .expect_ok(&["rustup", "toolchain", "remove", "beta"]) .await; cx.config - .expect_err_ex( + .expect_ok_contains( &["rustc", "--version"], - "", - for_host!( - r"error: toolchain 'beta-{0}' is not installed -help: run `rustup toolchain install beta-{0}` to install it -" - ), + "1.2.0 (hash-beta-1.2.0)", + "info: downloading component 'rustc'", ) .await; } @@ -346,9 +342,10 @@ async fn file_override_toolchain_err_handling() { let toolchain_file = cwd.join("rust-toolchain"); rustup::utils::raw::write_file(&toolchain_file, "beta").unwrap(); cx.config - .expect_err( + .expect_ok_contains( &["rustc", "--version"], - for_host!("toolchain 'beta-{0}' is not installed"), + "1.2.0 (hash-beta-1.2.0)", + "info: downloading component 'rustc'", ) .await; }