Further refactoring of self update logic

src/cli/common.rs

@@ -3,8 +3,6 @@
 use std::cell::RefCell;
 use std::fmt::Display;
 use std::fs;
-#[cfg(not(windows))]
-use std::io::ErrorKind;
 use std::io::{BufRead, Write};
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, LazyLock, Mutex};
@@ -15,7 +13,6 @@ use git_testament::{git_testament, render_testament};
 use tracing::{debug, error, info, trace, warn};
 use tracing_subscriber::{EnvFilter, Registry, reload::Handle};
 
-use super::self_update;
 use crate::{
     cli::download_tracker::DownloadTracker,
     config::Cfg,
@@ -303,67 +300,6 @@ pub(crate) async fn update_all_channels(
     Ok(exit_code)
 }
 
-#[derive(Clone, Copy, Debug)]
-pub(crate) enum SelfUpdatePermission {
-    HardFail,
-    #[cfg(not(windows))]
-    Skip,
-    Permit,
-}
-
-#[cfg(windows)]
-pub(crate) fn self_update_permitted(_explicit: bool) -> Result<SelfUpdatePermission> {
-    Ok(SelfUpdatePermission::Permit)
-}
-
-#[cfg(not(windows))]
-pub(crate) fn self_update_permitted(explicit: bool) -> Result<SelfUpdatePermission> {
-    // Detect if rustup is not meant to self-update
-    let current_exe = env::current_exe()?;
-    let current_exe_dir = current_exe.parent().expect("Rustup isn't in a directory‽");
-    if let Err(e) = tempfile::Builder::new()
-        .prefix("updtest")
-        .tempdir_in(current_exe_dir)
-    {
-        match e.kind() {
-            ErrorKind::PermissionDenied => {
-                trace!("Skipping self-update because we cannot write to the rustup dir");
-                if explicit {
-                    return Ok(SelfUpdatePermission::HardFail);
-                } else {
-                    return Ok(SelfUpdatePermission::Skip);
-                }
-            }
-            _ => return Err(e.into()),
-        }
-    }
-    Ok(SelfUpdatePermission::Permit)
-}
-
-/// Performs all of a self-update: check policy, download, apply and exit.
-pub(crate) async fn self_update(process: &Process) -> Result<utils::ExitCode> {
-    match self_update_permitted(false)? {
-        SelfUpdatePermission::HardFail => {
-            error!("Unable to self-update.  STOP");
-            return Ok(utils::ExitCode(1));
-        }
-        #[cfg(not(windows))]
-        SelfUpdatePermission::Skip => return Ok(utils::ExitCode(0)),
-        SelfUpdatePermission::Permit => {}
-    }
-
-    let setup_path = self_update::prepare_update(process).await?;
-
-    if let Some(setup_path) = &setup_path {
-        return self_update::run_update(setup_path);
-    } else {
-        // Try again in case we emitted "tool `{}` is already installed" last time.
-        self_update::install_proxies(process)?;
-    }
-
-    Ok(utils::ExitCode(0))
-}
-
 /// Print a list of items (targets or components) to stdout.
 ///
 /// `items` represents the list of items, with the name and a boolean

src/cli/rustup_mode.rs

@@ -894,16 +894,7 @@ async fn check_updates(cfg: &Cfg<'_>, opts: CheckOpts) -> Result<utils::ExitCode
         }
     }
 
