Allow updater to check for updates after downloading one (#30969)

Joseph T. Lyons and Ben Kunkle created 6 months ago

Closes https://github.com/zed-industries/zed/issues/8968

This PR addresses the following scenario:

1. User's Zed polls for an update, finds one, and installs it
2. User doesn't immediately restart Zed, a new update is released, and
the previous version of Zed would stop polling (ignoring the new update)
3. User eventually restarts Zed and is immediately prompted to install
another update

With this change, the auto-updater will continue polling for and
installing new versions even after an initial update is found, reducing
update prompts on restart.

---

This PR does not address the following scenario:

1. User's Zed polls for an update, finds one, and installs it
2. Another update is released before the next scheduled polling interval
3. User restarts Zed and is immediately prompted to install the newer
update

Release Notes:

- Improved the auto-updater to continue checking for updates even after
finding and installing an initial update. This reduces situations where
users are prompted to install another update immediately after
restarting from a previous update.

Co-authored-by: Ben Kunkle <[e-mail]>

Change summary

crates/activity_indicator/src/activity_indicator.rs |  2 
crates/auto_update/src/auto_update.rs               | 85 ++++++++++----
2 files changed, 63 insertions(+), 24 deletions(-)

Detailed changes

crates/activity_indicator/src/activity_indicator.rs

@@ -485,7 +485,7 @@ impl ActivityIndicator {
                         this.dismiss_error_message(&DismissErrorMessage, window, cx)
                     })),
                 }),
-                AutoUpdateStatus::Updated { binary_path } => Some(Content {
+                AutoUpdateStatus::Updated { binary_path, .. } => Some(Content {
                     icon: None,
                     message: "Click to restart and update Zed".to_string(),
                     on_click: Some(Arc::new({

crates/auto_update/src/auto_update.rs

@@ -39,13 +39,22 @@ struct UpdateRequestBody {
     destination: &'static str,
 }
 
+#[derive(Clone, PartialEq, Eq)]
+pub enum VersionCheckType {
+    Sha(String),
+    Semantic(SemanticVersion),
+}
+
 #[derive(Clone, PartialEq, Eq)]
 pub enum AutoUpdateStatus {
     Idle,
     Checking,
     Downloading,
     Installing,
-    Updated { binary_path: PathBuf },
+    Updated {
+        binary_path: PathBuf,
+        version: VersionCheckType,
+    },
     Errored,
 }
 
@@ -62,7 +71,7 @@ pub struct AutoUpdater {
     pending_poll: Option<Task<Option<()>>>,
 }
 
-#[derive(Deserialize, Debug)]
+#[derive(Deserialize, Clone, Debug)]
 pub struct JsonRelease {
     pub version: String,
     pub url: String,
@@ -307,7 +316,7 @@ impl AutoUpdater {
     }
 
     pub fn poll(&mut self, cx: &mut Context<Self>) {
-        if self.pending_poll.is_some() || self.status.is_updated() {
+        if self.pending_poll.is_some() {
             return;
         }
 
@@ -483,36 +492,63 @@ impl AutoUpdater {
         Self::get_release(this, asset, os, arch, None, release_channel, cx).await
     }
 
+    fn installed_update_version(&self) -> Option<VersionCheckType> {
+        match &self.status {
+            AutoUpdateStatus::Updated { version, .. } => Some(version.clone()),
+            _ => None,
+        }
+    }
+
     async fn update(this: Entity<Self>, mut cx: AsyncApp) -> Result<()> {
-        let (client, current_version, release_channel) = this.update(&mut cx, |this, cx| {
-            this.status = AutoUpdateStatus::Checking;
-            cx.notify();
-            (
-                this.http_client.clone(),
-                this.current_version,
-                ReleaseChannel::try_global(cx),
-            )
-        })?;
+        let (client, current_version, installed_update_version, release_channel) =
+            this.update(&mut cx, |this, cx| {
+                this.status = AutoUpdateStatus::Checking;
+                cx.notify();
+                (
+                    this.http_client.clone(),
+                    this.current_version,
+                    this.installed_update_version(),
+                    ReleaseChannel::try_global(cx),
+                )
+            })?;
 
         let release =
             Self::get_latest_release(&this, "zed", OS, ARCH, release_channel, &mut cx).await?;
 
-        let should_download = match *RELEASE_CHANNEL {
-            ReleaseChannel::Nightly => cx
-                .update(|cx| AppCommitSha::try_global(cx).map(|sha| release.version != sha.0))
-                .ok()
-                .flatten()
-                .unwrap_or(true),
-            _ => release.version.parse::<SemanticVersion>()? > current_version,
+        let update_version_to_install = match *RELEASE_CHANNEL {
+            ReleaseChannel::Nightly => {
+                let should_download = cx
+                    .update(|cx| AppCommitSha::try_global(cx).map(|sha| release.version != sha.0))
+                    .ok()
+                    .flatten()
+                    .unwrap_or(true);
+
+                should_download.then(|| VersionCheckType::Sha(release.version.clone()))
+            }
+            _ => {
+                let installed_version =
+                    installed_update_version.unwrap_or(VersionCheckType::Semantic(current_version));
+                match installed_version {
+                    VersionCheckType::Sha(_) => {
+                        log::warn!("Unexpected SHA-based version in non-nightly build");
+                        Some(installed_version)
+                    }
+                    VersionCheckType::Semantic(semantic_comparison_version) => {
+                        let latest_release_version = release.version.parse::<SemanticVersion>()?;
+                        let should_download = latest_release_version > semantic_comparison_version;
+                        should_download.then(|| VersionCheckType::Semantic(latest_release_version))
+                    }
+                }
+            }
         };
 
-        if !should_download {
+        let Some(update_version) = update_version_to_install else {
             this.update(&mut cx, |this, cx| {
                 this.status = AutoUpdateStatus::Idle;
                 cx.notify();
             })?;
             return Ok(());
-        }
+        };
 
         this.update(&mut cx, |this, cx| {
             this.status = AutoUpdateStatus::Downloading;
@@ -534,7 +570,7 @@ impl AutoUpdater {
         );
 
         let downloaded_asset = installer_dir.path().join(filename);
-        download_release(&downloaded_asset, release, client, &cx).await?;
+        download_release(&downloaded_asset, release.clone(), client, &cx).await?;
 
         this.update(&mut cx, |this, cx| {
             this.status = AutoUpdateStatus::Installing;
@@ -551,7 +587,10 @@ impl AutoUpdater {
         this.update(&mut cx, |this, cx| {
             this.set_should_show_update_notification(true, cx)
                 .detach_and_log_err(cx);
-            this.status = AutoUpdateStatus::Updated { binary_path };
+            this.status = AutoUpdateStatus::Updated {
+                binary_path,
+                version: update_version,
+            };
             cx.notify();
         })?;