Prepare user for helper installation before post-install steps

Xcodes/Backend/AppState+Install.swift

@@ -175,18 +175,19 @@ extension AppState {
                     .flatMap { installedXcode -> AnyPublisher<InstalledXcode, Error> in
                         self.setInstallationStep(of: availableXcode.version, to: .finishing)
 
-                        return self.enableDeveloperMode()
+                        return self.performPostInstallSteps(for: installedXcode)
-                            .map { installedXcode }
-                            .eraseToAnyPublisher()
-                    }
-                    .flatMap { installedXcode -> AnyPublisher<InstalledXcode, Error> in
-                        self.approveLicense(for: installedXcode)
-                            .map { installedXcode }
-                            .eraseToAnyPublisher()
-                    }
-                    .flatMap { installedXcode -> AnyPublisher<InstalledXcode, Error> in
-                        self.installComponents(for: installedXcode)
                             .map { installedXcode }
+                            // Show post-install errors but don't fail because of them
+                            .handleEvents(receiveCompletion: { [unowned self] completion in
+                                if case let .failure(error) = completion {
+                                    self.error = error
+                                }
+                            })
+                            .catch { _ in
+                                Just(installedXcode)
+                                    .setFailureType(to: Error.self)
+                                    .eraseToAnyPublisher()
+                            }
                             .eraseToAnyPublisher()
                     }
                     .eraseToAnyPublisher()
@@ -306,31 +307,95 @@ extension AppState {
 
         return info
     }
-
+    
-    func enableDeveloperMode() -> AnyPublisher<Void, Error> {
+    // MARK: - Post-Install
-        installHelperIfNecessary()
+    
-            .flatMap {
+    /// Attemps to install the helper once, then performs all post-install steps
-                Current.helper.devToolsSecurityEnable()
+    public func performPostInstallSteps(for xcode: InstalledXcode) {
+        performPostInstallSteps(for: xcode)
+            .sink(
+                receiveCompletion: { completion in
+                    if case let .failure(error) = completion {
+                        self.error = error
+                    }
+                }, 
+                receiveValue: {}
+            )
+            .store(in: &cancellables)
+    }
+    
+    /// Attemps to install the helper once, then performs all post-install steps
+    public func performPostInstallSteps(for xcode: InstalledXcode) -> AnyPublisher<Void, Error> {
+        let postInstallPublisher: AnyPublisher<Void, Error> =
+            Deferred { [unowned self] in
+                self.installHelperIfNecessary()
             }
+            .flatMap { [unowned self] in
+                self.enableDeveloperMode()
+            }
+            .flatMap { [unowned self] in
+                self.approveLicense(for: xcode)
+            }
+            .flatMap { [unowned self] in
+                self.installComponents(for: xcode)
+            }
+            .mapError { [unowned self] error in
+                Logger.appState.error("Performing post-install steps failed: \(error.legibleLocalizedDescription)")
+                return InstallationError.postInstallStepsNotPerformed(version: xcode.version, helperInstallState: self.helperInstallState)
+            }
+            .eraseToAnyPublisher()
+
+        guard helperInstallState == .installed else {
+            // If the helper isn't installed yet then we need to prepare the user for the install prompt,
+            // and then actually perform the installation,
+            // and the post-install steps need to wait until that is complete.
+            // This subject, which completes upon isPreparingUserForActionRequiringHelper being invoked, is used to achieve that.
+            // This is not the most straightforward code I've ever written...
+            let helperInstallConsentSubject = PassthroughSubject<Void, Error>()
+
+            // Need to dispatch this to avoid duplicate alerts, 
+            // the second of which will crash when force-unwrapping isPreparingUserForActionRequiringHelper 
+            DispatchQueue.main.async {
+                self.isPreparingUserForActionRequiringHelper = { [unowned self] userConsented in
+                    if userConsented {
+                        helperInstallConsentSubject.send()
+                    } else {
+                        Logger.appState.info("User did not consent to installing helper during post-install steps.")
+
+                        helperInstallConsentSubject.send(
+                            completion: .failure(
+                                InstallationError.postInstallStepsNotPerformed(version: xcode.version, helperInstallState: self.helperInstallState)
+                            )
+                        )
+                    }
+                }
+            }
+
+            return helperInstallConsentSubject
+                .flatMap { 
+                    postInstallPublisher 
+                }
+                .eraseToAnyPublisher()
+        }
+        
+        return postInstallPublisher
+    }
+
+    private func enableDeveloperMode() -> AnyPublisher<Void, Error> {
+        Current.helper.devToolsSecurityEnable()
             .flatMap {
                 Current.helper.addStaffToDevelopersGroup()
             }
             .eraseToAnyPublisher()
     }
 
-    func approveLicense(for xcode: InstalledXcode) -> AnyPublisher<Void, Error> {
+    private func approveLicense(for xcode: InstalledXcode) -> AnyPublisher<Void, Error> {
-        installHelperIfNecessary()
+        Current.helper.acceptXcodeLicense(xcode.path.string)
-            .flatMap {
-                Current.helper.acceptXcodeLicense(xcode.path.string)
-            }
             .eraseToAnyPublisher()
     }
 
-    func installComponents(for xcode: InstalledXcode) -> AnyPublisher<Void, Swift.Error> {
+    private func installComponents(for xcode: InstalledXcode) -> AnyPublisher<Void, Swift.Error> {
-        installHelperIfNecessary()
+        Current.helper.runFirstLaunch(xcode.path.string)
-            .flatMap {
-                Current.helper.runFirstLaunch(xcode.path.string)
-            }
             .flatMap {
                 Current.shell.getUserCacheDir().map { $0.out }
                     .combineLatest(
@@ -345,6 +410,8 @@ extension AppState {
             .eraseToAnyPublisher()
     }
     
+    // MARK: - 
+    
     func setInstallationStep(of version: Version, to step: InstallationStep) {
         DispatchQueue.main.async {
             guard let index = self.allXcodes.firstIndex(where: { $0.version.isEquivalent(to: version) }) else { return }
@@ -381,6 +448,7 @@ public enum InstallationError: LocalizedError, Equatable {
     case versionAlreadyInstalled(InstalledXcode)
     case invalidVersion(String)
     case versionNotInstalled(Version)
+    case postInstallStepsNotPerformed(version: Version, helperInstallState: HelperInstallState)
 
     public var errorDescription: String? {
         switch self {
@@ -433,6 +501,13 @@ public enum InstallationError: LocalizedError, Equatable {
             return "\(version) is not a valid version number."
         case let .versionNotInstalled(version):
             return "\(version.appleDescription) is not installed."
+        case let .postInstallStepsNotPerformed(version, helperInstallState):
+            switch helperInstallState {
+            case .installed:
+                return "Installation was completed, but some post-install steps weren't performed automatically. These will be performed when you first launch Xcode \(version.appleDescription)."
+            case .notInstalled, .unknown:
+                return "Installation was completed, but some post-install steps weren't performed automatically. Xcodes performs these steps with a privileged helper, which appears to not be installed. You can install it from Preferences > Advanced.\n\nThese steps will be performed when you first launch Xcode \(version.appleDescription)."
+            }
         }
     }
 }

Xcodes/Backend/AppState.swift

@@ -41,8 +41,8 @@ class AppState: ObservableObject {
     @Published var xcodeBeingConfirmedForInstallCancellation: Xcode?
     @Published var helperInstallState: HelperInstallState = .notInstalled
     /// Whether the user is being prepared for the helper installation alert with an explanation.
-    /// This closure will be performed after the user consents.
+    /// This closure will be performed after the user chooses whether or not to proceed.
-    @Published var isPreparingUserForActionRequiringHelper: (() -> Void)?
+    @Published var isPreparingUserForActionRequiringHelper: ((Bool) -> Void)?
 
     // MARK: - Errors
 
@@ -51,7 +51,7 @@ class AppState: ObservableObject {
     
     // MARK: - Publisher Cancellables
     
-    private var cancellables = Set<AnyCancellable>()
+    var cancellables = Set<AnyCancellable>()
     private var installationPublishers: [Version: AnyCancellable] = [:]
     private var selectPublisher: AnyCancellable?
     private var uninstallPublisher: AnyCancellable?
@@ -218,12 +218,13 @@ class AppState: ObservableObject {
     /// - Parameter shouldPrepareUserForHelperInstallation: Whether the user should be presented with an alert preparing them for helper installation.
     func installHelperIfNecessary(shouldPrepareUserForHelperInstallation: Bool = true) {
         guard helperInstallState == .installed || shouldPrepareUserForHelperInstallation == false else {
-            isPreparingUserForActionRequiringHelper = { [unowned self] in self.installHelperIfNecessary(shouldPrepareUserForHelperInstallation: false) }
+            isPreparingUserForActionRequiringHelper = { [unowned self] userConsented in
+                guard userConsented else { return }
+                self.installHelperIfNecessary(shouldPrepareUserForHelperInstallation: false) 
+            }
             return
         }
         
-        isPreparingUserForActionRequiringHelper = nil
-        
         installHelperIfNecessary()
             .sink(
                 receiveCompletion: { [unowned self] completion in
@@ -373,12 +374,13 @@ class AppState: ObservableObject {
     /// - Parameter shouldPrepareUserForHelperInstallation: Whether the user should be presented with an alert preparing them for helper installation before making the Xcode version active.
     func select(id: Xcode.ID, shouldPrepareUserForHelperInstallation: Bool = true) {
         guard helperInstallState == .installed || shouldPrepareUserForHelperInstallation == false else {
-            isPreparingUserForActionRequiringHelper = { [unowned self] in self.select(id: id, shouldPrepareUserForHelperInstallation: false) }
+            isPreparingUserForActionRequiringHelper = { [unowned self] userConsented in
+                guard userConsented else { return }
+                self.select(id: id, shouldPrepareUserForHelperInstallation: false) 
+            }
             return
         }
 
-        isPreparingUserForActionRequiringHelper = nil
-
         guard 
             let installedXcode = Current.files.installedXcodes(Path.root/"Applications").first(where: { $0.version == id }),
             selectPublisher == nil

Xcodes/Frontend/MainWindow.swift

@@ -58,9 +58,29 @@ struct MainWindow: View {
                         title: Text("Privileged Helper"), 
                         message: Text("Xcodes uses a separate privileged helper to perform tasks as root. These are things that would require sudo on the command line, including post-install steps and switching Xcode versions with xcode-select.\n\nYou'll be prompted for your macOS account password to install it."), 
                         primaryButton: .default(Text("Install"), action: {
-                            DispatchQueue.main.async(execute: appState.isPreparingUserForActionRequiringHelper!)
+                            // The isPreparingUserForActionRequiringHelper closure is set to nil by the alert's binding when its dismissed.
+                            // We need to capture it to be invoked after that happens.
+                            let helperAction = appState.isPreparingUserForActionRequiringHelper
+                            DispatchQueue.main.async { 
+                                // This really shouldn't be nil, but sometimes this alert is being shown twice and I don't know why.
+                                // There are some DispatchQueue.main.async's scattered around which make this better but in some situations it's still happening.
+                                // When that happens, the second time the user clicks an alert button isPreparingUserForActionRequiringHelper will be nil.
+                                // To at least not crash, we're using ?
+                                helperAction?(true) 
+                            }
                         }), 
-                        secondaryButton: .cancel()
+                        secondaryButton: .cancel {
+                            // The isPreparingUserForActionRequiringHelper closure is set to nil by the alert's binding when its dismissed.
+                            // We need to capture it to be invoked after that happens.
+                            let helperAction = appState.isPreparingUserForActionRequiringHelper
+                            DispatchQueue.main.async {
+                                // This really shouldn't be nil, but sometimes this alert is being shown twice and I don't know why.
+                                // There are some DispatchQueue.main.async's scattered around which make this better but in some situations it's still happening.
+                                // When that happens, the second time the user clicks an alert button isPreparingUserForActionRequiringHelper will be nil.
+                                // To at least not crash, we're using ?
+                                helperAction?(false) 
+                            }
+                        }
                     )
                 }
         )

Xcodes/Frontend/XcodeList/XcodeListViewRow.swift

@@ -37,13 +37,20 @@ struct XcodeListViewRow: View {
                 InstallButton(xcode: xcode)
             case .installing:
                 CancelInstallButton(xcode: xcode)
-            case .installed:
+            case let .installed(path):
                 SelectButton(xcode: xcode)
                 OpenButton(xcode: xcode)
                 RevealButton(xcode: xcode)
                 CopyPathButton(xcode: xcode)
                 Divider()
                 UninstallButton(xcode: xcode)
+                
+                #if DEBUG
+                Divider()
+                Button("Perform post-install steps") {
+                    appState.performPostInstallSteps(for: InstalledXcode(path: path)!) as Void
+                }
+                #endif
             }
         }
     }

XcodesTests/AppStateTests.swift

@@ -148,6 +148,8 @@ class AppStateTests: XCTestCase {
             .setFailureType(to: Error.self)
             .eraseToAnyPublisher()
         }
+        // Helper is already installed
+        subject.helperInstallState = .installed
 
         let allXcodesRecorder = subject.$allXcodes.record()
         let installRecorder = subject.install(
@@ -256,6 +258,8 @@ class AppStateTests: XCTestCase {
             .setFailureType(to: Error.self)
             .eraseToAnyPublisher()
         }
+        // Helper is already installed
+        subject.helperInstallState = .installed
 
         let allXcodesRecorder = subject.$allXcodes.record()
         let installRecorder = subject.install(