macOS: remove `readyToInstall` state in update capsule (#9529)
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 customised ui makes sense, since the current package is pretty small and only takes a few seconds to download for a normal network. And most of users intend to install this update. Also, there is no way back once you reach "Install and Restart" stage in `SPUStandardUserDriver`, unless you force quit ofc. ~~This pr also contains some further cleanup for #9170, since `InstallingView` is no longer visible for users.~~ #### `auto-download` When `auto-download = check`, Sparkle will call `UpdateDriver/showUpdateFound(with:state:reply:)`. When `auto-download = download`, previously no update ui was presented to the user, neither Sparkle's nor Ghostty’s. > I tried tick&untick auto download&install in Sparkle's alert, which doesn't seem to make any difference. With this pr, `auto-download = check` will behave the same, **`auto-download = download` will now prompt the user with a capsule.** https://github.com/user-attachments/assets/f994dd29-d348-4fea-8777-df3720d6e7af > [!NOTE] > I used AI to proofread my commentspull/9540/head
commit
1e7b8f6085
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue