Merge branch 'fix/upgrade-failures'

BrewBar/BrewBarApp.swift

@@ -47,6 +47,9 @@ class AppDelegate: NSObject, NSApplicationDelegate {
     /// last known outdated list, shown in the Upgrade All confirmation
     var cachedOutdated: OutdatedPackages = .none
     var lastNotifiedOutdated: Set<String> = []
+    /// packages whose last upgrade attempt failed; reset when an upgrade
+    /// run starts, displayed once by updateStatus after the run's refetch
+    var failedUpgrades: [String] = []
 
     let pathMonitor = NWPathMonitor()
     /// false until the monitor's first update, so the launch refresh waits
@@ -354,12 +357,15 @@ class AppDelegate: NSObject, NSApplicationDelegate {
 
         statusMenuItem.title = "Upgrading..."
         setMenuBarIcon("brewbar-updating")
+        failedUpgrades = []
 
         runBrew("update") { _, _ in
             // packages that turn outdated only after this brew update are not
             // in cachedOutdated yet; the final refetch will surface them
+            let formulaNames = formulae ? self.cachedOutdated.formulae.map(\.name) : []
             let caskNames = casks ? self.cachedOutdated.casks.map(\.name) : []
-            let upgradeCasksThenRefresh = {
+
+            self.upgradeFormulaeSequentially(formulaNames) {
                 self.upgradeCasksSequentially(caskNames) {
                     self.cleanupAfterUpgradeIfEnabled {
                         // a manual upgrade just ran; don't chain an auto-upgrade
@@ -367,14 +373,24 @@ class AppDelegate: NSObject, NSApplicationDelegate {
                     }
                 }
             }
+        }
+    }
 
-            if formulae, !self.cachedOutdated.formulae.isEmpty {
+    /// Formulae also upgrade one at a time: a single broken formula (no
-                self.runBrew("upgrade --formula") { _, _ in
+    /// bottle for this macOS, deprecated, …) makes brew's batch upgrade
-                    upgradeCasksThenRefresh()
+    /// error out, taking every other pending formula down with it.
-                }
+    func upgradeFormulaeSequentially(_ names: [String], completion: @escaping () -> Void) {
-            } else {
+        guard let next = names.first else {
-                upgradeCasksThenRefresh()
+            completion()
+            return
+        }
+
+        statusMenuItem.title = "Upgrading \(next)..."
+        runBrew("upgrade --formula \(next)", askpassMessage: "upgrade \(next)") { _, success in
+            if !success {
+                self.failedUpgrades.append(next)
             }
+            self.upgradeFormulaeSequentially(Array(names.dropFirst()), completion: completion)
         }
     }
 
@@ -398,12 +414,18 @@ class AppDelegate: NSObject, NSApplicationDelegate {
     /// (An explicitly named cask upgrades even when self-updating, no
     /// --greedy needed.)
     func upgradeCask(_ name: String, completion: @escaping () -> Void) {
-        runBrew("upgrade --cask \(name)", askpassMessage: "upgrade \(name)") { output, _ in
+        runBrew("upgrade --cask \(name)", askpassMessage: "upgrade \(name)") { output, success in
             guard output.contains("cannot be upgraded as-is") else {
+                if !success {
+                    self.failedUpgrades.append(name)
+                }
                 completion()
                 return
             }
-            self.runBrew("reinstall --cask --force \(name)", askpassMessage: "reinstall \(name)") { _, _ in
+            self.runBrew("reinstall --cask --force \(name)", askpassMessage: "reinstall \(name)") { _, reinstalled in
+                if !reinstalled {
+                    self.failedUpgrades.append(name)
+                }
                 completion()
             }
         }
@@ -507,23 +529,18 @@ class AppDelegate: NSObject, NSApplicationDelegate {
     func runAutoUpgrade(formulae: Bool, casks: Bool) {
         statusMenuItem.title = "Auto-upgrading..."
         setMenuBarIcon("brewbar-updating")
+        failedUpgrades = []
 
+        let formulaNames = formulae ? cachedOutdated.formulae.map(\.name) : []
         let caskNames = casks ? cachedOutdated.casks.map(\.name) : []
-        let upgradeCasksThenRefresh = {
+
+        upgradeFormulaeSequentially(formulaNames) {
             self.upgradeCasksSequentially(caskNames) {
                 self.cleanupAfterUpgradeIfEnabled {
                     self.fetchOutdated(allowAutoUpgrade: false)
                 }
             }
         }
-
-        if formulae {
-            runBrew("upgrade --formula") { _, _ in
-                upgradeCasksThenRefresh()
-            }
-        } else {
-            upgradeCasksThenRefresh()
-        }
     }
 
     func updateOutdatedMenu(with outdated: OutdatedPackages) {
@@ -552,7 +569,13 @@ class AppDelegate: NSObject, NSApplicationDelegate {
     }
 
     func updateStatus(count: Int) {
-        if count == 0 {
+        // surface what the last upgrade run could not handle — brew only
+        // reports these in its output, which lands in the logs. Consumed
+        // here so the next plain refresh shows the normal state again.
+        if !failedUpgrades.isEmpty {
+            statusMenuItem.title = "⚠️ Upgrade failed: \(failedUpgrades.joined(separator: ", ")) — see Logs"
+            failedUpgrades = []
+        } else if count == 0 {
             statusMenuItem.title = "✅ All up to date"
         } else {
             statusMenuItem.title = "⚠️ \(count) outdated"
@@ -564,6 +587,7 @@ class AppDelegate: NSObject, NSApplicationDelegate {
 
         statusMenuItem.title = "Upgrading \(name)..."
         setMenuBarIcon("brewbar-updating")
+        failedUpgrades = []
 
         let finish = {
             self.cleanupAfterUpgradeIfEnabled {
@@ -576,7 +600,12 @@ class AppDelegate: NSObject, NSApplicationDelegate {
         if cachedOutdated.casks.contains(where: { $0.name == name }) {
             upgradeCask(name, completion: finish)
         } else {
-            runBrew("upgrade \(name)", askpassMessage: "upgrade \(name)") { _, _ in finish() }
+            runBrew("upgrade \(name)", askpassMessage: "upgrade \(name)") { _, success in
+                if !success {
+                    self.failedUpgrades.append(name)
+                }
+                finish()
+            }
         }
     }