From 7e3aba7c9987e04f45f3986d33bbb5f648b66664 Mon Sep 17 00:00:00 2001 From: Lukas <134181853+bo2themax@users.noreply.github.com> Date: Sun, 9 Nov 2025 13:32:23 +0100 Subject: [PATCH] macOS: remove `readyToInstall` state in update capsule There is a sparkle-related 'issue' with the previous implementation. When you download/install in the `updateAvailable` state, if you don't install it, then check the updates again. Sparkle loses its downloaded stage in the delegate (it's normal when I use the sparkle source code). This time, when you click install in the `updateAvailable` state, it just uses the previous downloaded package and starts to install, without calling `showReady(toInstallAndRelaunch:)`. I think removing `readyToInstall` in our customed ui, will reduce one step to install an update for most of the users out there, which makes sense, since the current package is pretty small, only takes a few seconds to download for a normal network, and they intended to install this update. --- macos/Sources/App/macOS/AppDelegate.swift | 8 ++++ .../Features/Update/UpdateController.swift | 3 +- .../Features/Update/UpdateDelegate.swift | 12 ++++- .../Features/Update/UpdateDriver.swift | 4 +- .../Features/Update/UpdatePopoverView.swift | 46 ++----------------- .../Features/Update/UpdateSimulator.swift | 13 +----- .../Features/Update/UpdateViewModel.swift | 38 ++++----------- macos/Tests/Update/UpdateStateTests.swift | 12 ++--- macos/Tests/Update/UpdateViewModelTests.swift | 10 ++-- 9 files changed, 44 insertions(+), 102 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index 5da2f1d5b..57e0212bb 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -815,6 +815,14 @@ class AppDelegate: NSObject, autoUpdate == .check || autoUpdate == .download updateController.updater.automaticallyDownloadsUpdates = autoUpdate == .download + /** + To test `auto-update` easily, uncomment the line below and + delete `SUEnableAutomaticChecks` in Ghostty-Info.plist. + + Note: When `auto-update = download`, you may need to + `Clean Build Folder` if a background install has already begun. + */ + //updateController.updater.checkForUpdatesInBackground() } // Config could change keybindings, so update everything that depends on that diff --git a/macos/Sources/Features/Update/UpdateController.swift b/macos/Sources/Features/Update/UpdateController.swift index 8a2a939bd..939eed420 100644 --- a/macos/Sources/Features/Update/UpdateController.swift +++ b/macos/Sources/Features/Update/UpdateController.swift @@ -10,7 +10,6 @@ import Combine class UpdateController { private(set) var updater: SPUUpdater private let userDriver: UpdateDriver - private let updaterDelegate = UpdaterDelegate() private var installCancellable: AnyCancellable? var viewModel: UpdateViewModel { @@ -32,7 +31,7 @@ class UpdateController { hostBundle: hostBundle, applicationBundle: hostBundle, userDriver: userDriver, - delegate: updaterDelegate + delegate: userDriver ) } diff --git a/macos/Sources/Features/Update/UpdateDelegate.swift b/macos/Sources/Features/Update/UpdateDelegate.swift index 1112c1f44..26242b49e 100644 --- a/macos/Sources/Features/Update/UpdateDelegate.swift +++ b/macos/Sources/Features/Update/UpdateDelegate.swift @@ -1,7 +1,7 @@ import Sparkle import Cocoa -class UpdaterDelegate: NSObject, SPUUpdaterDelegate { +extension UpdateDriver: SPUUpdaterDelegate { func feedURLString(for updater: SPUUpdater) -> String? { guard let appDelegate = NSApplication.shared.delegate as? AppDelegate else { return nil @@ -16,6 +16,16 @@ class UpdaterDelegate: NSObject, SPUUpdaterDelegate { } } + /// Called when an update is scheduled to install silently, + /// which occurs when `auto-update = download`. + /// + /// When `auto-update = check`, Sparkle will call the corresponding + /// delegate method on the responsible driver instead. + func updater(_ updater: SPUUpdater, willInstallUpdateOnQuit item: SUAppcastItem, immediateInstallationBlock immediateInstallHandler: @escaping () -> Void) -> Bool { + viewModel.state = .installing(.init(isAutoUpdate: true, retryTerminatingApplication: immediateInstallHandler)) + return true + } + func updaterWillRelaunchApplication(_ updater: SPUUpdater) { // When the updater is relaunching the application we want to get macOS // to invalidate and re-encode all of our restorable state so that when diff --git a/macos/Sources/Features/Update/UpdateDriver.swift b/macos/Sources/Features/Update/UpdateDriver.swift index 27be6b0df..94b43a3f3 100644 --- a/macos/Sources/Features/Update/UpdateDriver.swift +++ b/macos/Sources/Features/Update/UpdateDriver.swift @@ -164,10 +164,10 @@ class UpdateDriver: NSObject, SPUUserDriver { } func showReady(toInstallAndRelaunch reply: @escaping @Sendable (SPUUserUpdateChoice) -> Void) { - viewModel.state = .readyToInstall(.init(reply: reply)) - if !hasUnobtrusiveTarget { standard.showReady(toInstallAndRelaunch: reply) + } else { + reply(.install) } } diff --git a/macos/Sources/Features/Update/UpdatePopoverView.swift b/macos/Sources/Features/Update/UpdatePopoverView.swift index 770b9aedd..08d25a4d1 100644 --- a/macos/Sources/Features/Update/UpdatePopoverView.swift +++ b/macos/Sources/Features/Update/UpdatePopoverView.swift @@ -35,10 +35,10 @@ struct UpdatePopoverView: View { case .extracting(let extracting): ExtractingView(extracting: extracting) - case .readyToInstall(let ready): - ReadyToInstallView(ready: ready, dismiss: dismiss) - case .installing(let installing): + // This is only required when `installing.isAutoUpdate == true`, + // but we keep it anyway, just in case something unexpected + // happens during installing InstallingView(installing: installing, dismiss: dismiss) case .notFound(let notFound): @@ -181,7 +181,7 @@ fileprivate struct UpdateAvailableView: View { Spacer() - Button("Install") { + Button("Install and Relaunch") { update.reply(.install) dismiss() } @@ -274,44 +274,6 @@ fileprivate struct ExtractingView: View { } } -fileprivate struct ReadyToInstallView: View { - let ready: UpdateState.ReadyToInstall - let dismiss: DismissAction - - var body: some View { - VStack(alignment: .leading, spacing: 16) { - VStack(alignment: .leading, spacing: 8) { - Text("Ready to Install") - .font(.system(size: 13, weight: .semibold)) - - Text("The update is ready to install.") - .font(.system(size: 11)) - .foregroundColor(.secondary) - } - - HStack(spacing: 8) { - Button("Later") { - ready.reply(.dismiss) - dismiss() - } - .keyboardShortcut(.cancelAction) - .controlSize(.small) - - Spacer() - - Button("Install and Relaunch") { - ready.reply(.install) - dismiss() - } - .keyboardShortcut(.defaultAction) - .buttonStyle(.borderedProminent) - .controlSize(.small) - } - } - .padding(16) - } -} - fileprivate struct InstallingView: View { let installing: UpdateState.Installing let dismiss: DismissAction diff --git a/macos/Sources/Features/Update/UpdateSimulator.swift b/macos/Sources/Features/Update/UpdateSimulator.swift index c855282c0..cb4383a00 100644 --- a/macos/Sources/Features/Update/UpdateSimulator.swift +++ b/macos/Sources/Features/Update/UpdateSimulator.swift @@ -262,18 +262,7 @@ enum UpdateSimulator { if j == 5 { DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { - viewModel.state = .readyToInstall(.init( - reply: { choice in - if choice == .install { - viewModel.state = .installing(.init(retryTerminatingApplication: { - print("Restart button clicked in simulator - resetting to idle") - viewModel.state = .idle - })) - } else { - viewModel.state = .idle - } - } - )) + simulateInstalling(viewModel) } } } diff --git a/macos/Sources/Features/Update/UpdateViewModel.swift b/macos/Sources/Features/Update/UpdateViewModel.swift index 7a92337cc..96cbe7c3d 100644 --- a/macos/Sources/Features/Update/UpdateViewModel.swift +++ b/macos/Sources/Features/Update/UpdateViewModel.swift @@ -30,10 +30,8 @@ class UpdateViewModel: ObservableObject { return "Downloading…" case .extracting(let extracting): return String(format: "Preparing: %.0f%%", extracting.progress * 100) - case .readyToInstall: - return "Ready to Install Update" - case .installing: - return "Restart to Complete Update" + case .installing(let install): + return install.isAutoUpdate ? "Restart to Complete Update" : "Installing…" case .notFound: return "No Updates Available" case .error(let err): @@ -69,8 +67,6 @@ class UpdateViewModel: ObservableObject { return "arrow.down.circle" case .extracting: return "shippingbox" - case .readyToInstall: - return "restart.circle.fill" case .installing: return "power.circle" case .notFound: @@ -96,10 +92,8 @@ class UpdateViewModel: ObservableObject { return "Downloading the update package" case .extracting: return "Extracting and preparing the update" - case .readyToInstall: - return "Update is ready to install" - case .installing: - return "Installing update and preparing to restart" + case let .installing(install): + return install.isAutoUpdate ? "Restart to Complete Update" : "Installing update and preparing to restart" case .notFound: return "You are running the latest version" case .error: @@ -136,7 +130,7 @@ class UpdateViewModel: ObservableObject { return .white case .checking: return .secondary - case .updateAvailable, .readyToInstall: + case .updateAvailable: return .accentColor case .downloading, .extracting, .installing: return .secondary @@ -154,8 +148,6 @@ class UpdateViewModel: ObservableObject { return Color(nsColor: NSColor.systemBlue.blended(withFraction: 0.3, of: .black) ?? .systemBlue) case .updateAvailable: return .accentColor - case .readyToInstall: - return Color(nsColor: NSColor.systemGreen.blended(withFraction: 0.3, of: .black) ?? .systemGreen) case .notFound: return Color(nsColor: NSColor.systemBlue.blended(withFraction: 0.5, of: .black) ?? .systemBlue) case .error: @@ -170,7 +162,7 @@ class UpdateViewModel: ObservableObject { switch state { case .permissionRequest: return .white - case .updateAvailable, .readyToInstall: + case .updateAvailable: return .white case .notFound: return .white @@ -191,7 +183,6 @@ enum UpdateState: Equatable { case error(Error) case downloading(Downloading) case extracting(Extracting) - case readyToInstall(ReadyToInstall) case installing(Installing) var isIdle: Bool { @@ -206,7 +197,6 @@ enum UpdateState: Equatable { .updateAvailable, .downloading, .extracting, - .readyToInstall, .installing: return true @@ -223,8 +213,6 @@ enum UpdateState: Equatable { available.reply(.dismiss) case .downloading(let downloading): downloading.cancel() - case .readyToInstall(let ready): - ready.reply(.dismiss) case .notFound(let notFound): notFound.acknowledgement() case .error(let err): @@ -241,8 +229,6 @@ enum UpdateState: Equatable { switch self { case .updateAvailable(let available): available.reply(.install) - case .readyToInstall(let ready): - ready.reply(.install) default: break } @@ -266,10 +252,8 @@ enum UpdateState: Equatable { return lDown.progress == rDown.progress && lDown.expectedLength == rDown.expectedLength case (.extracting(let lExt), .extracting(let rExt)): return lExt.progress == rExt.progress - case (.readyToInstall, .readyToInstall): - return true - case (.installing, .installing): - return true + case (.installing(let lInstall), .installing(let rInstall)): + return lInstall.isAutoUpdate == rInstall.isAutoUpdate default: return false } @@ -379,11 +363,9 @@ enum UpdateState: Equatable { let progress: Double } - struct ReadyToInstall { - let reply: @Sendable (SPUUserUpdateChoice) -> Void - } - struct Installing { + /// True if this state is triggered by ``Ghostty/UpdateDriver/updater(_:willInstallUpdateOnQuit:immediateInstallationBlock:)`` + var isAutoUpdate = false let retryTerminatingApplication: () -> Void } } diff --git a/macos/Tests/Update/UpdateStateTests.swift b/macos/Tests/Update/UpdateStateTests.swift index 269cd3153..1d1d7b37d 100644 --- a/macos/Tests/Update/UpdateStateTests.swift +++ b/macos/Tests/Update/UpdateStateTests.swift @@ -25,9 +25,11 @@ struct UpdateStateTests { } @Test func testInstallingEquality() { - let state1: UpdateState = .installing(.init(retryTerminatingApplication: {})) - let state2: UpdateState = .installing(.init(retryTerminatingApplication: {})) + let state1: UpdateState = .installing(.init(isAutoUpdate: false, retryTerminatingApplication: {})) + let state2: UpdateState = .installing(.init(isAutoUpdate: false, retryTerminatingApplication: {})) #expect(state1 == state2) + let state3: UpdateState = .installing(.init(isAutoUpdate: true, retryTerminatingApplication: {})) + #expect(state3 != state2) } @Test func testPermissionRequestEquality() { @@ -38,12 +40,6 @@ struct UpdateStateTests { #expect(state1 == state2) } - @Test func testReadyToInstallEquality() { - let state1: UpdateState = .readyToInstall(.init(reply: { _ in })) - let state2: UpdateState = .readyToInstall(.init(reply: { _ in })) - #expect(state1 == state2) - } - @Test func testDownloadingEqualityWithSameProgress() { let state1: UpdateState = .downloading(.init(cancel: {}, expectedLength: 1000, progress: 500)) let state2: UpdateState = .downloading(.init(cancel: {}, expectedLength: 1000, progress: 500)) diff --git a/macos/Tests/Update/UpdateViewModelTests.swift b/macos/Tests/Update/UpdateViewModelTests.swift index e41804e08..5b223c59d 100644 --- a/macos/Tests/Update/UpdateViewModelTests.swift +++ b/macos/Tests/Update/UpdateViewModelTests.swift @@ -50,15 +50,11 @@ struct UpdateViewModelTests { #expect(viewModel.text == "Preparing: 75%") } - @Test func testReadyToInstallText() { - let viewModel = UpdateViewModel() - viewModel.state = .readyToInstall(.init(reply: { _ in })) - #expect(viewModel.text == "Ready to Install Update") - } - @Test func testInstallingText() { let viewModel = UpdateViewModel() - viewModel.state = .installing(.init(retryTerminatingApplication: {})) + viewModel.state = .installing(.init(isAutoUpdate: false, retryTerminatingApplication: {})) + #expect(viewModel.text == "Installing…") + viewModel.state = .installing(.init(isAutoUpdate: true, retryTerminatingApplication: {})) #expect(viewModel.text == "Restart to Complete Update") }