From cb507c3d02b321c1d2f8e2ad71e447dab8508f4c Mon Sep 17 00:00:00 2001 From: Brandon Evans Date: Sat, 23 Jan 2021 19:25:56 -0700 Subject: [PATCH] Prepare user for helper installation before post-install steps --- Xcodes/Backend/AppState+Install.swift | 127 ++++++++++++++---- Xcodes/Backend/AppState.swift | 20 +-- Xcodes/Frontend/MainWindow.swift | 24 +++- .../Frontend/XcodeList/XcodeListViewRow.swift | 9 +- XcodesTests/AppStateTests.swift | 4 + 5 files changed, 146 insertions(+), 38 deletions(-) diff --git a/Xcodes/Backend/AppState+Install.swift b/Xcodes/Backend/AppState+Install.swift index 8f41d67..fad9ad5 100644 --- a/Xcodes/Backend/AppState+Install.swift +++ b/Xcodes/Backend/AppState+Install.swift @@ -175,18 +175,19 @@ extension AppState { .flatMap { installedXcode -> AnyPublisher in self.setInstallationStep(of: availableXcode.version, to: .finishing) - return self.enableDeveloperMode() - .map { installedXcode } - .eraseToAnyPublisher() - } - .flatMap { installedXcode -> AnyPublisher in - self.approveLicense(for: installedXcode) - .map { installedXcode } - .eraseToAnyPublisher() - } - .flatMap { installedXcode -> AnyPublisher in - self.installComponents(for: installedXcode) + return self.performPostInstallSteps(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 { - installHelperIfNecessary() - .flatMap { - Current.helper.devToolsSecurityEnable() + + // MARK: - Post-Install + + /// Attemps to install the helper once, then performs all post-install steps + 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 { + let postInstallPublisher: AnyPublisher = + 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() + + // 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 { + Current.helper.devToolsSecurityEnable() .flatMap { Current.helper.addStaffToDevelopersGroup() } .eraseToAnyPublisher() } - func approveLicense(for xcode: InstalledXcode) -> AnyPublisher { - installHelperIfNecessary() - .flatMap { - Current.helper.acceptXcodeLicense(xcode.path.string) - } + private func approveLicense(for xcode: InstalledXcode) -> AnyPublisher { + Current.helper.acceptXcodeLicense(xcode.path.string) .eraseToAnyPublisher() } - func installComponents(for xcode: InstalledXcode) -> AnyPublisher { - installHelperIfNecessary() - .flatMap { - Current.helper.runFirstLaunch(xcode.path.string) - } + private func installComponents(for xcode: InstalledXcode) -> AnyPublisher { + 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)." + } } } } diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index cedd539..087bb10 100644 --- a/Xcodes/Backend/AppState.swift +++ b/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. - @Published var isPreparingUserForActionRequiringHelper: (() -> Void)? + /// This closure will be performed after the user chooses whether or not to proceed. + @Published var isPreparingUserForActionRequiringHelper: ((Bool) -> Void)? // MARK: - Errors @@ -51,7 +51,7 @@ class AppState: ObservableObject { // MARK: - Publisher Cancellables - private var cancellables = Set() + var cancellables = Set() 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 diff --git a/Xcodes/Frontend/MainWindow.swift b/Xcodes/Frontend/MainWindow.swift index a430376..890919a 100644 --- a/Xcodes/Frontend/MainWindow.swift +++ b/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) + } + } ) } ) diff --git a/Xcodes/Frontend/XcodeList/XcodeListViewRow.swift b/Xcodes/Frontend/XcodeList/XcodeListViewRow.swift index 97c3c3c..1d7f099 100644 --- a/Xcodes/Frontend/XcodeList/XcodeListViewRow.swift +++ b/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 } } } diff --git a/XcodesTests/AppStateTests.swift b/XcodesTests/AppStateTests.swift index 11fef6a..c9fffb2 100644 --- a/XcodesTests/AppStateTests.swift +++ b/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(