diff --git a/Xcodes.xcodeproj/project.pbxproj b/Xcodes.xcodeproj/project.pbxproj index 7d23bd9..0fc2613 100644 --- a/Xcodes.xcodeproj/project.pbxproj +++ b/Xcodes.xcodeproj/project.pbxproj @@ -80,6 +80,7 @@ CAC281DA259F985100B8AB0B /* InstallationStep.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAC281D9259F985100B8AB0B /* InstallationStep.swift */; }; CAC281E2259FA44600B8AB0B /* Bundle+XcodesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAC281E1259FA44600B8AB0B /* Bundle+XcodesTests.swift */; }; CAC281E7259FA45A00B8AB0B /* Environment+Mock.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAC281E6259FA45A00B8AB0B /* Environment+Mock.swift */; }; + CAC9F92D25BCDA4400B4965F /* HelperInstallState.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAC9F92C25BCDA4400B4965F /* HelperInstallState.swift */; }; CAD2E7A22449574E00113D76 /* XcodesApp.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAD2E7A12449574E00113D76 /* XcodesApp.swift */; }; CAD2E7A42449574E00113D76 /* XcodeListView.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAD2E7A32449574E00113D76 /* XcodeListView.swift */; }; CAD2E7A62449575000113D76 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = CAD2E7A52449575000113D76 /* Assets.xcassets */; }; @@ -231,6 +232,7 @@ CAC281D9259F985100B8AB0B /* InstallationStep.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InstallationStep.swift; sourceTree = ""; }; CAC281E1259FA44600B8AB0B /* Bundle+XcodesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Bundle+XcodesTests.swift"; sourceTree = ""; }; CAC281E6259FA45A00B8AB0B /* Environment+Mock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Environment+Mock.swift"; sourceTree = ""; }; + CAC9F92C25BCDA4400B4965F /* HelperInstallState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HelperInstallState.swift; sourceTree = ""; }; CAD2E79E2449574E00113D76 /* Xcodes.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = Xcodes.app; sourceTree = BUILT_PRODUCTS_DIR; }; CAD2E7A12449574E00113D76 /* XcodesApp.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodesApp.swift; sourceTree = ""; }; CAD2E7A32449574E00113D76 /* XcodeListView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodeListView.swift; sourceTree = ""; }; @@ -417,6 +419,7 @@ CAFBDB942598FE96003DCC5A /* FocusedValues.swift */, CABFA9AC2592EEE900380FEE /* Foundation.swift */, CA9FF9352595B44700E47BAF /* HelperClient.swift */, + CAC9F92C25BCDA4400B4965F /* HelperInstallState.swift */, CAC281D9259F985100B8AB0B /* InstallationStep.swift */, CA9FF8862595607900E47BAF /* InstalledXcode.swift */, CAA8587B25A2B37900ACF8C0 /* IsTesting.swift */, @@ -779,6 +782,7 @@ CABFAA2D2592FBFC00380FEE /* Configure.swift in Sources */, CA73510D257BFCEF00EA9CF8 /* NSAttributedString+.swift in Sources */, CAFBDB952598FE96003DCC5A /* FocusedValues.swift in Sources */, + CAC9F92D25BCDA4400B4965F /* HelperInstallState.swift in Sources */, CAC281CD259F97FA00B8AB0B /* ObservingProgressIndicator.swift in Sources */, CABFA9C22592EEEA00380FEE /* Publisher+Resumable.swift in Sources */, CAFBDC68259A308B003DCC5A /* InfoPane.swift in Sources */, 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 0f34ca8..087bb10 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -40,6 +40,9 @@ class AppState: ObservableObject { @Published var xcodeBeingConfirmedForUninstallation: Xcode? @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 chooses whether or not to proceed. + @Published var isPreparingUserForActionRequiringHelper: ((Bool) -> Void)? // MARK: - Errors @@ -48,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? @@ -203,7 +206,25 @@ class AppState: ObservableObject { // MARK: - Helper - func installHelperIfNecessary() { + /// Install the privileged helper if it isn't already installed. + /// + /// The way this is done is a little roundabout, because it requires user interaction in an alert before installation should be attempted. + /// The first time this method is invoked should be with `shouldPrepareUserForHelperInstallation` set to true. + /// If the helper is already installed, then nothing will happen. + /// If the helper is not already installed, the user will be prepared for installation and this method will return early. + /// If they consent to installing the helper then this method will be invoked again with `shouldPrepareUserForHelperInstallation` set to false. + /// This will install the helper. + /// + /// - 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] userConsented in + guard userConsented else { return } + self.installHelperIfNecessary(shouldPrepareUserForHelperInstallation: false) + } + return + } + installHelperIfNecessary() .sink( receiveCompletion: { [unowned self] completion in @@ -339,7 +360,27 @@ class AppState: ObservableObject { NSWorkspace.shared.activateFileViewerSelecting([installedXcode.path.url]) } - func select(id: Xcode.ID) { + /// Make an Xcode active, a.k.a select it, in the `xcode-select` sense. + /// + /// The underlying work is done by the privileged helper, so we need to make sure that it's installed first. + /// The way this is done is a little roundabout, because it requires user interaction in an alert before the `selectPublisher` is subscribed to. + /// The first time this method is invoked should be with `shouldPrepareUserForHelperInstallation` set to true. + /// If the helper is already installed, the Xcode will be made active immediately. + /// If the helper is not already installed, the user will be prepared for installation and this method will return early. + /// If they consent to installing the helper then this method will be invoked again with `shouldPrepareUserForHelperInstallation` set to false. + /// This will install the helper and make the Xcode active. + /// + /// - Parameter id: The identifier of the Xcode to make active. + /// - 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] userConsented in + guard userConsented else { return } + self.select(id: id, shouldPrepareUserForHelperInstallation: false) + } + return + } + guard let installedXcode = Current.files.installedXcodes(Path.root/"Applications").first(where: { $0.version == id }), selectPublisher == nil @@ -457,12 +498,6 @@ class AppState: ObservableObject { // MARK: - Nested Types - enum HelperInstallState: Equatable { - case unknown - case notInstalled - case installed - } - struct AlertContent: Identifiable { var title: String var message: String diff --git a/Xcodes/Backend/HelperInstallState.swift b/Xcodes/Backend/HelperInstallState.swift new file mode 100644 index 0000000..922ab07 --- /dev/null +++ b/Xcodes/Backend/HelperInstallState.swift @@ -0,0 +1,7 @@ +import Foundation + +public enum HelperInstallState: Equatable { + case unknown + case notInstalled + case installed +} diff --git a/Xcodes/Frontend/MainWindow.swift b/Xcodes/Frontend/MainWindow.swift index 006740c..890919a 100644 --- a/Xcodes/Frontend/MainWindow.swift +++ b/Xcodes/Frontend/MainWindow.swift @@ -50,6 +50,40 @@ struct MainWindow: View { secondFactorView(appState.secondFactorData!) .environmentObject(appState) } + // This overlay is only here to work around the one-alert-per-view limitation + .overlay( + Color.clear + .alert(isPresented: $appState.isPreparingUserForActionRequiringHelper.isNotNil) { + Alert( + 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: { + // 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 { + // 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) + } + } + ) + } + ) // I'm expecting to be able to use this modifier on a List row, but using it at the top level here is the only way that has made XcodeCommands work so far. // FB8954571 focusedValue(_:_:) on List row doesn't propagate value to @FocusedValue .focusedValue(\.selectedXcode, SelectedXcode(appState.allXcodes.first { $0.id == selectedXcodeID })) diff --git a/Xcodes/Frontend/Preferences/AdvancedPreferencePane.swift b/Xcodes/Frontend/Preferences/AdvancedPreferencePane.swift index 00c03e9..42a162e 100644 --- a/Xcodes/Frontend/Preferences/AdvancedPreferencePane.swift +++ b/Xcodes/Frontend/Preferences/AdvancedPreferencePane.swift @@ -60,7 +60,7 @@ struct AdvancedPreferencePane: View { } } - 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.") + 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.") .font(.footnote) .fixedSize(horizontal: false, vertical: true) 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(