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.
pull/9529/head
Lukas 2025-11-09 13:32:23 +01:00
parent b5912006ca
commit 7e3aba7c99
No known key found for this signature in database
GPG Key ID: 845CB61BD38F4E49
9 changed files with 44 additions and 102 deletions

View File

@ -815,6 +815,14 @@ class AppDelegate: NSObject,
autoUpdate == .check || autoUpdate == .download autoUpdate == .check || autoUpdate == .download
updateController.updater.automaticallyDownloadsUpdates = updateController.updater.automaticallyDownloadsUpdates =
autoUpdate == .download 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 // Config could change keybindings, so update everything that depends on that

View File

@ -10,7 +10,6 @@ import Combine
class UpdateController { class UpdateController {
private(set) var updater: SPUUpdater private(set) var updater: SPUUpdater
private let userDriver: UpdateDriver private let userDriver: UpdateDriver
private let updaterDelegate = UpdaterDelegate()
private var installCancellable: AnyCancellable? private var installCancellable: AnyCancellable?
var viewModel: UpdateViewModel { var viewModel: UpdateViewModel {
@ -32,7 +31,7 @@ class UpdateController {
hostBundle: hostBundle, hostBundle: hostBundle,
applicationBundle: hostBundle, applicationBundle: hostBundle,
userDriver: userDriver, userDriver: userDriver,
delegate: updaterDelegate delegate: userDriver
) )
} }

View File

@ -1,7 +1,7 @@
import Sparkle import Sparkle
import Cocoa import Cocoa
class UpdaterDelegate: NSObject, SPUUpdaterDelegate { extension UpdateDriver: SPUUpdaterDelegate {
func feedURLString(for updater: SPUUpdater) -> String? { func feedURLString(for updater: SPUUpdater) -> String? {
guard let appDelegate = NSApplication.shared.delegate as? AppDelegate else { guard let appDelegate = NSApplication.shared.delegate as? AppDelegate else {
return nil 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) { func updaterWillRelaunchApplication(_ updater: SPUUpdater) {
// When the updater is relaunching the application we want to get macOS // 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 // to invalidate and re-encode all of our restorable state so that when

View File

@ -164,10 +164,10 @@ class UpdateDriver: NSObject, SPUUserDriver {
} }
func showReady(toInstallAndRelaunch reply: @escaping @Sendable (SPUUserUpdateChoice) -> Void) { func showReady(toInstallAndRelaunch reply: @escaping @Sendable (SPUUserUpdateChoice) -> Void) {
viewModel.state = .readyToInstall(.init(reply: reply))
if !hasUnobtrusiveTarget { if !hasUnobtrusiveTarget {
standard.showReady(toInstallAndRelaunch: reply) standard.showReady(toInstallAndRelaunch: reply)
} else {
reply(.install)
} }
} }

View File

@ -35,10 +35,10 @@ struct UpdatePopoverView: View {
case .extracting(let extracting): case .extracting(let extracting):
ExtractingView(extracting: extracting) ExtractingView(extracting: extracting)
case .readyToInstall(let ready):
ReadyToInstallView(ready: ready, dismiss: dismiss)
case .installing(let installing): 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) InstallingView(installing: installing, dismiss: dismiss)
case .notFound(let notFound): case .notFound(let notFound):
@ -181,7 +181,7 @@ fileprivate struct UpdateAvailableView: View {
Spacer() Spacer()
Button("Install") { Button("Install and Relaunch") {
update.reply(.install) update.reply(.install)
dismiss() 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 { fileprivate struct InstallingView: View {
let installing: UpdateState.Installing let installing: UpdateState.Installing
let dismiss: DismissAction let dismiss: DismissAction

View File

@ -262,18 +262,7 @@ enum UpdateSimulator {
if j == 5 { if j == 5 {
DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
viewModel.state = .readyToInstall(.init( simulateInstalling(viewModel)
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
}
}
))
} }
} }
} }

View File

@ -30,10 +30,8 @@ class UpdateViewModel: ObservableObject {
return "Downloading…" return "Downloading…"
case .extracting(let extracting): case .extracting(let extracting):
return String(format: "Preparing: %.0f%%", extracting.progress * 100) return String(format: "Preparing: %.0f%%", extracting.progress * 100)
case .readyToInstall: case .installing(let install):
return "Ready to Install Update" return install.isAutoUpdate ? "Restart to Complete Update" : "Installing…"
case .installing:
return "Restart to Complete Update"
case .notFound: case .notFound:
return "No Updates Available" return "No Updates Available"
case .error(let err): case .error(let err):
@ -69,8 +67,6 @@ class UpdateViewModel: ObservableObject {
return "arrow.down.circle" return "arrow.down.circle"
case .extracting: case .extracting:
return "shippingbox" return "shippingbox"
case .readyToInstall:
return "restart.circle.fill"
case .installing: case .installing:
return "power.circle" return "power.circle"
case .notFound: case .notFound:
@ -96,10 +92,8 @@ class UpdateViewModel: ObservableObject {
return "Downloading the update package" return "Downloading the update package"
case .extracting: case .extracting:
return "Extracting and preparing the update" return "Extracting and preparing the update"
case .readyToInstall: case let .installing(install):
return "Update is ready to install" return install.isAutoUpdate ? "Restart to Complete Update" : "Installing update and preparing to restart"
case .installing:
return "Installing update and preparing to restart"
case .notFound: case .notFound:
return "You are running the latest version" return "You are running the latest version"
case .error: case .error:
@ -136,7 +130,7 @@ class UpdateViewModel: ObservableObject {
return .white return .white
case .checking: case .checking:
return .secondary return .secondary
case .updateAvailable, .readyToInstall: case .updateAvailable:
return .accentColor return .accentColor
case .downloading, .extracting, .installing: case .downloading, .extracting, .installing:
return .secondary return .secondary
@ -154,8 +148,6 @@ class UpdateViewModel: ObservableObject {
return Color(nsColor: NSColor.systemBlue.blended(withFraction: 0.3, of: .black) ?? .systemBlue) return Color(nsColor: NSColor.systemBlue.blended(withFraction: 0.3, of: .black) ?? .systemBlue)
case .updateAvailable: case .updateAvailable:
return .accentColor return .accentColor
case .readyToInstall:
return Color(nsColor: NSColor.systemGreen.blended(withFraction: 0.3, of: .black) ?? .systemGreen)
case .notFound: case .notFound:
return Color(nsColor: NSColor.systemBlue.blended(withFraction: 0.5, of: .black) ?? .systemBlue) return Color(nsColor: NSColor.systemBlue.blended(withFraction: 0.5, of: .black) ?? .systemBlue)
case .error: case .error:
@ -170,7 +162,7 @@ class UpdateViewModel: ObservableObject {
switch state { switch state {
case .permissionRequest: case .permissionRequest:
return .white return .white
case .updateAvailable, .readyToInstall: case .updateAvailable:
return .white return .white
case .notFound: case .notFound:
return .white return .white
@ -191,7 +183,6 @@ enum UpdateState: Equatable {
case error(Error) case error(Error)
case downloading(Downloading) case downloading(Downloading)
case extracting(Extracting) case extracting(Extracting)
case readyToInstall(ReadyToInstall)
case installing(Installing) case installing(Installing)
var isIdle: Bool { var isIdle: Bool {
@ -206,7 +197,6 @@ enum UpdateState: Equatable {
.updateAvailable, .updateAvailable,
.downloading, .downloading,
.extracting, .extracting,
.readyToInstall,
.installing: .installing:
return true return true
@ -223,8 +213,6 @@ enum UpdateState: Equatable {
available.reply(.dismiss) available.reply(.dismiss)
case .downloading(let downloading): case .downloading(let downloading):
downloading.cancel() downloading.cancel()
case .readyToInstall(let ready):
ready.reply(.dismiss)
case .notFound(let notFound): case .notFound(let notFound):
notFound.acknowledgement() notFound.acknowledgement()
case .error(let err): case .error(let err):
@ -241,8 +229,6 @@ enum UpdateState: Equatable {
switch self { switch self {
case .updateAvailable(let available): case .updateAvailable(let available):
available.reply(.install) available.reply(.install)
case .readyToInstall(let ready):
ready.reply(.install)
default: default:
break break
} }
@ -266,10 +252,8 @@ enum UpdateState: Equatable {
return lDown.progress == rDown.progress && lDown.expectedLength == rDown.expectedLength return lDown.progress == rDown.progress && lDown.expectedLength == rDown.expectedLength
case (.extracting(let lExt), .extracting(let rExt)): case (.extracting(let lExt), .extracting(let rExt)):
return lExt.progress == rExt.progress return lExt.progress == rExt.progress
case (.readyToInstall, .readyToInstall): case (.installing(let lInstall), .installing(let rInstall)):
return true return lInstall.isAutoUpdate == rInstall.isAutoUpdate
case (.installing, .installing):
return true
default: default:
return false return false
} }
@ -379,11 +363,9 @@ enum UpdateState: Equatable {
let progress: Double let progress: Double
} }
struct ReadyToInstall {
let reply: @Sendable (SPUUserUpdateChoice) -> Void
}
struct Installing { struct Installing {
/// True if this state is triggered by ``Ghostty/UpdateDriver/updater(_:willInstallUpdateOnQuit:immediateInstallationBlock:)``
var isAutoUpdate = false
let retryTerminatingApplication: () -> Void let retryTerminatingApplication: () -> Void
} }
} }

View File

@ -25,9 +25,11 @@ struct UpdateStateTests {
} }
@Test func testInstallingEquality() { @Test func testInstallingEquality() {
let state1: UpdateState = .installing(.init(retryTerminatingApplication: {})) let state1: UpdateState = .installing(.init(isAutoUpdate: false, retryTerminatingApplication: {}))
let state2: UpdateState = .installing(.init(retryTerminatingApplication: {})) let state2: UpdateState = .installing(.init(isAutoUpdate: false, retryTerminatingApplication: {}))
#expect(state1 == state2) #expect(state1 == state2)
let state3: UpdateState = .installing(.init(isAutoUpdate: true, retryTerminatingApplication: {}))
#expect(state3 != state2)
} }
@Test func testPermissionRequestEquality() { @Test func testPermissionRequestEquality() {
@ -38,12 +40,6 @@ struct UpdateStateTests {
#expect(state1 == state2) #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() { @Test func testDownloadingEqualityWithSameProgress() {
let state1: UpdateState = .downloading(.init(cancel: {}, expectedLength: 1000, progress: 500)) let state1: UpdateState = .downloading(.init(cancel: {}, expectedLength: 1000, progress: 500))
let state2: UpdateState = .downloading(.init(cancel: {}, expectedLength: 1000, progress: 500)) let state2: UpdateState = .downloading(.init(cancel: {}, expectedLength: 1000, progress: 500))

View File

@ -50,15 +50,11 @@ struct UpdateViewModelTests {
#expect(viewModel.text == "Preparing: 75%") #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() { @Test func testInstallingText() {
let viewModel = UpdateViewModel() 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") #expect(viewModel.text == "Restart to Complete Update")
} }