From e21e4d9cdf5f27331ebde150100d37e6944b28b0 Mon Sep 17 00:00:00 2001 From: Brandon Evans Date: Fri, 22 Jan 2021 22:18:59 -0700 Subject: [PATCH 1/2] Handle helper installation errors --- Xcodes/Backend/AppState+Install.swift | 27 +++++++------- Xcodes/Backend/AppState.swift | 35 ++++++++++++++----- Xcodes/Backend/Environment.swift | 2 +- Xcodes/Backend/HelperClient.swift | 4 ++- .../Preferences/AdvancedPreferencePane.swift | 2 +- 5 files changed, 44 insertions(+), 26 deletions(-) diff --git a/Xcodes/Backend/AppState+Install.swift b/Xcodes/Backend/AppState+Install.swift index 2d07bb2..3840fb2 100644 --- a/Xcodes/Backend/AppState+Install.swift +++ b/Xcodes/Backend/AppState+Install.swift @@ -303,11 +303,10 @@ extension AppState { } func enableDeveloperMode() -> AnyPublisher { - if helperInstallState == .notInstalled { - installHelper() - } - - return Current.helper.devToolsSecurityEnable() + installHelperIfNecessary() + .flatMap { + Current.helper.devToolsSecurityEnable() + } .flatMap { Current.helper.addStaffToDevelopersGroup() } @@ -315,20 +314,18 @@ extension AppState { } func approveLicense(for xcode: InstalledXcode) -> AnyPublisher { - if helperInstallState == .notInstalled { - installHelper() - } - - return Current.helper.acceptXcodeLicense(xcode.path.string) + installHelperIfNecessary() + .flatMap { + Current.helper.acceptXcodeLicense(xcode.path.string) + } .eraseToAnyPublisher() } func installComponents(for xcode: InstalledXcode) -> AnyPublisher { - if helperInstallState == .notInstalled { - installHelper() - } - - return Current.helper.runFirstLaunch(xcode.path.string) + installHelperIfNecessary() + .flatMap { + Current.helper.runFirstLaunch(xcode.path.string) + } .flatMap { Current.shell.getUserCacheDir().map { $0.out } .combineLatest( diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index d676bc3..0f34ca8 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -203,9 +203,29 @@ class AppState: ObservableObject { // MARK: - Helper - func installHelper() { - Current.helper.install() - checkIfHelperIsInstalled() + func installHelperIfNecessary() { + installHelperIfNecessary() + .sink( + receiveCompletion: { [unowned self] completion in + if case let .failure(error) = completion { + self.error = error + } + }, + receiveValue: {} + ) + .store(in: &cancellables) + } + + func installHelperIfNecessary() -> AnyPublisher { + Result { + if helperInstallState == .notInstalled { + try Current.helper.install() + checkIfHelperIsInstalled() + } + } + .publisher + .subscribe(on: DispatchQueue.main) + .eraseToAnyPublisher() } private func checkIfHelperIsInstalled() { @@ -320,16 +340,15 @@ class AppState: ObservableObject { } func select(id: Xcode.ID) { - if helperInstallState == .notInstalled { - installHelper() - } - guard let installedXcode = Current.files.installedXcodes(Path.root/"Applications").first(where: { $0.version == id }), selectPublisher == nil else { return } - selectPublisher = HelperClient().switchXcodePath(installedXcode.path.string) + selectPublisher = installHelperIfNecessary() + .flatMap { + Current.helper.switchXcodePath(installedXcode.path.string) + } .flatMap { [unowned self] _ in self.updateSelectedXcodePath() } diff --git a/Xcodes/Backend/Environment.swift b/Xcodes/Backend/Environment.swift index 16c2826..dea687b 100644 --- a/Xcodes/Backend/Environment.swift +++ b/Xcodes/Backend/Environment.swift @@ -237,7 +237,7 @@ public struct Defaults { private let helperClient = HelperClient() public struct Helper { - var install: () -> Void = helperClient.install + var install: () throws -> Void = helperClient.install var checkIfLatestHelperIsInstalled: () -> AnyPublisher = helperClient.checkIfLatestHelperIsInstalled var getVersion: () -> AnyPublisher = helperClient.getVersion var switchXcodePath: (_ absolutePath: String) -> AnyPublisher = helperClient.switchXcodePath diff --git a/Xcodes/Backend/HelperClient.swift b/Xcodes/Backend/HelperClient.swift index ae72a3e..75b4ef3 100644 --- a/Xcodes/Backend/HelperClient.swift +++ b/Xcodes/Backend/HelperClient.swift @@ -307,7 +307,7 @@ final class HelperClient { // MARK: - Install // From https://github.com/securing/SimpleXPCApp/ - func install() { + func install() throws { Logger.helperClient.info(#function) var authItem = kSMRightBlessPrivilegedHelper.withCString { name in @@ -329,6 +329,8 @@ final class HelperClient { Logger.helperClient.info("\(#function): Finished installation") } catch { Logger.helperClient.error("\(#function): \(error.localizedDescription)") + + throw error } } diff --git a/Xcodes/Frontend/Preferences/AdvancedPreferencePane.swift b/Xcodes/Frontend/Preferences/AdvancedPreferencePane.swift index 4dc028d..00c03e9 100644 --- a/Xcodes/Frontend/Preferences/AdvancedPreferencePane.swift +++ b/Xcodes/Frontend/Preferences/AdvancedPreferencePane.swift @@ -55,7 +55,7 @@ struct AdvancedPreferencePane: View { HStack { Text("Helper is not installed") Button("Install helper") { - appState.installHelper() + appState.installHelperIfNecessary() } } } From 6b5c288a407e8b8f60320141924d182f2676c917 Mon Sep 17 00:00:00 2001 From: Brandon Evans Date: Fri, 22 Jan 2021 22:19:15 -0700 Subject: [PATCH 2/2] Unwrap auth error messages to avoid "Optional(...)" --- Xcodes/Backend/HelperClient.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Xcodes/Backend/HelperClient.swift b/Xcodes/Backend/HelperClient.swift index 75b4ef3..e917900 100644 --- a/Xcodes/Backend/HelperClient.swift +++ b/Xcodes/Backend/HelperClient.swift @@ -337,7 +337,11 @@ final class HelperClient { private func executeAuthorizationFunction(_ authorizationFunction: () -> (OSStatus) ) throws { let osStatus = authorizationFunction() guard osStatus == errAuthorizationSuccess else { - throw HelperClientError.message(String(describing: SecCopyErrorMessageString(osStatus, nil))) + if let message = SecCopyErrorMessageString(osStatus, nil) { + throw HelperClientError.message(String(message as NSString)) + } else { + throw HelperClientError.message("Unknown error") + } } }