Merge pull request #84 from RobotsAndPencils/prepare-user-for-helper-installation

Prepare user for helper installation before attempting to install
This commit is contained in:
Brandon Evans 2021-01-25 18:33:48 -07:00 committed by GitHub
commit 9a2057bcd5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 203 additions and 37 deletions

View file

@ -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 = "<group>"; };
CAC281E1259FA44600B8AB0B /* Bundle+XcodesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Bundle+XcodesTests.swift"; sourceTree = "<group>"; };
CAC281E6259FA45A00B8AB0B /* Environment+Mock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Environment+Mock.swift"; sourceTree = "<group>"; };
CAC9F92C25BCDA4400B4965F /* HelperInstallState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HelperInstallState.swift; sourceTree = "<group>"; };
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 = "<group>"; };
CAD2E7A32449574E00113D76 /* XcodeListView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodeListView.swift; sourceTree = "<group>"; };
@ -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 */,

View file

@ -175,18 +175,19 @@ extension AppState {
.flatMap { installedXcode -> AnyPublisher<InstalledXcode, Error> in
self.setInstallationStep(of: availableXcode.version, to: .finishing)
return self.enableDeveloperMode()
.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)
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<Void, Error> {
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<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> {
installHelperIfNecessary()
.flatMap {
Current.helper.acceptXcodeLicense(xcode.path.string)
}
private func approveLicense(for xcode: InstalledXcode) -> AnyPublisher<Void, Error> {
Current.helper.acceptXcodeLicense(xcode.path.string)
.eraseToAnyPublisher()
}
func installComponents(for xcode: InstalledXcode) -> AnyPublisher<Void, Swift.Error> {
installHelperIfNecessary()
.flatMap {
Current.helper.runFirstLaunch(xcode.path.string)
}
private func installComponents(for xcode: InstalledXcode) -> AnyPublisher<Void, Swift.Error> {
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)."
}
}
}
}

View file

@ -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<AnyCancellable>()
var cancellables = Set<AnyCancellable>()
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

View file

@ -0,0 +1,7 @@
import Foundation
public enum HelperInstallState: Equatable {
case unknown
case notInstalled
case installed
}

View file

@ -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 }))

View file

@ -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)

View file

@ -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
}
}
}

View file

@ -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(