Skip to content
This repository was archived by the owner on Feb 2, 2021. It is now read-only.

Fix detection of Javac version #1037

Merged
merged 2 commits into from
Jan 3, 2018
Merged

Fix detection of Javac version #1037

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
487e70e
Fix detection of Javac version
rosen-vladimirov Dec 29, 2017
8445473
Improve appendZeroesToVersion and add unit tests
rosen-vladimirov Jan 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions declarations.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1123,8 +1123,6 @@ interface ISysInfoData extends IPlatform {
nodeGypVer: string;

// dependencies
/** version of java, as returned by `java -version` */
javaVer: string;
/** Xcode version string as returned by `xcodebuild -version`. Valid only on Mac */
xcodeVer: string;
/** Version string of adb, as returned by `adb version` */
Expand Down Expand Up @@ -1156,9 +1154,6 @@ interface ISysInfo {
*/
getSysInfo(pathToPackageJson: string, androidToolsInfo?: { pathToAdb: string }): Promise<ISysInfoData>;

/** Returns Java version. **/
getJavaVersion(): Promise<string>;

/** Returns Java compiler version. **/
getJavaCompilerVersion(): Promise<string>;

Expand Down
17 changes: 1 addition & 16 deletions sys-info-base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,6 @@ export class SysInfoBase implements ISysInfo {

private monoVerRegExp = /version (\d+[.]\d+[.]\d+) /gm;
private sysInfoCache: ISysInfoData = undefined;
private javaVerCache: string = null;
public async getJavaVersion(): Promise<string> {
if (!this.javaVerCache) {
try {
// different java has different format for `java -version` command
const output = (await this.$childProcess.spawnFromEvent("java", ["-version"], "exit")).stderr;
this.javaVerCache = /(?:openjdk|java) version \"((?:\d+\.)+(?:\d+))/i.exec(output)[1];
} catch (e) {
this.javaVerCache = null;
}
}
return this.javaVerCache;
}

private npmVerCache: string = null;
public async getNpmVersion(): Promise<string> {
Expand All @@ -47,7 +34,7 @@ export class SysInfoBase implements ISysInfo {
const output = await this.exec(`"${pathToJavaCompilerExecutable}" -version`, { showStderr: true });
// for other versions of java javac version output is not on first line
// thus can't use ^ for starts with in regex
this.javaCompilerVerCache = output ? /javac (.*)/i.exec(output.stderr)[1] : null;
this.javaCompilerVerCache = output ? /javac (.*)/i.exec(`${output.stderr}${os.EOL}${output.stdout}`)[1] : null;
} catch (e) {
this.$logger.trace(`Command "${pathToJavaCompilerExecutable} --version" failed: ${e}`);
this.javaCompilerVerCache = null;
Expand Down Expand Up @@ -154,8 +141,6 @@ export class SysInfoBase implements ISysInfo {

res.npmVer = await this.getNpmVersion();

res.javaVer = await this.getJavaVersion();

res.nodeGypVer = await this.getNodeGypVersion();
res.xcodeVer = await this.getXCodeVersion();
res.xcodeprojGemLocation = await this.getXCodeProjGemLocation();
Expand Down
34 changes: 25 additions & 9 deletions test/unit-tests/sys-info-base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ interface IChildProcessResultDescription {
interface IChildProcessResults {
uname: IChildProcessResultDescription;
npmV: IChildProcessResultDescription;
javaVersion: IChildProcessResultDescription;
javacVersion: IChildProcessResultDescription;
nodeGypVersion: IChildProcessResultDescription;
xCodeVersion: IChildProcessResultDescription;
Expand All @@ -32,7 +31,7 @@ interface IChildProcessResults {
}

function getResultFromChildProcess(childProcessResultDescription: IChildProcessResultDescription, spawnFromEventOpts?: { throwError: boolean }): any {
if (childProcessResultDescription.shouldThrowError) {
if (!childProcessResultDescription || childProcessResultDescription.shouldThrowError) {
if (spawnFromEventOpts && !spawnFromEventOpts.throwError) {
return {
stderr: "This one throws error.",
Expand All @@ -51,7 +50,6 @@ function createChildProcessResults(childProcessResult: IChildProcessResults): ID
return {
"uname -a": childProcessResult.uname,
"npm -v": childProcessResult.npmV,
"java": childProcessResult.javaVersion,
'"javac" -version': childProcessResult.javacVersion,
"node-gyp -v": childProcessResult.nodeGypVersion,
"xcodebuild -version": childProcessResult.xCodeVersion,
Expand All @@ -65,7 +63,7 @@ function createChildProcessResults(childProcessResult: IChildProcessResults): ID
};
}

function createTestInjector(childProcessResult: IChildProcessResults, hostInfoData: { isWindows: boolean, dotNetVersion: string, isDarwin: boolean }, itunesError: string): IInjector {
function createTestInjector(childProcessResult: IChildProcessResults, hostInfoData: { isWindows: boolean, dotNetVersion?: string, isDarwin: boolean }, itunesError: string): IInjector {
const injector = new Yok();
const childProcessResultDictionary = createChildProcessResults(childProcessResult);
injector.register("childProcess", {
Expand All @@ -79,7 +77,7 @@ function createTestInjector(childProcessResult: IChildProcessResults, hostInfoDa
});

injector.register("hostInfo", {
dotNetVersion: () => Promise.resolve(hostInfoData.dotNetVersion),
dotNetVersion: () => Promise.resolve(hostInfoData.dotNetVersion || "4.5.1"),
isWindows: hostInfoData.isWindows,
isDarwin: hostInfoData.isDarwin
});
Expand Down Expand Up @@ -123,7 +121,6 @@ describe("sysInfoBase", () => {
childProcessResult = {
uname: { result: "name" },
npmV: { result: "2.14.1" },
javaVersion: { result: { stderr: 'java version "1.8.0_60"' } },
javacVersion: { result: { stderr: 'javac 1.8.0_60' } },
nodeGypVersion: { result: "2.0.0" },
xCodeVersion: { result: "6.4.0" },
Expand All @@ -142,7 +139,6 @@ describe("sysInfoBase", () => {
describe("returns correct results when everything is installed", () => {
const assertCommonValues = (result: ISysInfoData) => {
assert.deepEqual(result.npmVer, childProcessResult.npmV.result);
assert.deepEqual(result.javaVer, "1.8.0");
assert.deepEqual(result.javacVersion, "1.8.0_60");
assert.deepEqual(result.nodeGypVer, childProcessResult.nodeGypVersion.result);
assert.deepEqual(result.adbVer, childProcessResult.adbVersion.result);
Expand Down Expand Up @@ -214,12 +210,33 @@ describe("sysInfoBase", () => {
});
});

describe("getJavaCompilerVersion", () => {
const verifyJavaCompilerVersion = async (javaCompilerVersion: string, resultStream: string): Promise<void> => {
const javaCompilerChildProcessResult: any = {
[resultStream]: `javac ${javaCompilerVersion}`
};

javaCompilerChildProcessResult.javacVersion = { result: javaCompilerChildProcessResult };
testInjector = createTestInjector(javaCompilerChildProcessResult, { isWindows: false, isDarwin: true }, null);
sysInfoBase = testInjector.resolve("sysInfoBase");
const actualJavaCompilerVersion = await sysInfoBase.getJavaCompilerVersion();
assert.deepEqual(actualJavaCompilerVersion, javaCompilerVersion);
};

it("returns correct javac version when it is printed on stderr (Java 8)", () => {
return verifyJavaCompilerVersion("1.8.0_152", "stderr");
});

it("returns correct javac version when it is printed on stdout (Java 9)", () => {
return verifyJavaCompilerVersion("9.0.1", "stdout");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rosen-vladimirov some distributions of java return 9 when java -version is called, in which case I think the regex might break. I haven't tested it but I encountered such an issue in the past.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this. In case 9 is returned, the current code will work fine, but this one will fail. I'll handle the case there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Plamen5kov fixed in this PR NativeScript/nativescript-cli#3294

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rosen-vladimirov I tested it, and CLI provides a message to install java 1.8.0.

});
});

describe("returns correct results when exceptions are raised during sysInfo data collection", () => {
beforeEach(() => {
childProcessResult = {
uname: { shouldThrowError: true },
npmV: { shouldThrowError: true },
javaVersion: { shouldThrowError: true },
javacVersion: { shouldThrowError: true },
nodeGypVersion: { shouldThrowError: true },
xCodeVersion: { shouldThrowError: true },
Expand Down Expand Up @@ -253,7 +270,6 @@ describe("sysInfoBase", () => {
sysInfoBase = testInjector.resolve("sysInfoBase");
const result = await sysInfoBase.getSysInfo(toolsPackageJson);
assert.deepEqual(result.npmVer, null);
assert.deepEqual(result.javaVer, null);
assert.deepEqual(result.javacVersion, null);
assert.deepEqual(result.nodeGypVer, null);
assert.deepEqual(result.xcodeVer, null);
Expand Down