-    let self_update_mode = SelfUpdateMode::from_cfg(cfg)?;
-    // 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.
-    let self_update = !cfg!(feature = "no-self-update")
-        && self_update_mode == SelfUpdateMode::Enable
-        && !opts.no_self_update;
-
-    if self_update && check_rustup_update(cfg.process).await? {
+    if check_rustup_update(opts.no_self_update, cfg).await? {
         update_available = true;
     }
 
@@ -919,14 +910,6 @@ async fn update(
     let mut exit_code = utils::ExitCode(0);
 
     common::warn_if_host_is_emulated(cfg.process);
-    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**
-    // and has **no** no-self-update parameter.
-    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;
     if let Some(p) = opts.profile {
         cfg.set_profile_override(p);
@@ -987,31 +970,19 @@ 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();
     }
 
-    if !cfg!(feature = "no-self-update") && self_update_mode == SelfUpdateMode::CheckOnly {
-        check_rustup_update(cfg.process).await?;
-    }
-
-    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")
+    if !ensure_active_toolchain {
+        exit_code &= self_update::self_update(opts.no_self_update, cfg).await?;
     }
 
     Ok(exit_code)

src/cli/self_update.rs

@@ -34,7 +34,7 @@ use std::borrow::Cow;
 use std::env::{self, consts::EXE_SUFFIX};
 use std::fmt;
 use std::fs;
-use std::io::Write;
+use std::io::{self, Write};
 use std::path::{Component, MAIN_SEPARATOR, Path, PathBuf};
 use std::process::Command;
 use std::str::FromStr;
@@ -1049,6 +1049,83 @@ pub(crate) fn uninstall(no_prompt: bool, process: &Process) -> Result<utils::Exi
     Ok(utils::ExitCode(0))
 }
 
+#[derive(Clone, Copy, Debug)]
+pub(crate) enum SelfUpdatePermission {
+    HardFail,
+    #[cfg(not(windows))]
+    Skip,
+    Permit,
+}
+
+#[cfg(windows)]
+pub(crate) fn self_update_permitted(_explicit: bool) -> Result<SelfUpdatePermission> {
+    Ok(SelfUpdatePermission::Permit)
+}
+
+#[cfg(not(windows))]
+pub(crate) fn self_update_permitted(explicit: bool) -> Result<SelfUpdatePermission> {
+    // Detect if rustup is not meant to self-update
+    let current_exe = env::current_exe()?;
+    let current_exe_dir = current_exe.parent().expect("Rustup isn't in a directory‽");
+    if let Err(e) = tempfile::Builder::new()
+        .prefix("updtest")
+        .tempdir_in(current_exe_dir)
+    {
+        match e.kind() {
+            io::ErrorKind::PermissionDenied => {
+                trace!("Skipping self-update because we cannot write to the rustup dir");
+                if explicit {
+                    return Ok(SelfUpdatePermission::HardFail);
+                } else {
+                    return Ok(SelfUpdatePermission::Skip);
+                }
+            }
+            _ => return Err(e.into()),
+        }
+    }
+    Ok(SelfUpdatePermission::Permit)
+}
+
+/// Performs all of a self-update: check policy, download, apply and exit.
+pub(crate) async fn self_update(disabled: bool, cfg: &Cfg<'_>) -> Result<utils::ExitCode> {
+    // 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**
+    // and has **no** no-self-update parameter.
+    let self_update_mode = SelfUpdateMode::from_cfg(cfg)?;
+    if cfg!(feature = "no-self-update") && !disabled {
+        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")
+    } else if self_update_mode == SelfUpdateMode::CheckOnly {
+        check_rustup_update(disabled, cfg).await?;
+        return Ok(utils::ExitCode(0));
+    } else if disabled {
+        info!("self-update is disabled by command line argument");
+        return Ok(utils::ExitCode(0));
+    }
+
+    match self_update_permitted(false)? {
+        SelfUpdatePermission::HardFail => {
+            error!("Unable to self-update.  STOP");
+            return Ok(utils::ExitCode(1));
+        }
+        #[cfg(not(windows))]
+        SelfUpdatePermission::Skip => return Ok(utils::ExitCode(0)),
+        SelfUpdatePermission::Permit => {}
+    }
+
+    let setup_path = prepare_update(&cfg.process).await?;
+
+    if let Some(setup_path) = &setup_path {
+        return run_update(setup_path);
+    } else {
+        // Try again in case we emitted "tool `{}` is already installed" last time.
+        install_proxies(&cfg.process)?;
+    }
+
+    Ok(utils::ExitCode(0))
+}
+
 /// Self update downloads rustup-init to `CARGO_HOME`/bin/rustup-init
 /// and runs it.
 ///
@@ -1067,11 +1144,11 @@ pub(crate) fn uninstall(no_prompt: bool, process: &Process) -> Result<utils::Exi
 pub(crate) async fn update(cfg: &Cfg<'_>) -> Result<utils::ExitCode> {
     common::warn_if_host_is_emulated(cfg.process);
 
-    use common::SelfUpdatePermission::*;
+    use SelfUpdatePermission::*;
     let update_permitted = if cfg!(feature = "no-self-update") {
         HardFail
     } else {
-        common::self_update_permitted(true)?
+        self_update_permitted(true)?
     };
     match update_permitted {
         HardFail => {
@@ -1261,13 +1338,22 @@ impl fmt::Display for SchemaVersion {
 }
 
 /// Returns whether an update was available
-pub(crate) async fn check_rustup_update(process: &Process) -> anyhow::Result<bool> {
-    let mut t = process.stdout().terminal(process);
+pub(crate) async fn check_rustup_update(disabled: bool, cfg: &Cfg<'_>) -> anyhow::Result<bool> {
+    // 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.
+    let self_update_mode = SelfUpdateMode::from_cfg(cfg)?;
+    if cfg!(feature = "no-self-update") || self_update_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 - ")?;