From 586659ef887e7afa1ba5b6dc858f8d7407dea500 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 23 May 2025 13:13:02 +0200 Subject: [PATCH 1/5] Replace trivial enum with bool --- src/cli/rustup_mode.rs | 9 ++------- src/cli/self_update.rs | 21 ++++++--------------- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index e72f5203b5..2156a6d6c8 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -25,7 +25,7 @@ use crate::{ common::{self, PackageUpdate, update_console_filter}, errors::CLIError, help::*, - self_update::{self, RustupUpdateAvailable, SelfUpdateMode, check_rustup_update}, + self_update::{self, SelfUpdateMode, check_rustup_update}, topical_doc, }, command, @@ -898,12 +898,7 @@ async fn check_updates(cfg: &Cfg<'_>, opts: CheckOpts) -> Result Result { - let mut update_available = RustupUpdateAvailable::False; - +/// Returns whether an update was available +pub(crate) async fn check_rustup_update(process: &Process) -> anyhow::Result { let mut t = process.stdout().terminal(process); // Get current rustup version let current_version = env!("CARGO_PKG_VERSION"); @@ -1269,21 +1262,19 @@ pub(crate) async fn check_rustup_update(process: &Process) -> Result {available_version}")?; + true } else { let _ = t.fg(terminalsource::Color::Green); write!(t.lock(), "Up to date")?; let _ = t.reset(); writeln!(t.lock(), " : {current_version}")?; - } - - Ok(update_available) + false + }) } #[tracing::instrument(level = "trace")] From 12175a96a56cf65bb09f217afa1d8b578f47132e Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 23 May 2025 13:23:28 +0200 Subject: [PATCH 2/5] Move Cfg::get_self_update_mode() to SelfUpdateMode::from_cfg() --- src/cli/rustup_mode.rs | 5 +++-- src/cli/self_update.rs | 15 +++++++++++++++ src/config.rs | 15 --------------- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 2156a6d6c8..ca1080668c 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -889,7 +889,8 @@ async fn check_updates(cfg: &Cfg<'_>, opts: CheckOpts) -> Result self_update_mode > no-self-update args. // Check for update only if rustup does **not** have the no-self-update feature, // and auto-self-update is configured to **enable** @@ -914,7 +915,7 @@ async fn update( let mut exit_code = utils::ExitCode(0); common::warn_if_host_is_emulated(cfg.process); - let self_update_mode = cfg.get_self_update_mode()?; + let self_update_mode = SelfUpdateMode::from_cfg(cfg)?; // Priority: no-self-update feature > self_update_mode > no-self-update args. // Update only if rustup does **not** have the no-self-update feature, // and auto-self-update is configured to **enable** diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 520379da7c..dce5ba54d4 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -261,6 +261,21 @@ pub enum SelfUpdateMode { } impl SelfUpdateMode { + pub(crate) fn from_cfg(cfg: &Cfg<'_>) -> anyhow::Result { + if cfg.process.var("CI").is_ok() && cfg.process.var("RUSTUP_CI").is_err() { + // If we're in CI (but not rustup's own CI, which wants to test this stuff!), + // disable automatic self updates. + return Ok(SelfUpdateMode::Disable); + } + + cfg.settings_file.with(|s| { + Ok(match s.auto_self_update { + Some(mode) => mode, + None => SelfUpdateMode::Enable, + }) + }) + } + pub(crate) fn as_str(&self) -> &'static str { match self { Self::Enable => "enable", diff --git a/src/config.rs b/src/config.rs index 8d14448bc1..ad1c0978c9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -409,21 +409,6 @@ impl<'a> Cfg<'a> { .with(|s| Ok(s.profile.unwrap_or_default())) } - pub(crate) fn get_self_update_mode(&self) -> Result { - if self.process.var("CI").is_ok() && self.process.var("RUSTUP_CI").is_err() { - // If we're in CI (but not rustup's own CI, which wants to test this stuff!), - // disable automatic self updates. - return Ok(SelfUpdateMode::Disable); - } - - self.settings_file.with(|s| { - Ok(match s.auto_self_update { - Some(mode) => mode, - None => SelfUpdateMode::Enable, - }) - }) - } - pub(crate) fn ensure_toolchains_dir(&self) -> Result<(), anyhow::Error> { utils::ensure_dir_exists("toolchains", &self.toolchains_dir, &|n| { (self.notify_handler)(n) From 98286b2dd9f42ca6c34b42f5d2e275ed2225c92a Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 8 Jul 2025 15:41:43 +0200 Subject: [PATCH 3/5] Remove Cargo feature indirection --- src/cli/rustup_mode.rs | 10 +++++----- src/cli/self_update.rs | 9 ++------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index ca1080668c..3e966ee647 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -895,7 +895,7 @@ async fn check_updates(cfg: &Cfg<'_>, opts: CheckOpts) -> Result, auto_self_update_mode: SelfUpdateMode, ) -> Result { - if self_update::NEVER_SELF_UPDATE { + if cfg!(feature = "no-self-update") { let mut args = cfg.process.args_os(); let arg0 = args.next().map(PathBuf::from); let arg0 = arg0 diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index dce5ba54d4..a9dc44ce20 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -246,11 +246,6 @@ impl InstallOpts<'_> { } } -#[cfg(feature = "no-self-update")] -pub(crate) const NEVER_SELF_UPDATE: bool = true; -#[cfg(not(feature = "no-self-update"))] -pub(crate) const NEVER_SELF_UPDATE: bool = false; - #[derive(Clone, Copy, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] #[serde(rename_all = "kebab-case")] pub enum SelfUpdateMode { @@ -955,7 +950,7 @@ async fn maybe_install_rust( } pub(crate) fn uninstall(no_prompt: bool, process: &Process) -> Result { - if NEVER_SELF_UPDATE { + if cfg!(feature = "no-self-update") { error!("self-uninstall is disabled for this build of rustup"); error!("you should probably use your system package manager to uninstall rustup"); return Ok(utils::ExitCode(1)); @@ -1073,7 +1068,7 @@ pub(crate) async fn update(cfg: &Cfg<'_>) -> Result { common::warn_if_host_is_emulated(cfg.process); use common::SelfUpdatePermission::*; - let update_permitted = if NEVER_SELF_UPDATE { + let update_permitted = if cfg!(feature = "no-self-update") { HardFail } else { common::self_update_permitted(true)? From d14a7dc9b9ad8f911d22c8a6bf0d5a3a9f98861d Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 2 Jun 2025 10:24:33 +0200 Subject: [PATCH 4/5] Show channel updates even if self update is not allowed I don't think this complexity is worth it? Looking at the changes from when this indirection was introduced does not make it clearer. --- src/cli/common.rs | 33 ++++++++++++--------------------- src/cli/rustup_mode.rs | 2 +- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/src/cli/common.rs b/src/cli/common.rs index a4db27ee75..93406e60ef 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -291,24 +291,20 @@ pub(crate) async fn update_all_channels( info!("no updatable toolchains installed"); } - let show_channel_updates = || { - if !toolchains.is_empty() { - writeln!(cfg.process.stdout().lock())?; - - let t = toolchains - .into_iter() - .map(|(p, s)| (PackageUpdate::Toolchain(p), s)) - .collect(); - show_channel_updates(cfg, t)?; - } - Ok(()) - }; + if !toolchains.is_empty() { + writeln!(cfg.process.stdout().lock())?; + + let t = toolchains + .into_iter() + .map(|(p, s)| (PackageUpdate::Toolchain(p), s)) + .collect(); + show_channel_updates(cfg, t)?; + } if do_self_update { - exit_code &= self_update(show_channel_updates, cfg.process).await?; - } else { - show_channel_updates()?; + exit_code &= self_update(cfg.process).await?; } + Ok(exit_code) } @@ -350,10 +346,7 @@ pub(crate) fn self_update_permitted(explicit: bool) -> Result(before_restart: F, process: &Process) -> Result -where - F: FnOnce() -> Result<()>, -{ +pub(crate) async fn self_update(process: &Process) -> Result { match self_update_permitted(false)? { SelfUpdatePermission::HardFail => { error!("Unable to self-update. STOP"); @@ -366,8 +359,6 @@ where let setup_path = self_update::prepare_update(process).await?; - before_restart()?; - if let Some(setup_path) = &setup_path { return self_update::run_update(setup_path); } else { diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 3e966ee647..63e07caee0 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -984,7 +984,7 @@ async fn update( } } if self_update { - exit_code &= common::self_update(|| Ok(()), cfg.process).await?; + exit_code &= common::self_update(cfg.process).await?; } } else if ensure_active_toolchain { let (toolchain, reason) = cfg.ensure_active_toolchain(force_non_host, true).await?; From bf04ae1e3df371ef9e4053e0d260875931734a16 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 2 Jun 2025 10:28:14 +0200 Subject: [PATCH 5/5] Extract self_update() from update_all_channels() --- src/cli/common.rs | 7 +------ src/cli/rustup_mode.rs | 6 +++++- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/cli/common.rs b/src/cli/common.rs index 93406e60ef..031d149c95 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -280,12 +280,11 @@ fn show_channel_updates( pub(crate) async fn update_all_channels( cfg: &Cfg<'_>, - do_self_update: bool, force_update: bool, ) -> Result { let toolchains = cfg.update_all_channels(force_update).await?; let has_update_error = toolchains.iter().any(|(_, r)| r.is_err()); - let mut exit_code = utils::ExitCode(if has_update_error { 1 } else { 0 }); + let exit_code = utils::ExitCode(if has_update_error { 1 } else { 0 }); if toolchains.is_empty() { info!("no updatable toolchains installed"); @@ -301,10 +300,6 @@ pub(crate) async fn update_all_channels( show_channel_updates(cfg, t)?; } - if do_self_update { - exit_code &= self_update(cfg.process).await?; - } - Ok(exit_code) } diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 63e07caee0..a46cacff2c 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -991,7 +991,11 @@ async fn update( info!("the active toolchain `{toolchain}` has been installed"); info!("it's active because: {reason}"); } else { - exit_code &= common::update_all_channels(cfg, self_update, opts.force).await?; + exit_code &= common::update_all_channels(cfg, opts.force).await?; + if self_update { + exit_code &= common::self_update(cfg.process).await?; + } + info!("cleaning up downloads & tmp directories"); utils::delete_dir_contents_following_links(&cfg.download_dir); cfg.tmp_cx.clean();