From 21a926232e22502adc6b99f201ad005fc2dd8e20 Mon Sep 17 00:00:00 2001 From: John Knox Date: Tue, 9 Jun 2020 06:53:42 -0700 Subject: [PATCH] Add idb check to doctor Summary: Adds a check for idb to flipper-doctor. This depends on the flipper settings, to know where to look for idb, so I've made it so you can pass settings into the doctor when running it. When run from the command line, you don't get the settings. This is because settings loading currently depends on redux so I haven't extracted it into its own package. Not that this will notify ALL open source iOS users with a doctor warning because they don't have idb installed. The error message says you can disable it in settings, which will silence this warning. CHANGELOG: The open source version now works with physical iOS devices. Reviewed By: passy Differential Revision: D21883086 fbshipit-source-id: f28c43487e88a6c07ef3cc3da2764026726c9f18 --- desktop/app/src/chrome/DoctorBar.tsx | 8 +--- desktop/app/src/chrome/DoctorSheet.tsx | 8 +--- desktop/app/src/chrome/SettingsSheet.tsx | 28 +++++++++--- .../app/src/chrome/settings/configFields.tsx | 2 + desktop/app/src/dispatcher/iOSDevice.tsx | 4 +- desktop/app/src/reducers/settings.tsx | 2 + desktop/app/src/utils/runHealthchecks.tsx | 14 +++--- desktop/doctor/src/index.ts | 45 ++++++++++++++++++- 8 files changed, 85 insertions(+), 26 deletions(-) diff --git a/desktop/app/src/chrome/DoctorBar.tsx b/desktop/app/src/chrome/DoctorBar.tsx index bc7dbcdb6..779fb665b 100644 --- a/desktop/app/src/chrome/DoctorBar.tsx +++ b/desktop/app/src/chrome/DoctorBar.tsx @@ -141,13 +141,9 @@ class DoctorBar extends Component { } export default connect( - ({ - settingsState: {enableAndroid, enableIOS}, - healthchecks: {healthcheckReport}, - }) => ({ - enableAndroid, - enableIOS, + ({settingsState, healthchecks: {healthcheckReport}}) => ({ healthcheckReport, + settings: settingsState, }), { setActiveSheet, diff --git a/desktop/app/src/chrome/DoctorSheet.tsx b/desktop/app/src/chrome/DoctorSheet.tsx index e898a4a85..e1d035c6c 100644 --- a/desktop/app/src/chrome/DoctorSheet.tsx +++ b/desktop/app/src/chrome/DoctorSheet.tsx @@ -406,13 +406,9 @@ class DoctorSheet extends Component { } export default connect( - ({ - healthchecks: {healthcheckReport}, - settingsState: {enableAndroid, enableIOS}, - }) => ({ + ({healthchecks: {healthcheckReport}, settingsState}) => ({ healthcheckReport, - enableAndroid, - enableIOS, + settings: settingsState, }), { startHealthchecks, diff --git a/desktop/app/src/chrome/SettingsSheet.tsx b/desktop/app/src/chrome/SettingsSheet.tsx index e58471af7..08df6622d 100644 --- a/desktop/app/src/chrome/SettingsSheet.tsx +++ b/desktop/app/src/chrome/SettingsSheet.tsx @@ -85,6 +85,7 @@ class SettingsSheet extends Component { enableAndroid, androidHome, enableIOS, + enablePhysicalIOS, enablePrefetching, idbPath, reactNative, @@ -138,16 +139,29 @@ class SettingsSheet extends Component { content={'iOS development is only supported on MacOS'} /> )} - { this.setState({ - updatedSettings: {...this.state.updatedSettings, idbPath: v}, + updatedSettings: { + ...this.state.updatedSettings, + enablePhysicalIOS: v, + }, }); - }} - /> + }}> + { + this.setState({ + updatedSettings: {...this.state.updatedSettings, idbPath: v}, + }); + }} + /> + { isXcodeDetected().then((isDetected) => { store.dispatch(setXcodeDetected(isDetected)); if (isDetected) { - startDevicePortForwarders(); + if (store.getState().settingsState.enablePhysicalIOS) { + startDevicePortForwarders(); + } return queryDevicesForever(store, logger); } }); diff --git a/desktop/app/src/reducers/settings.tsx b/desktop/app/src/reducers/settings.tsx index 8966193ee..516ad71e2 100644 --- a/desktop/app/src/reducers/settings.tsx +++ b/desktop/app/src/reducers/settings.tsx @@ -21,6 +21,7 @@ export type Settings = { androidHome: string; enableAndroid: boolean; enableIOS: boolean; + enablePhysicalIOS: boolean; /** * If unset, this will assume the value of the GK setting. * Note that this setting has no effect in the open source version @@ -57,6 +58,7 @@ const initialState: Settings = { androidHome: getDefaultAndroidSdkPath(), enableAndroid: true, enableIOS: os.platform() === 'darwin', + enablePhysicalIOS: os.platform() === 'darwin', enablePrefetching: Tristate.Unset, idbPath: '/usr/local/bin/idb', jsApps: { diff --git a/desktop/app/src/utils/runHealthchecks.tsx b/desktop/app/src/utils/runHealthchecks.tsx index 6c51350c6..0d49044c3 100644 --- a/desktop/app/src/utils/runHealthchecks.tsx +++ b/desktop/app/src/utils/runHealthchecks.tsx @@ -25,15 +25,19 @@ export type HealthcheckEventsHandler = { }; export type HealthcheckSettings = { - enableAndroid: boolean; - enableIOS: boolean; + settings: { + enableAndroid: boolean; + enableIOS: boolean; + enablePhysicalIOS: boolean; + idbPath: string; + }; }; export type HealthcheckOptions = HealthcheckEventsHandler & HealthcheckSettings; async function launchHealthchecks(options: HealthcheckOptions): Promise { const healthchecks = getHealthchecks(); - if (!options.enableAndroid) { + if (!options.settings.enableAndroid) { healthchecks.android = { label: healthchecks.android.label, isSkipped: true, @@ -41,7 +45,7 @@ async function launchHealthchecks(options: HealthcheckOptions): Promise { 'Healthcheck is skipped, because "Android Development" option is disabled in the Flipper settings', }; } - if (!options.enableIOS) { + if (!options.settings.enableIOS) { healthchecks.ios = { label: healthchecks.ios.label, isSkipped: true, @@ -57,7 +61,7 @@ async function launchHealthchecks(options: HealthcheckOptions): Promise { continue; } for (const h of category.healthchecks) { - const checkResult = await h.run(environmentInfo); + const checkResult = await h.run(environmentInfo, options.settings); const metricName = `doctor:${h.key.replace('.', ':')}.healthcheck`; // e.g. "doctor:ios:xcode-select.healthcheck" if (checkResult.hasProblem) { hasProblems = true; diff --git a/desktop/doctor/src/index.ts b/desktop/doctor/src/index.ts index d98be88e5..cf132cb59 100644 --- a/desktop/doctor/src/index.ts +++ b/desktop/doctor/src/index.ts @@ -34,11 +34,19 @@ export type Healthchecks = { ios: HealthcheckCategory | SkippedHealthcheckCategory; }; +export type Settings = { + idbPath: string; + enablePhysicalIOS: boolean; +}; + export type Healthcheck = { key: string; label: string; isRequired?: boolean; - run: (env: EnvironmentInfo) => Promise; + run: ( + env: EnvironmentInfo, + settings?: Settings, + ) => Promise; }; export type HealthchecRunResult = { @@ -204,6 +212,41 @@ export function getHealthchecks(): Healthchecks { }; }, }, + { + key: 'ios.idb', + label: 'IDB installed', + isRequired: false, + run: async ( + _: EnvironmentInfo, + settings?: {enablePhysicalIOS: boolean; idbPath: string}, + ) => { + if (!settings) { + return { + hasProblem: false, + message: + 'Not enough context to check IDB installation. Needs to be run through Flipper UI.', + }; + } + if (!settings.enablePhysicalIOS) { + return { + hasProblem: false, + message: + 'Using physical iOS devices is disabled in settings. So IDB is not required.', + }; + } + const result = await tryExecuteCommand( + `${settings?.idbPath} --help`, + ); + const hasProblem = result.hasProblem; + const message = hasProblem + ? `IDB is required to use Flipper with iOS devices. It can be installed from https://github.com/facebook/idb and configured in Flipper settings. You can also disable physical iOS device support in settings. Current setting: ${settings.idbPath} isn't a valid IDB installation.` + : 'Flipper is configured to use your IDB installation.'; + return { + hasProblem, + message, + }; + }, + }, ], } : {