Skip to content

Commit d6ad87e

Browse files
rosen-vladimirovrosen-vladimirov
committed
fix: prompts to install javac should not be shown on macOS
macOS has handle when you try to execute java or javac to prompt to install it. However, we do not want to bother the user with such prompts. Currently we are executing `javac -version` to determine the version. Instead of this, check if there's `javac` in the PATH before executing the command. In case there's no javac in the path, do not execute `javac -version`, so we'll not prompt the user to install it.
1 parent dc410fd commit d6ad87e

File tree

5 files changed

+105
-32
lines changed

5 files changed

+105
-32
lines changed

‎lib/constants.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,5 @@ export class Constants {
1515
public static ANDROID_RUNTIME = "tns-android";
1616
public static VERSION_PROPERTY_NAME = "version";
1717
public static XCODE_MIN_REQUIRED_VERSION = 9;
18+
public static JAVAC_EXECUTABLE_NAME = "javac";
1819
}

‎lib/sys-info.ts‎

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,8 @@ export class SysInfo implements NativeScriptDoctor.ISysInfo {
5757

5858
public getJavaCompilerVersion(): Promise<string> {
5959
return this.getValueForProperty(() => this.javaCompilerVerCache, async (): Promise<string> => {
60-
const javaCompileExecutableName = "javac";
61-
const javaHome = process.env["JAVA_HOME"];
62-
const pathToJavaCompilerExecutable = javaHome ? path.join(javaHome, "bin", javaCompileExecutableName) : javaCompileExecutableName;
63-
try {
64-
const output = await this.childProcess.exec(`"${pathToJavaCompilerExecutable}" -version`);
65-
return SysInfo.JAVA_COMPILER_VERSION_REGEXP.exec(`${output.stderr}${EOL}${output.stdout}`)[1];
66-
} catch (err) {
67-
return null;
68-
}
60+
const javacVersion = (await this.getJavacVersionFromJavaHome()) || (await this.getJavacVersionFromPath());
61+
return javacVersion;
6962
});
7063
}
7164

@@ -499,4 +492,44 @@ export class SysInfo implements NativeScriptDoctor.ISysInfo {
499492
return result;
500493
});
501494
}
495+
496+
private async getJavacVersionFromPath(): Promise<string> {
497+
let javacVersion: string = null;
498+
499+
try {
500+
const detectionCommand = this.hostInfo.isWindows ? "where" : "which";
501+
// if this command succeeds, we have javac in the PATH. In case it is not there, it will throw an error.
502+
await this.childProcess.exec(`${detectionCommand} ${Constants.JAVAC_EXECUTABLE_NAME}`);
503+
javacVersion = await this.executeJavacVersion(Constants.JAVAC_EXECUTABLE_NAME);
504+
} catch (err) { /* intentionally left blank */ }
505+
506+
return javacVersion;
507+
}
508+
509+
private async getJavacVersionFromJavaHome(): Promise<string> {
510+
let javacVersion: string = null;
511+
512+
try {
513+
const javaHome = process.env["JAVA_HOME"];
514+
const javacExecutableFile = this.hostInfo.isWindows ? `${Constants.JAVAC_EXECUTABLE_NAME}.exe` : Constants.JAVAC_EXECUTABLE_NAME;
515+
516+
if (javaHome) {
517+
const pathToJavaCompilerExecutable = path.join(javaHome, "bin", javacExecutableFile);
518+
if (this.fileSystem.exists(pathToJavaCompilerExecutable)) {
519+
javacVersion = await this.executeJavacVersion(pathToJavaCompilerExecutable);
520+
}
521+
}
522+
} catch (err) { /* intentionally left blank */ }
523+
524+
return javacVersion;
525+
}
526+
527+
private async executeJavacVersion(pathToJavaCompilerExecutable: string): Promise<string> {
528+
try {
529+
const output = await this.childProcess.exec(`"${pathToJavaCompilerExecutable}" -version`);
530+
return SysInfo.JAVA_COMPILER_VERSION_REGEXP.exec(`${output.stderr}${EOL}${output.stdout}`)[1];
531+
} catch (err) {
532+
return null;
533+
}
534+
}
502535
}

‎package-lock.json‎

