Simplify updates

src/cli/common.rs

@@ -280,35 +280,26 @@ fn show_channel_updates(
 
 pub(crate) async fn update_all_channels(
     cfg: &Cfg<'_>,
-    do_self_update: bool,
     force_update: bool,
 ) -> Result<utils::ExitCode> {
     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");
     }
 
-    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())?;
 
-    if do_self_update {
-        exit_code &= self_update(show_channel_updates, cfg.process).await?;
-    } else {
-        show_channel_updates()?;
+        let t = toolchains
+            .into_iter()
+            .map(|(p, s)| (PackageUpdate::Toolchain(p), s))
+            .collect();
+        show_channel_updates(cfg, t)?;
     }
+
     Ok(exit_code)
 }
 
@@ -350,10 +341,7 @@ pub(crate) fn self_update_permitted(explicit: bool) -> Result<SelfUpdatePermissi
 }
 
 /// Performs all of a self-update: check policy, download, apply and exit.
-pub(crate) async fn self_update<F>(before_restart: F, process: &Process) -> Result<utils::ExitCode>
-where
-    F: FnOnce() -> Result<()>,
-{
+pub(crate) async fn self_update(process: &Process) -> Result<utils::ExitCode> {
     match self_update_permitted(false)? {
         SelfUpdatePermission::HardFail => {
             error!("Unable to self-update.  STOP");
@@ -366,8 +354,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 {

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,
@@ -889,21 +889,17 @@ async fn check_updates(cfg: &Cfg<'_>, opts: CheckOpts) -> Result<utils::ExitCode
             }
         }
     }
-    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.
     // 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
+    let self_update = !cfg!(feature = "no-self-update")
         && self_update_mode == SelfUpdateMode::Enable
         && !opts.no_self_update;
 
-    if self_update
-        && matches!(
-            check_rustup_update(cfg.process).await?,
-            RustupUpdateAvailable::True
-        )
-    {
+    if self_update && check_rustup_update(cfg.process).await? {
         update_available = true;
     }
 
@@ -919,12 +915,12 @@ 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**
     // 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;
@@ -988,24 +984,28 @@ 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?;
         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();
     }
 
-    if !self_update::NEVER_SELF_UPDATE && self_update_mode == SelfUpdateMode::CheckOnly {
+    if !cfg!(feature = "no-self-update") && self_update_mode == SelfUpdateMode::CheckOnly {
         check_rustup_update(cfg.process).await?;
     }
 
-    if self_update::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")
     }
@@ -1778,7 +1778,7 @@ fn set_auto_self_update(
     cfg: &mut Cfg<'_>,
     auto_self_update_mode: SelfUpdateMode,
 ) -> Result<utils::ExitCode> {
-    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

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 {
@@ -261,6 +256,21 @@ pub enum SelfUpdateMode {
 }
 
 impl SelfUpdateMode {
+    pub(crate) fn from_cfg(cfg: &Cfg<'_>) -> anyhow::Result<Self> {
+        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",
@@ -940,7 +950,7 @@ async fn maybe_install_rust(
 }
 
 pub(crate) fn uninstall(no_prompt: bool, process: &Process) -> Result<utils::ExitCode> {
-    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));
@@ -1058,7 +1068,7 @@ pub(crate) async fn update(cfg: &Cfg<'_>) -> Result<utils::ExitCode> {
     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)?
@@ -1250,15 +1260,8 @@ impl fmt::Display for SchemaVersion {
     }
 }
 
-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
-pub(crate) enum RustupUpdateAvailable {
-    True,
-    False,
-}
-
-pub(crate) async fn check_rustup_update(process: &Process) -> Result<RustupUpdateAvailable> {
-    let mut update_available = RustupUpdateAvailable::False;
-
+/// 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);
     // Get current rustup version
     let current_version = env!("CARGO_PKG_VERSION");
@@ -1269,21 +1272,19 @@ pub(crate) async fn check_rustup_update(process: &Process) -> Result<RustupUpdat
     let _ = t.attr(terminalsource::Attr::Bold);
     write!(t.lock(), "rustup - ")?;
 
-    if current_version != available_version {
-        update_available = RustupUpdateAvailable::True;
-
+    Ok(if current_version != available_version {
         let _ = t.fg(terminalsource::Color::Yellow);
         write!(t.lock(), "Update available")?;
         let _ = t.reset();
         writeln!(t.lock(), " : {current_version} -> {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")]

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<SelfUpdateMode> {
-        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)