From b31b4f536197f0d9b67a9d7022e283c2dc878569 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 23 May 2025 13:13:02 +0200 Subject: [PATCH 1/8] 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 7476d32fa9..d751cb5469 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -19,7 +19,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, @@ -838,12 +838,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 a3ab8865c81cfd1453b6a8eefa729c21e19db776 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 23 May 2025 13:23:28 +0200 Subject: [PATCH 2/8] Move Cfg::get_self_update_mode() to SelfUpdateMode::from_cfg() --- src/cli/rustup_mode.rs | 4 ++-- src/cli/self_update.rs | 15 +++++++++++++++ src/config.rs | 15 --------------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index d751cb5469..ba565c7a28 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -829,7 +829,7 @@ 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** @@ -854,7 +854,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 88cbed4a99..87ba363acf 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 635c8eade8..5057fcffab 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 783fe7f5a2d1207bc39c8f166d1b50ff938b6152 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 23 May 2025 15:33:54 +0200 Subject: [PATCH 3/8] Centralize self update check logic --- src/cli/rustup_mode.rs | 20 ++------------------ src/cli/self_update.rs | 22 +++++++++++++++++++--- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index ba565c7a28..6e0ca6691f 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -830,15 +830,7 @@ 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** - // and has **no** no-self-update parameter. - let self_update = !self_update::NEVER_SELF_UPDATE - && self_update_mode == SelfUpdateMode::Enable - && !opts.no_self_update; - - if self_update && check_rustup_update(cfg.process).await? { + if check_rustup_update(self_update_mode, opts.no_self_update, cfg).await? { update_available = true; } @@ -936,15 +928,7 @@ async fn update( cfg.tmp_cx.clean(); } - if !self_update::NEVER_SELF_UPDATE && self_update_mode == SelfUpdateMode::CheckOnly { - check_rustup_update(cfg.process).await?; - } - - if self_update::NEVER_SELF_UPDATE { - info!("self-update is disabled for this build of rustup"); - info!("any updates to rustup will need to be fetched with your system package manager") - } - + check_rustup_update(self_update_mode, false, cfg).await?; Ok(exit_code) } diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 87ba363acf..429f11bc73 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -1266,13 +1266,29 @@ impl fmt::Display for SchemaVersion { } /// Returns whether an update was available -pub(crate) async fn check_rustup_update(process: &Process) -> anyhow::Result { - let mut t = process.stdout().terminal(process); +pub(crate) async fn check_rustup_update( + mode: SelfUpdateMode, + disabled: bool, + cfg: &Cfg<'_>, +) -> anyhow::Result { + // Priority: no-self-update feature > 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** + // and has **no** no-self-update parameter. + if NEVER_SELF_UPDATE { + info!("self-update is disabled for this build of rustup"); + info!("any updates to rustup will need to be fetched with your system package manager"); + return Ok(false); + } else if mode == SelfUpdateMode::Disable || disabled { + return Ok(false); + } + + let mut t = cfg.process.stdout().terminal(cfg.process); // Get current rustup version let current_version = env!("CARGO_PKG_VERSION"); // Get available rustup version - let available_version = get_available_rustup_version(process).await?; + let available_version = get_available_rustup_version(cfg.process).await?; let _ = t.attr(terminalsource::Attr::Bold); write!(t.lock(), "rustup - ")?; From 490eafcc2c091e73f304b7ed9e9c33d8a7fddbc8 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 23 May 2025 15:43:34 +0200 Subject: [PATCH 4/8] Remove Cargo feature indirection --- src/cli/rustup_mode.rs | 4 ++-- src/cli/self_update.rs | 11 +++-------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 6e0ca6691f..8ad66dedec 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -851,7 +851,7 @@ async fn update( // Update only if rustup does **not** have the no-self-update feature, // and auto-self-update is configured to **enable** // and has **no** no-self-update parameter. - let self_update = !self_update::NEVER_SELF_UPDATE + let self_update = !cfg!(feature = "no-self-update") && self_update_mode == SelfUpdateMode::Enable && !opts.no_self_update; let force_non_host = opts.force_non_host; @@ -1706,7 +1706,7 @@ fn set_auto_self_update( cfg: &mut Cfg<'_>, 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 429f11bc73..9fb54bee8a 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)? @@ -1275,7 +1270,7 @@ pub(crate) async fn check_rustup_update( // Check for update only if rustup does **not** have the no-self-update feature, // and auto-self-update is configured to **enable** // and has **no** no-self-update parameter. - if NEVER_SELF_UPDATE { + if cfg!(feature = "no-self-update") { info!("self-update is disabled for this build of rustup"); info!("any updates to rustup will need to be fetched with your system package manager"); return Ok(false); From 55524bf17b79a741c69fc9b5d8322abe8cf3f408 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 2 Jun 2025 10:24:33 +0200 Subject: [PATCH 5/8] 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 cc554a38df..ea2a9e49d0 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(ref 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 8ad66dedec..233dad40b4 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -915,7 +915,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 dd7a5553238873cf062a3e67ff5fb6d684b6a1c1 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 2 Jun 2025 10:28:14 +0200 Subject: [PATCH 6/8] 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 ea2a9e49d0..3c1c581d2f 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 233dad40b4..4620bd46bd 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -922,7 +922,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(); From d7dc3c454cc16d24d586aa45ee9ad672f34aecf2 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 2 Jun 2025 10:33:26 +0200 Subject: [PATCH 7/8] Clarify self update logic in update() --- src/cli/rustup_mode.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 4620bd46bd..406e061491 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -914,25 +914,23 @@ async fn update( cfg.set_default(Some(&desc.into()))?; } } - if self_update { - 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?; info!("the active toolchain `{toolchain}` has been installed"); info!("it's active because: {reason}"); } else { 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(); } - check_rustup_update(self_update_mode, false, cfg).await?; + if self_update && !ensure_active_toolchain { + exit_code &= common::self_update(cfg.process).await?; + } else { + check_rustup_update(self_update_mode, false, cfg).await?; + } + Ok(exit_code) } From ab28bcba460816ec8f9956b060c54dcb93cbe520 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 2 Jun 2025 10:36:42 +0200 Subject: [PATCH 8/8] Propagate no_self_update option --- src/cli/rustup_mode.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 406e061491..c7d01df8a5 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -928,7 +928,7 @@ async fn update( if self_update && !ensure_active_toolchain { exit_code &= common::self_update(cfg.process).await?; } else { - check_rustup_update(self_update_mode, false, cfg).await?; + check_rustup_update(self_update_mode, opts.no_self_update, cfg).await?; } Ok(exit_code)