Lines changed: 22 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"main": "lib/index.js",
66
"types": "./typings/nativescript-doctor.d.ts",
77
"scripts": {
8-
"test": "node_modules/.bin/istanbul cover node_modules/.bin/mocha -- --recursive"
8+
"test": "node_modules/.bin/istanbul cover ./node_modules/mocha/bin/_mocha -- --recursive"
99
},
1010
"repository": {
1111
"type": "git",

‎test/sys-info.ts‎

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ function createChildProcessResults(childProcessResult: IChildProcessResults): ID
8282
"npm -v": childProcessResult.npmV,
8383
"node -v": childProcessResult.nodeV,
8484
'"javac" -version': childProcessResult.javacVersion,
85+
'which javac': { result: '' },
86+
'where javac': { result: '' },
8587
"node-gyp -v": childProcessResult.nodeGypVersion,
8688
"xcodebuild -version": childProcessResult.xCodeVersion,
8789
"pod --version": childProcessResult.podVersion,
@@ -178,16 +180,19 @@ describe("SysInfo unit tests", () => {
178180

179181
describe("Should execute correct commands to check for", () => {
180182
let spawnFromEventCommand: string;
181-
let execCommand: string;
183+
let execCommands: string[] = [];
184+
let fileSystem: any;
185+
let hostInfo: any;
182186

183187
beforeEach(() => {
188+
execCommands = [];
184189
const childProcess: ChildProcess = {
185190
spawnFromEvent: async (command: string, args: string[], event: string) => {
186191
spawnFromEventCommand = `${command} ${args.join(" ")}`;
187192
return { stdout: "", stderr: "" };
188193
},
189194
exec: async (command: string) => {
190-
execCommand = command;
195+
execCommands.push(command);
191196
return { stdout: "", stderr: "" };
192197
},
193198
execFile: async () => {
@@ -197,29 +202,49 @@ describe("SysInfo unit tests", () => {
197202
};
198203

199204
const helpers = new Helpers(null);
200-
sysInfo = new SysInfo(childProcess, null, helpers, null, null, androidToolsInfo);
205+
fileSystem = {
206+
exists: () => false,
207+
extractZip: () => Promise.resolve(),
208+
readDirectory: () => Promise.resolve([])
209+
};
210+
211+
hostInfo = {
212+
isWindows: false
213+
};
214+
215+
sysInfo = new SysInfo(childProcess, fileSystem, helpers, hostInfo, null, androidToolsInfo);
201216
});
202217

203218
it("java compiler version when there is JAVA_HOME.", async () => {
204219
const originalJavaHome = process.env[JavaHomeName];
205220
process.env[JavaHomeName] = "mock";
206221

207222
const pathToJavac = path.join(process.env[JavaHomeName], "bin", "javac");
223+
fileSystem.exists = () => true;
208224
await sysInfo.getJavaCompilerVersion();
209225

210226
process.env[JavaHomeName] = originalJavaHome;
211-
assert.deepEqual(execCommand, `"${pathToJavac}" -version`);
227+
assert.deepEqual(execCommands[0], `"${pathToJavac}" -version`);
212228
});
213229

214-
it("java compiler version when there is no JAVA_HOME.", async () => {
230+
it("java compiler version when there is no JAVA_HOME on non-Windows OS", async () => {
215231
const originalJavaHome = process.env[JavaHomeName];
216232

217233
delete process.env[JavaHomeName];
234+
await sysInfo.getJavaCompilerVersion();
218235

236+
process.env[JavaHomeName] = originalJavaHome;
237+
assert.deepEqual(execCommands, ['which javac', '"javac" -version']);
238+
});
239+
240+
it("java compiler version when there is no JAVA_HOME on Window OS", async () => {
241+
const originalJavaHome = process.env[JavaHomeName];
242+
hostInfo.isWindows = true;
243+
delete process.env[JavaHomeName];
219244
await sysInfo.getJavaCompilerVersion();
220245

221246
process.env[JavaHomeName] = originalJavaHome;
222-
assert.deepEqual(execCommand, `"javac" -version`);
247+
assert.deepEqual(execCommands, ['where javac', '"javac" -version']);
223248
});
224249
});
225250

@@ -352,7 +377,7 @@ describe("SysInfo unit tests", () => {
352377
childProcessResult.adbVersion = {
353378
result: null
354379
};
355-
sysInfo = mockSysInfo(childProcessResult, { isWindows: false, isDarwin: false, dotNetVersion: "4.5.1"}, null);
380+
sysInfo = mockSysInfo(childProcessResult, { isWindows: false, isDarwin: false, dotNetVersion: "4.5.1" }, null);
356381
const adbVersion = await sysInfo.getAdbVersion();
357382
const isAndroidSdkConfiguredCorrectly = await sysInfo.isAndroidSdkConfiguredCorrectly();
358383
assert.deepEqual(adbVersion, null);
@@ -362,12 +387,12 @@ describe("SysInfo unit tests", () => {
362387

363388
describe("pythonInfo", () => {
364389
it("should return null when platform is windows", async () => {
365-
sysInfo = mockSysInfo(childProcessResult, { isWindows: true, isDarwin: false, dotNetVersion: "4.5.1"}, null);
390+
sysInfo = mockSysInfo(childProcessResult, { isWindows: true, isDarwin: false, dotNetVersion: "4.5.1" }, null);
366391
const pythonInfo = await sysInfo.getPythonInfo();
367392
assert.deepEqual(pythonInfo, null);
368393
});
369394
it("should return null when platform is linux", async () => {
370-
sysInfo = mockSysInfo(childProcessResult, { isWindows: false, isDarwin: false, dotNetVersion: "4.5.1"}, null);
395+
sysInfo = mockSysInfo(childProcessResult, { isWindows: false, isDarwin: false, dotNetVersion: "4.5.1" }, null);
371396
const pythonInfo = await sysInfo.getPythonInfo();
372397
assert.deepEqual(pythonInfo, null);
373398
});
@@ -385,7 +410,7 @@ describe("SysInfo unit tests", () => {
385410
});
386411
it("should return {isInstalled: true, isSixPackageInstalled: false} when python is installed but six package is not", async () => {
387412
childProcessResult.pythonInfo = { shouldThrowError: true, errorCode: 1 };
388-
sysInfo = mockSysInfo(childProcessResult, { isWindows: false, isDarwin: true, dotNetVersion: "4.5.1"}, null);
413+
sysInfo = mockSysInfo(childProcessResult, { isWindows: false, isDarwin: true, dotNetVersion: "4.5.1" }, null);
389414
const pythonInfo = await sysInfo.getPythonInfo();
390415
assert.deepEqual(pythonInfo, { isInstalled: true, isSixPackageInstalled: false });
391416
});
@@ -528,12 +553,12 @@ ${expectedCliVersion}`;
528553
assert.deepEqual(result.isCocoaPodsWorkingCorrectly, true);
529554
assert.deepEqual(result.xcprojInfo, undefined);
530555
assert.deepEqual(result.isCocoaPodsUpdateRequired, false);
531-
assert.deepEqual(result.pythonInfo, {isInstalled: false, isSixPackageInstalled: false, installationErrorMessage: "Cannot read property 'shouldThrowError' of undefined"});
556+
assert.deepEqual(result.pythonInfo, { isInstalled: false, isSixPackageInstalled: false, installationErrorMessage: "Cannot read property 'shouldThrowError' of undefined" });
532557
};
533558

534559
it("iOS platform is specified", async () => {
535560
sysInfo = mockSysInfo(childProcessResult, { isWindows: false, isDarwin: true, dotNetVersion });
536-
const result = await sysInfo.getSysInfo({platform: "iOS"});
561+
const result = await sysInfo.getSysInfo({ platform: "iOS" });
537562

538563
assertCommonSysInfo(result);
539564
assertiOSSysInfo(result);
@@ -547,7 +572,7 @@ ${expectedCliVersion}`;
547572
});
548573
it("Android platform is specified", async () => {
549574
sysInfo = mockSysInfo(childProcessResult, { isWindows: false, isDarwin: true, dotNetVersion }, { existsResult: true });
550-
const result = await sysInfo.getSysInfo({platform: "Android"});
575+
const result = await sysInfo.getSysInfo({ platform: "Android" });
551576

552577
assertCommonSysInfo(result);
553578
assertAndroidSysInfo(result);
@@ -561,7 +586,7 @@ ${expectedCliVersion}`;
561586
assert.deepEqual(result.isCocoaPodsUpdateRequired, undefined);
562587
assert.deepEqual(result.pythonInfo, undefined);
563588
});
564-
it("no platform is specified", async() => {
589+
it("no platform is specified", async () => {
565590
sysInfo = mockSysInfo(childProcessResult, { isWindows: false, isDarwin: true, dotNetVersion });
566591
const result = await sysInfo.getSysInfo();
567592
assertCommonSysInfo(result);

0 commit comments

Comments
 (0)