From dbebb40956259dbe6ca03333129fb65d048539f8 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 8 May 2024 19:13:28 +0200 Subject: [PATCH] linux: Store binary path before restart to handle deleted binary file (#11568) This fixes restart after updates not working on Linux. On Linux we can't reliably get the binary path after an update, because the original binary was deleted and the path will contain ` (deleted)`. See: https://github.com/rust-lang/rust/issues/69343 We *could* strip ` (deleted)` off, but that feels nasty. So instead we save the original binary path, before we do the installation, then restart. Later on, we can also change this to be a _new_ binary path returned by the installers, which we then have to start. Release Notes: - N/A --- .../src/activity_indicator.rs | 9 +++++--- crates/auto_update/src/auto_update.rs | 21 ++++++++++++++----- crates/collab_ui/src/collab_titlebar_item.rs | 4 ++-- crates/gpui/src/app.rs | 4 ++-- crates/gpui/src/platform.rs | 2 +- crates/gpui/src/platform/linux/platform.rs | 16 ++++++++------ crates/gpui/src/platform/mac/platform.rs | 2 +- crates/gpui/src/platform/test/platform.rs | 2 +- crates/gpui/src/platform/windows/platform.rs | 2 +- crates/workspace/src/workspace.rs | 12 ++++++++--- 10 files changed, 49 insertions(+), 25 deletions(-) diff --git a/crates/activity_indicator/src/activity_indicator.rs b/crates/activity_indicator/src/activity_indicator.rs index ade2bf7c71b783609254b026b6704dbeb5063303..b0ff76327864527f14d96f4c09f9abc520d918c4 100644 --- a/crates/activity_indicator/src/activity_indicator.rs +++ b/crates/activity_indicator/src/activity_indicator.rs @@ -281,11 +281,14 @@ impl ActivityIndicator { message: "Installing Zed update…".to_string(), on_click: None, }, - AutoUpdateStatus::Updated => Content { + AutoUpdateStatus::Updated { binary_path } => Content { icon: None, message: "Click to restart and update Zed".to_string(), - on_click: Some(Arc::new(|_, cx| { - workspace::restart(&Default::default(), cx) + on_click: Some(Arc::new({ + let restart = workspace::Restart { + binary_path: Some(binary_path.clone()), + }; + move |_, cx| workspace::restart(&restart, cx) })), }, AutoUpdateStatus::Errored => Content { diff --git a/crates/auto_update/src/auto_update.rs b/crates/auto_update/src/auto_update.rs index 2c95f9af5fc59616666d72f196da976ad3c1d3d6..d106d49488a55bac0f4fa7213f0a7cb9e0a41d30 100644 --- a/crates/auto_update/src/auto_update.rs +++ b/crates/auto_update/src/auto_update.rs @@ -56,16 +56,22 @@ struct UpdateRequestBody { telemetry: bool, } -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq)] pub enum AutoUpdateStatus { Idle, Checking, Downloading, Installing, - Updated, + Updated { binary_path: PathBuf }, Errored, } +impl AutoUpdateStatus { + pub fn is_updated(&self) -> bool { + matches!(self, Self::Updated { .. }) + } +} + pub struct AutoUpdater { status: AutoUpdateStatus, current_version: SemanticVersion, @@ -306,7 +312,7 @@ impl AutoUpdater { } pub fn poll(&mut self, cx: &mut ModelContext) { - if self.pending_poll.is_some() || self.status == AutoUpdateStatus::Updated { + if self.pending_poll.is_some() || self.status.is_updated() { return; } @@ -328,7 +334,7 @@ impl AutoUpdater { } pub fn status(&self) -> AutoUpdateStatus { - self.status + self.status.clone() } pub fn dismiss_error(&mut self, cx: &mut ModelContext) { @@ -404,6 +410,11 @@ impl AutoUpdater { cx.notify(); })?; + // We store the path of our current binary, before we install, since installation might + // delete it. Once deleted, it's hard to get the path to our binary on Linux. + // So we cache it here, which allows us to then restart later on. + let binary_path = cx.update(|cx| cx.app_path())??; + match OS { "macos" => install_release_macos(&temp_dir, downloaded_asset, &cx).await, "linux" => install_release_linux(&temp_dir, downloaded_asset, &cx).await, @@ -413,7 +424,7 @@ 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; + this.status = AutoUpdateStatus::Updated { binary_path }; cx.notify(); })?; diff --git a/crates/collab_ui/src/collab_titlebar_item.rs b/crates/collab_ui/src/collab_titlebar_item.rs index acbf06d54dfbb2f3cbd961b037bef6a5d9550207..fd1aa74779c9f7301485ee46f2cf7c7b34478f00 100644 --- a/crates/collab_ui/src/collab_titlebar_item.rs +++ b/crates/collab_ui/src/collab_titlebar_item.rs @@ -677,7 +677,7 @@ impl CollabTitlebarItem { client::Status::UpgradeRequired => { let auto_updater = auto_update::AutoUpdater::get(cx); let label = match auto_updater.map(|auto_update| auto_update.read(cx).status()) { - Some(AutoUpdateStatus::Updated) => "Please restart Zed to Collaborate", + Some(AutoUpdateStatus::Updated { .. }) => "Please restart Zed to Collaborate", Some(AutoUpdateStatus::Installing) | Some(AutoUpdateStatus::Downloading) | Some(AutoUpdateStatus::Checking) => "Updating...", @@ -691,7 +691,7 @@ impl CollabTitlebarItem { .label_size(LabelSize::Small) .on_click(|_, cx| { if let Some(auto_updater) = auto_update::AutoUpdater::get(cx) { - if auto_updater.read(cx).status() == AutoUpdateStatus::Updated { + if auto_updater.read(cx).status().is_updated() { workspace::restart(&Default::default(), cx); return; } diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index e7d9ac93065c168ba9993fab4422cc0ec193d653..76aef9e6099c6fcc05d97824ac3d4de929c4da3f 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -642,8 +642,8 @@ impl AppContext { } /// Restart the application. - pub fn restart(&self) { - self.platform.restart() + pub fn restart(&self, binary_path: Option) { + self.platform.restart(binary_path) } /// Returns the local timezone at the platform level. diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 1312b66bbe59dbed0c09b29701b424c5be8d64df..26e1868ff39f6c2d11807d9eab2a71a4df777a7e 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -98,7 +98,7 @@ pub(crate) trait Platform: 'static { fn run(&self, on_finish_launching: Box); fn quit(&self); - fn restart(&self); + fn restart(&self, binary_path: Option); fn activate(&self, ignoring_other_apps: bool); fn hide(&self); fn hide_other_apps(&self); diff --git a/crates/gpui/src/platform/linux/platform.rs b/crates/gpui/src/platform/linux/platform.rs index f09f92d31a5ddb25b7d95268613a641ab7798e5a..159e6e9220f7a760c41471f63be15a3d51cfa656 100644 --- a/crates/gpui/src/platform/linux/platform.rs +++ b/crates/gpui/src/platform/linux/platform.rs @@ -136,17 +136,21 @@ impl Platform for P { self.with_common(|common| common.signal.stop()); } - fn restart(&self) { + fn restart(&self, binary_path: Option) { use std::os::unix::process::CommandExt as _; // get the process id of the current process let app_pid = std::process::id().to_string(); // get the path to the executable - let app_path = match self.app_path() { - Ok(path) => path, - Err(err) => { - log::error!("Failed to get app path: {:?}", err); - return; + let app_path = if let Some(path) = binary_path { + path + } else { + match self.app_path() { + Ok(path) => path, + Err(err) => { + log::error!("Failed to get app path: {:?}", err); + return; + } } }; diff --git a/crates/gpui/src/platform/mac/platform.rs b/crates/gpui/src/platform/mac/platform.rs index b5b220d942a780d859847ccddda93b5d23ba19a8..f7754687ea111372ebb0ad5118151aae5300f314 100644 --- a/crates/gpui/src/platform/mac/platform.rs +++ b/crates/gpui/src/platform/mac/platform.rs @@ -396,7 +396,7 @@ impl Platform for MacPlatform { } } - fn restart(&self) { + fn restart(&self, _binary_path: Option) { use std::os::unix::process::CommandExt as _; let app_pid = std::process::id().to_string(); diff --git a/crates/gpui/src/platform/test/platform.rs b/crates/gpui/src/platform/test/platform.rs index c243f4fac17e4734fbbd2b51fbb086ecf6d85271..8fa558e435a6602600ba2ccc51411d3a924e804e 100644 --- a/crates/gpui/src/platform/test/platform.rs +++ b/crates/gpui/src/platform/test/platform.rs @@ -140,7 +140,7 @@ impl Platform for TestPlatform { fn quit(&self) {} - fn restart(&self) { + fn restart(&self, _: Option) { unimplemented!() } diff --git a/crates/gpui/src/platform/windows/platform.rs b/crates/gpui/src/platform/windows/platform.rs index f6d53c5aa14ad9eae0add861222d934b527a1342..2da0e7acb1f2d1ac288b05ec56511db54a90b261 100644 --- a/crates/gpui/src/platform/windows/platform.rs +++ b/crates/gpui/src/platform/windows/platform.rs @@ -268,7 +268,7 @@ impl Platform for WindowsPlatform { .detach(); } - fn restart(&self) { + fn restart(&self, _: Option) { let pid = std::process::id(); let Some(app_path) = self.app_path().log_err() else { return; diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 20f88ab0efacc632b02e4519418884a0a8d8bfd5..ac84ac8d4c03621c9611081ea077db3eb68a5958 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -130,7 +130,6 @@ actions!( NewCenterTerminal, NewSearch, Feedback, - Restart, Welcome, ToggleZoom, ToggleLeftDock, @@ -185,6 +184,11 @@ pub struct CloseInactiveTabsAndPanes { #[derive(Clone, Deserialize, PartialEq)] pub struct SendKeystrokes(pub String); +#[derive(Clone, Deserialize, PartialEq, Default)] +pub struct Restart { + pub binary_path: Option, +} + impl_actions!( workspace, [ @@ -194,6 +198,7 @@ impl_actions!( CloseInactiveTabsAndPanes, NewFileInDirection, OpenTerminal, + Restart, Save, SaveAll, SwapPaneInDirection, @@ -5020,7 +5025,7 @@ pub fn join_in_room_project( }) } -pub fn restart(_: &Restart, cx: &mut AppContext) { +pub fn restart(restart: &Restart, cx: &mut AppContext) { let should_confirm = WorkspaceSettings::get_global(cx).confirm_quit; let mut workspace_windows = cx .windows() @@ -5046,6 +5051,7 @@ pub fn restart(_: &Restart, cx: &mut AppContext) { .ok(); } + let binary_path = restart.binary_path.clone(); cx.spawn(|mut cx| async move { if let Some(prompt) = prompt { let answer = prompt.await?; @@ -5065,7 +5071,7 @@ pub fn restart(_: &Restart, cx: &mut AppContext) { } } - cx.update(|cx| cx.restart()) + cx.update(|cx| cx.restart(binary_path)) }) .detach_and_log_err(cx); }