fix(cli/rustup-mode): add missing self-update in rustup toolchain install

src/cli/rustup_mode.rs

@@ -946,13 +946,7 @@ async fn update(
 
     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 should_self_update = !opts.no_self_update;
     let force_non_host = opts.force_non_host;
     cfg.profile_override = opts.profile;
 
@@ -1014,33 +1008,21 @@ async fn update(
                 cfg.set_default(Some(&desc.into()))?;
             }
         }
-        if self_update {
-            exit_code &= self_update::self_update(&dl_cfg).await?;
-        }
+        exit_code &= self_update_mode.update(should_self_update, &dl_cfg).await?;
     } else if ensure_active_toolchain {
         let (toolchain, source) = cfg.ensure_active_toolchain(force_non_host, true).await?;
         info!("the active toolchain `{toolchain}` has been installed");
         info!("it's active because: {}", source.to_reason());
+        exit_code &= self_update_mode.update(should_self_update, &dl_cfg).await?;
     } else {
         exit_code &= common::update_all_channels(cfg, opts.force).await?;
-        if self_update {
-            exit_code &= self_update::self_update(&dl_cfg).await?;
-        }
+        exit_code &= self_update_mode.update(should_self_update, &dl_cfg).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(&dl_cfg).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")
-    }
-
     Ok(exit_code)
 }
 

src/cli/self_update.rs

@@ -50,7 +50,6 @@ use same_file::Handle;
 use serde::{Deserialize, Serialize};
 use tracing::{error, info, trace, warn};
 
-use crate::dist::download::DownloadCfg;
 use crate::{
     DUP_TOOLS, TOOLS,
     cli::{
@@ -59,7 +58,7 @@ use crate::{
         markdown::md,
     },
     config::Cfg,
-    dist::{PartialToolchainDesc, Profile, TargetTriple, ToolchainDesc},
+    dist::{PartialToolchainDesc, Profile, TargetTriple, ToolchainDesc, download::DownloadCfg},
     download::download_file,
     errors::RustupError,
     install::UpdateStatus,
@@ -68,7 +67,7 @@ use crate::{
         DistributableToolchain, MaybeOfficialToolchainName, ResolvableToolchainName, Toolchain,
         ToolchainName,
     },
-    utils,
+    utils::{self, ExitCode},
 };
 
 #[cfg(unix)]
@@ -281,6 +280,56 @@ impl SelfUpdateMode {
             Self::CheckOnly => "check-only",
         }
     }
