Doctor complains Android SDK is not installed

Summary:
There are complaints about Android SDK being reported as "not installed" when it is actually installed. To address them, I changed the way how we detect SDK and also added some minimal actionable feedback for each check.

The problem with the previous implementation of Android SDK check via "envinfo" is that the library uses "sdkmanager" tool under the hood, and this tool doesn't work on Java 9+. To fix this I'm changing the way how we assume SDK is installed to simple check for "adb" tool existence.

Actionable feedback is shown on Doctor report when you click to an item.

Reviewed By: jknoxville

Differential Revision: D19517769

fbshipit-source-id: 1c21f1bdcd05c7c0ae3f97b9c3454efa2c861d26

doctor/package.json

@@ -1,6 +1,6 @@
 {
   "name": "flipper-doctor",
-  "version": "0.6.1",
+  "version": "0.7.0",
   "description": "Utility for checking for issues with a flipper installation",
   "main": "lib/index.js",
   "types": "lib/index.d.ts",

doctor/src/environmentInfo.ts

@@ -35,35 +35,14 @@ async function retrieveAndParseEnvInfo(): Promise<any> {
   return JSON.parse(
     await run(
       {
-        SDKs: ['iOS SDK', 'Android SDK'],
+        SDKs: ['iOS SDK'],
         IDEs: ['Xcode'],
-        Languages: ['Java'],
       },
       {json: true, showNotFound: true},
     ),
   );
 }
 
-// Temporary workaround for https://github.com/facebook/flipper/issues/667 until it properly fixed in 'envinfo'.
+export async function getEnvInfo(): Promise<EnvironmentInfo> {
-async function workaroundForNewerJavaVersions(envInfo: any) {
-  try {
-    if (envInfo.Languages.Java && envInfo.Languages.Java.version) {
-      const [majorVersion] = envInfo.Languages.Java.version
-        .split('.')
-        .slice(0, 1)
-        .map((x: string) => parseInt(x, 10));
-      if (8 < majorVersion && majorVersion < 11) {
-        process.env.JAVA_OPTS =
-          '-XX:+IgnoreUnrecognizedVMOptions --add-modules java.se.ee';
   return await retrieveAndParseEnvInfo();
 }
-    }
-  } catch (e) {
-    console.error(e);
-  }
-  return envInfo;
-}
-
-export async function getEnvInfo(): Promise<EnvironmentInfo> {
-  return workaroundForNewerJavaVersions(await retrieveAndParseEnvInfo());
-}

doctor/src/index.ts

@@ -10,8 +10,10 @@
 import {exec} from 'child_process';
 import {promisify} from 'util';
 import {EnvironmentInfo, getEnvInfo} from './environmentInfo';
-export {getEnvInfo} from './environmentInfo';
+export {EnvironmentInfo, getEnvInfo} from './environmentInfo';
 import * as watchman from 'fb-watchman';
+import * as fs from 'fs';
+import * as path from 'path';
 
 export type HealthcheckCategory = {
   label: string;
@@ -36,12 +38,12 @@ export type Healthcheck = {
   key: string;
   label: string;
   isRequired?: boolean;
-  run: (
+  run: (env: EnvironmentInfo) => Promise<HealthchecRunResult>;
-    env: EnvironmentInfo,
+};
-  ) => Promise<{
+
+export type HealthchecRunResult = {
   hasProblem: boolean;
-    helpUrl?: string;
+  message: string;
-  }>;
 };
 
 export type CategoryResult = [
@@ -68,9 +70,14 @@ export function getHealthchecks(): Healthchecks {
           key: 'common.openssl',
           label: 'OpenSSL Installed',
           run: async (_: EnvironmentInfo) => {
-            const isAvailable = await commandSucceeds('openssl version');
+            const result = await tryExecuteCommand('openssl version');
+            const hasProblem = result.hasProblem;
+            const message = hasProblem
+              ? `OpenSSL (https://wiki.openssl.org/index.php/Binaries) is not installed or not added to PATH. ${result.message}.`
+              : `OpenSSL (https://wiki.openssl.org/index.php/Binaries) is installed and added to PATH. ${result.message}.`;
             return {
-              hasProblem: !isAvailable,
+              hasProblem,
+              message,
             };
           },
         },
@@ -81,6 +88,9 @@ export function getHealthchecks(): Healthchecks {
             const isAvailable = await isWatchmanAvailable();
             return {
               hasProblem: !isAvailable,
+              message: isAvailable
+                ? 'Watchman file watching service (https://facebook.github.io/watchman/) is installed and added to PATH. Live reloading after changes during Flipper plugin development is enabled.'
+                : 'Watchman file watching service (https://facebook.github.io/watchman/) is not installed or not added to PATH. Live reloading after changes during Flipper plugin development is disabled.',
             };
           },
         },
@@ -95,9 +105,28 @@ export function getHealthchecks(): Healthchecks {
           key: 'android.sdk',
           label: 'SDK Installed',
           isRequired: true,
-          run: async (e: EnvironmentInfo) => ({
+          run: async (_: EnvironmentInfo) => {
-            hasProblem: e.SDKs['Android SDK'] === 'Not Found',
+            if (process.env.ANDROID_HOME) {
-          }),
+              const androidHome = process.env.ANDROID_HOME;
+              if (!fs.existsSync(androidHome)) {
+                return {
+                  hasProblem: true,
+                  message: `ANDROID_HOME points to a folder which does not exist: ${androidHome}.`,
+                };
+              }
+              const platformToolsDir = path.join(androidHome, 'platform-tools');
+              if (!fs.existsSync(path.join(androidHome, 'platform-tools'))) {
+                return {
+                  hasProblem: true,
+                  message: `Android SDK Platform Tools not found at the expected location "${platformToolsDir}". Probably they are not installed.`,
+                };
+              }
+              return await tryExecuteCommand(
+                path.join(platformToolsDir, 'adb') + ' version',
+              );
+            }
+            return await tryExecuteCommand('adb version');
+          },
         },
       ],
     },
@@ -112,40 +141,66 @@ export function getHealthchecks(): Healthchecks {
                 key: 'ios.sdk',
                 label: 'SDK Installed',
                 isRequired: true,
-                run: async (e: EnvironmentInfo) => ({
+                run: async (e: EnvironmentInfo) => {
-                  hasProblem:
+                  const hasProblem =
                     !e.SDKs['iOS SDK'] ||
                     !e.SDKs['iOS SDK'].Platforms ||
-                    !e.SDKs['iOS SDK'].Platforms.length,
+                    !e.SDKs['iOS SDK'].Platforms.length;
-                }),
+                  const message = hasProblem
+                    ? 'iOS SDK is not installed. You can install it using Xcode (https://developer.apple.com/xcode/).'
+                    : `iOS SDK is installed for the following platforms: ${JSON.stringify(
+                        e.SDKs['iOS SDK'].Platforms,
+                      )}.`;
+                  return {
+                    hasProblem,
+                    message,
+                  };
+                },
               },
               {
                 key: 'ios.xcode',
                 label: 'XCode Installed',
                 isRequired: true,
-                run: async (e: EnvironmentInfo) => ({
+                run: async (e: EnvironmentInfo) => {
-                  hasProblem: e.IDEs == null || e.IDEs.Xcode == null,
+                  const hasProblem = e.IDEs == null || e.IDEs.Xcode == null;
-                }),
+                  const message = hasProblem
+                    ? 'Xcode (https://developer.apple.com/xcode/) is not installed.'
+                    : `Xcode version ${e.IDEs.Xcode.version} is installed at "${e.IDEs.Xcode.path}".`;
+                  return {
+                    hasProblem,
+                    message,
+                  };
+                },
               },
               {
                 key: 'ios.xcode-select',
                 label: 'xcode-select set',
                 isRequired: true,
-                run: async (_: EnvironmentInfo) => ({
+                run: async (_: EnvironmentInfo) => {
-                  hasProblem: !(await commandSucceeds('xcode-select -p')),
+                  const result = await tryExecuteCommand('xcode-select -p');
-                }),
+                  const hasProblem = result.hasProblem;
+                  const message = hasProblem
+                    ? `Xcode version is not selected. You can select it using command "sudo xcode-select -switch <path/to/>Xcode.app". ${result.message}.`
+                    : `Xcode version is selected. ${result.message}.`;
+                  return {
+                    hasProblem,
+                    message,
+                  };
+                },
               },
               {
                 key: 'ios.instruments',
                 label: 'Instruments exists',
                 isRequired: true,
                 run: async (_: EnvironmentInfo) => {
-                  const hasInstruments = await commandSucceeds(
+                  const result = await tryExecuteCommand('which instruments');
-                    'which instruments',
+                  const hasProblem = result.hasProblem;
-                  );
+                  const message = hasProblem
-
+                    ? `Instruments not found. Please try to re-install Xcode (https://developer.apple.com/xcode/). ${result.message}.`
+                    : `Instruments are installed. ${result.message}.`;
                   return {
-                    hasProblem: !hasInstruments,
+                    hasProblem,
+                    message,
                   };
                 },
               },
@@ -153,7 +208,7 @@ export function getHealthchecks(): Healthchecks {
           }
         : {
             isSkipped: true,
-            skipReason: `Healthcheck is skipped, because iOS development is not supported on the current platform "${process.platform}"`,
+            skipReason: `Healthcheck is skipped, because iOS development is not supported on the current platform "${process.platform}".`,
           }),
     },
   };
@@ -199,10 +254,21 @@ export async function runHealthchecks(): Promise<
   return results;
 }
 
-async function commandSucceeds(command: string): Promise<boolean> {
+async function tryExecuteCommand(
-  return await promisify(exec)(command)
+  command: string,
-    .then(() => true)
+): Promise<HealthchecRunResult> {
-    .catch(() => false);
+  try {
+    const output = await promisify(exec)(command);
+    return {
+      hasProblem: false,
+      message: `Command "${command}" successfully executed with output: ${output.stdout}`,
+    };
+  } catch (err) {
+    return {
+      hasProblem: true,
+      message: `Command "${command}" failed to execute with output: ${err.message}`,
+    };
+  }
 }
 
 async function isWatchmanAvailable(): Promise<boolean> {

package.json

@@ -140,7 +140,7 @@
     "expand-tilde": "^2.0.2",
     "express": "^4.15.2",
     "fb-watchman": "^2.0.0",
-    "flipper-doctor": "^0.6.1",
+    "flipper-doctor": "^0.7.0",
     "fs-extra": "^8.0.1",
     "immer": "^5.2.1",
     "immutable": "^4.0.0-rc.12",

src/chrome/DoctorSheet.tsx

@@ -88,6 +88,7 @@ const SideContainer = styled(FlexBox)({
 const SideContainerText = styled(Text)({
   display: 'block',
   wordWrap: 'break-word',
+  overflow: 'auto',
 });
 
 const HealthcheckLabel = styled(Text)({
@@ -170,6 +171,7 @@ function HealthcheckIcon(props: {checkResult: HealthcheckResult}) {
 function HealthcheckDisplay(props: {
   label: string;
   result: HealthcheckResult;
+  selected?: boolean;
   onClick?: () => void;
 }) {
   return (
@@ -177,6 +179,7 @@ function HealthcheckDisplay(props: {
       <HealthcheckDisplayContainer shrink title={props.result.message}>
         <HealthcheckIcon checkResult={props.result} />
         <HealthcheckLabel
+          bold={props.selected}
           underline={!!props.onClick}
           cursor={props.onClick && 'pointer'}
           onClick={props.onClick}>
@@ -187,27 +190,25 @@ function HealthcheckDisplay(props: {
   );
 }
 
-function SideMessageDisplay(props: {
+function SideMessageDisplay(props: {children: React.ReactNode}) {
-  isHealthcheckInProgress: boolean;
+  return <SideContainerText selectable>{props.children}</SideContainerText>;
-  hasProblems: boolean;
+}
-}) {
+
-  if (props.isHealthcheckInProgress) {
+function ResultMessage(props: {result: HealthcheckResult}) {
+  if (status === 'IN_PROGRESS') {
+    return <p>Doctor is running healthchecks...</p>;
+  } else if (hasProblems(props.result)) {
     return (
-      <SideContainerText selectable>
+      <p>
-        Doctor is running healthchecks...
+        Doctor has discovered problems with your installation. Please click to
-      </SideContainerText>
+        each item to get details.
-    );
+      </p>
-  } else if (props.hasProblems) {
-    return (
-      <SideContainerText selectable>
-        Doctor has discovered problems with your installation.
-      </SideContainerText>
     );
   } else {
     return (
-      <SideContainerText selectable>
+      <p>
         All good! Doctor has not discovered any issues with your installation.
-      </SideContainerText>
+      </p>
     );
   }
 }
@@ -224,6 +225,7 @@ function hasNewProblems(result: HealthcheckResult) {
 export type State = {
   acknowledgeCheckboxVisible: boolean;
   acknowledgeOnClose?: boolean;
+  selectedCheckKey?: string;
 };
 
 type Props = OwnProps & StateFromProps & DispatchFromProps;
@@ -296,10 +298,20 @@ class DoctorSheet extends Component<Props, State> {
     helpUrl && shell.openExternal(helpUrl);
   }
 
-  async runHealthchecks() {
+  async runHealthchecks(): Promise<void> {
     await runHealthchecks(this.props);
   }
 
+  getCheckMessage(checkKey: string): string {
+    for (const cat of Object.values(this.props.healthcheckReport.categories)) {
+      const check = Object.values(cat.checks).find(chk => chk.key === checkKey);
+      if (check) {
+        return check.result.message || '';
+      }
+    }
+    return '';
+  }
+
   render() {
     return (
       <Container>
@@ -319,12 +331,17 @@ class DoctorSheet extends Component<Props, State> {
                         {Object.values(category.checks).map(check => (
                           <HealthcheckDisplay
                             key={check.key}
+                            selected={check.key === this.state.selectedCheckKey}
                             label={check.label}
                             result={check.result}
-                            onClick={
+                            onClick={() =>
-                              check.result.helpUrl
+                              this.setState({
-                                ? () => this.openHelpUrl(check.result.helpUrl)
+                                ...this.state,
-                                : undefined
+                                selectedCheckKey:
+                                  this.state.selectedCheckKey === check.key
+                                    ? undefined
+                                    : check.key,
+                              })
                             }
                           />
                         ))}
@@ -344,12 +361,16 @@ class DoctorSheet extends Component<Props, State> {
           </HealthcheckListContainer>
           <Spacer />
           <SideContainer shrink>
-            <SideMessageDisplay
+            <SideMessageDisplay>
-              isHealthcheckInProgress={
+              <SideContainerText selectable>
-                this.props.healthcheckReport.result.status === 'IN_PROGRESS'
+                {this.state.selectedCheckKey && (
-              }
+                  <p>{this.getCheckMessage(this.state.selectedCheckKey)}</p>
-              hasProblems={hasProblems(this.props.healthcheckReport.result)}
+                )}
-            />
+                {!this.state.selectedCheckKey && (
+                  <ResultMessage result={this.props.healthcheckReport.result} />
+                )}
+              </SideContainerText>
+            </SideMessageDisplay>
           </SideContainer>
         </FlexRow>
         <FlexRow>

src/reducers/__tests__/healthchecks.node.tsx

@@ -14,8 +14,7 @@ import {
   updateHealthcheckResult,
   acknowledgeProblems,
 } from '../healthchecks';
-import {Healthchecks} from 'flipper-doctor';
+import {Healthchecks, EnvironmentInfo} from 'flipper-doctor';
-import {EnvironmentInfo} from 'flipper-doctor/lib/environmentInfo';
 
 const HEALTHCHECKS: Healthchecks = {
   ios: {
@@ -27,7 +26,7 @@ const HEALTHCHECKS: Healthchecks = {
         key: 'ios.sdk',
         label: 'SDK Installed',
         run: async (_env: EnvironmentInfo) => {
-          return {hasProblem: false};
+          return {hasProblem: false, message: ''};
         },
       },
     ],
@@ -41,7 +40,7 @@ const HEALTHCHECKS: Healthchecks = {
         key: 'android.sdk',
         label: 'SDK Installed',
         run: async (_env: EnvironmentInfo) => {
-          return {hasProblem: true};
+          return {hasProblem: true, message: 'Error'};
         },
       },
     ],
@@ -55,7 +54,7 @@ const HEALTHCHECKS: Healthchecks = {
         key: 'common.openssl',
         label: 'OpenSSL Istalled',
         run: async (_env: EnvironmentInfo) => {
-          return {hasProblem: false};
+          return {hasProblem: false, message: ''};
         },
       },
     ],

src/reducers/healthchecks.tsx

@@ -60,7 +60,6 @@ export type HealthcheckResult = {
   status: HealthcheckStatus;
   isAcknowledged?: boolean;
   message?: string;
-  helpUrl?: string;
 };
 
 export type HealthcheckReportItem = {

src/utils/runHealthchecks.tsx

@@ -66,14 +66,14 @@ async function launchHealthchecks(options: HealthcheckOptions): Promise<void> {
         checkResult.hasProblem && h.isRequired
           ? {
               status: 'FAILED',
-              helpUrl: checkResult.helpUrl,
+              message: checkResult.message,
             }
           : checkResult.hasProblem && !h.isRequired
           ? {
               status: 'WARNING',
-              helpUrl: checkResult.helpUrl,
+              message: checkResult.message,
             }
-          : {status: 'SUCCESS'};
+          : {status: 'SUCCESS', message: checkResult.message};
       options.updateHealthcheckResult(categoryKey, h.key, result);
     }
   }

yarn.lock

@@ -4192,10 +4192,10 @@ flatted@^2.0.0:
   resolved "https://registry.yarnpkg.com/flatted/-/flatted-2.0.1.tgz#69e57caa8f0eacbc281d2e2cb458d46fdb449e08"
   integrity sha512-a1hQMktqW9Nmqr5aktAux3JMNqaucxGcjtjWnZLHX7yyPCmlSV3M54nGYbqT8K+0GhF3NBgmJCc3ma+WOgX8Jg==
 
-flipper-doctor@^0.6.1:
+flipper-doctor@^0.7.0:
-  version "0.6.1"
+  version "0.7.0"
-  resolved "https://registry.yarnpkg.com/flipper-doctor/-/flipper-doctor-0.6.1.tgz#7a10cbe655293332c509d3ca37611bc32ee5d514"
+  resolved "https://registry.yarnpkg.com/flipper-doctor/-/flipper-doctor-0.7.0.tgz#642aca4004add6e94e29fa69e5ea58b5ab7b724a"
-  integrity sha512-XRN5LqTK9J+2K5ixPwEODHYRyfL3hs9qDsKzcPoecoHP2DxPBOJNm2d2+J03lqFMedYE63a6+WhWUXOQoBRyEQ==
+  integrity sha512-cdT/nXiRkJH3Y2HYr3rCZyjOgk/+hLry4QYxp7gvKhKA6Nvy4SiPgSCae2aBTdFSe1KoQeqD3mFURrkVlZcsWg==
   dependencies:
     "@types/node" "^12.12.12"
     envinfo "^7.4.0"