+
+    /// Optionally performs a self-update: check policy, download, apply and exit.
+    ///
+    /// Whether the self-update is executed is based on both compile-time and runtime
+    /// configurations, where the priority is as follows:
+    /// no-self-update feature > self update mode > CLI flag
+    ///
+    /// i.e. update only if rustup does **not** have the no-self-update feature,
+    /// and self update mode is configured to **enable**
+    /// and has **no** `--no-self-update` CLI flag.
+    pub(crate) async fn update(
+        &self,
+        should_self_update: bool,
+        dl_cfg: &DownloadCfg<'_>,
+    ) -> Result<ExitCode> {
+        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(ExitCode(0));
+        }
+        match self {
+            Self::Enable if should_self_update => (),
+            Self::CheckOnly => {
+                check_rustup_update(dl_cfg).await?;
+                return Ok(ExitCode(0));
+            }
+            _ => return Ok(ExitCode(0)),
+        }
+
+        match self_update_permitted(false)? {
+            SelfUpdatePermission::HardFail => {
+                error!("Unable to self-update.  STOP");
+                return Ok(ExitCode(1));
+            }
+            #[cfg(not(windows))]
+            SelfUpdatePermission::Skip => return Ok(ExitCode(0)),
+            SelfUpdatePermission::Permit => {}
+        }
+
+        let setup_path = prepare_update(dl_cfg).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(dl_cfg.process)?;
+        }
+
+        Ok(ExitCode(0))
+    }
 }
 
 impl ValueEnum for SelfUpdateMode {
@@ -523,9 +572,9 @@ pub(crate) async fn install(
     no_prompt: bool,
     mut opts: InstallOpts<'_>,
     cfg: &mut Cfg<'_>,
-) -> Result<utils::ExitCode> {
+) -> Result<ExitCode> {
     #[cfg_attr(not(unix), allow(unused_mut))]
-    let mut exit_code = utils::ExitCode(0);
+    let mut exit_code = ExitCode(0);
 
     opts.validate(cfg.process).map_err(|e| {
         anyhow!(
@@ -565,7 +614,7 @@ pub(crate) async fn install(
             match common::confirm_advanced(customized_install, cfg.process)? {
                 Confirm::No => {
                     info!("aborting installation");
-                    return Ok(utils::ExitCode(0));
+                    return Ok(ExitCode(0));
                 }
                 Confirm::Yes => {
                     break;
@@ -591,7 +640,7 @@ pub(crate) async fn install(
             windows::ensure_prompt(cfg.process)?;
         }
 
-        return Ok(utils::ExitCode(1));
+        return Ok(ExitCode(1));
     }
 
     let cargo_home = canonical_cargo_home(cfg.process)?;
@@ -983,11 +1032,11 @@ pub(crate) fn uninstall(
     no_prompt: bool,
     no_modify_path: bool,
     process: &Process,
-) -> Result<utils::ExitCode> {
+) -> Result<ExitCode> {
     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));
+        return Ok(ExitCode(1));
     }
 
     let cargo_home = process.cargo_home()?;
@@ -1009,7 +1058,7 @@ pub(crate) fn uninstall(
         md(&mut process.stdout(), msg);
         if !common::confirm("\nContinue? (y/N)", false, process)? {
             info!("aborting uninstallation");
-            return Ok(utils::ExitCode(0));
+            return Ok(ExitCode(0));
         }
     }
 
@@ -1086,7 +1135,7 @@ pub(crate) fn uninstall(
 
     info!("rustup is uninstalled");
 
-    Ok(utils::ExitCode(0))
+    Ok(ExitCode(0))
 }
 
 #[derive(Clone, Copy, Debug)]
@@ -1126,30 +1175,6 @@ pub(crate) fn self_update_permitted(explicit: bool) -> Result<SelfUpdatePermissi
     Ok(SelfUpdatePermission::Permit)
 }
 
-/// Performs all of a self-update: check policy, download, apply and exit.
-pub(crate) async fn self_update(dl_cfg: &DownloadCfg<'_>) -> 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 = prepare_update(dl_cfg).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(dl_cfg.process)?;
-    }
-
-    Ok(utils::ExitCode(0))
-}
-
 /// Self update downloads rustup-init to `CARGO_HOME`/bin/rustup-init
 /// and runs it.
 ///
@@ -1165,7 +1190,7 @@ pub(crate) async fn self_update(dl_cfg: &DownloadCfg<'_>) -> Result<utils::ExitC
 /// (and on windows this process will not be running to do it),
 /// rustup-init is stored in `CARGO_HOME`/bin, and then deleted next
 /// time rustup runs.
-pub(crate) async fn update(cfg: &Cfg<'_>) -> Result<utils::ExitCode> {
+pub(crate) async fn update(cfg: &Cfg<'_>) -> Result<ExitCode> {
     common::warn_if_host_is_emulated(cfg.process);
 
     use SelfUpdatePermission::*;
@@ -1179,12 +1204,12 @@ pub(crate) async fn update(cfg: &Cfg<'_>) -> Result<utils::ExitCode> {
             // TODO: Detect which package manager and be more useful.
             error!("self-update is disabled for this build of rustup");
             error!("you should probably use your system package manager to update rustup");
-            return Ok(utils::ExitCode(1));
+            return Ok(ExitCode(1));
         }
         #[cfg(not(windows))]
         Skip => {
             info!("Skipping self-update at this time");
-            return Ok(utils::ExitCode(0));
+            return Ok(ExitCode(0));
         }
         Permit => {}
     }
@@ -1193,7 +1218,7 @@ pub(crate) async fn update(cfg: &Cfg<'_>) -> Result<utils::ExitCode> {
         Some(setup_path) => {
             let Some(version) = get_and_parse_new_rustup_version(&setup_path) else {
                 error!("failed to get rustup version");
-                return Ok(utils::ExitCode(1));
+                return Ok(ExitCode(1));
             };
 
             let _ = common::show_channel_update(
@@ -1214,7 +1239,7 @@ pub(crate) async fn update(cfg: &Cfg<'_>) -> Result<utils::ExitCode> {
         }
     }
 
-    Ok(utils::ExitCode(0))
+    Ok(ExitCode(0))
 }
 
 fn get_and_parse_new_rustup_version(path: &Path) -> Option<String> {