Skip to content

Add check for JAVA 9 #3294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 3, 2018
Merged

Add check for JAVA 9 #3294

merged 2 commits into from
Jan 3, 2018

Conversation

rosen-vladimirov
Copy link
Contributor

@rosen-vladimirov rosen-vladimirov commented Dec 29, 2017

Currently Gradle cannot work with JAVA 9, so detect if it has been used and break the build for Android. Also the check will print warning when tns doctor is called.

Handle case when Javac version is a single number (9 for example)

In some cases javac version is a single number, for example 9. In this case our validation fails to detect the correct version and to check if we support this version.
In order to resolve this issue, use the appendZeroesToVersion method in order to make the versin semver valid.
Change the return type of validateJavacVersion method - it does not require to return a Promise.
Add unit tests for validateJavacVersion method.

Also update submodule, where the following change is applied:

Fix detection of Javac version

The command javac -version prints result to stderr when JAVA 8 is used and to stdout when JAVA 9 is used. Current check in CLI uses the stderr output, so when JAVA 9 is installed it fails to detect the correct version.
In order to support both JAVA 8 and JAVA 9, capture both stdout and stderr and get the version from there.
Also remove unneeded check for Java version - we care about JAVA Compiler, which is included in JDK.

Merge after telerik/mobile-cli-lib#1037

Currently Gradle cannot work with JAVA 9, so detect if it has been used and break the build for Android. Also the check will print warning when `tns doctor` is called.
Also update submodule, where the following change is applied:

Fix detection of Javac version

The command `javac -version` prints result to stderr when JAVA 8 is used and to stdout when JAVA 9 is used. Current check in CLI uses the stderr output, so when JAVA 9 is installed it fails to detect the correct version.
In order to support both JAVA 8 and JAVA 9, capture both stdout and stderr and get the version from there.
Also remove unneeded check for Java version - we care about JAVA Compiler, which is included in JDK.
hasProblemWithJavaVersion = true;
this.printMessage(`Javac version ${installedJavaVersion} is not supported. You have to install at least ${AndroidToolsInfo.MIN_JAVA_VERSION}.`, additionalMessage);
} else if (semver.gt(installedJavaCompilerVersion, AndroidToolsInfo.MAX_JAVA_VERSION)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should be gte

Copy link
Contributor

@Mitko-Kerezov Mitko-Kerezov left a comment

Choose a reason for hiding this comment

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

Looks okay to me

if (matchingVersion && matchingVersion[1]) {
if (semver.lt(matchingVersion[1], AndroidToolsInfo.MIN_JAVA_VERSION)) {

const matchingVersion = (installedJavacVersion ? appendZeroesToVersion(installedJavacVersion, 3) : "").match(AndroidToolsInfo.VERSION_REGEX);
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be shortened to

const matchingVersion = appendZeroesToVersion(installedJavacVersion || "", 3).match(AndroidToolsInfo.VERSION_REGEX);

as appendZeroesToVersion would return an empty string if an empty string is passed as an argument.

const androidToolsInfo = testInjector.resolve<IAndroidToolsInfo>(AndroidToolsInfo);
assert.deepEqual(androidToolsInfo.validateJavacVersion(javacVersion), expectedResult);
if (warnings && warnings.length) {

Copy link
Contributor

Choose a reason for hiding this comment

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

We could do without this new line

In some cases javac version is a single number, for example 9. In this case our validation fails to detect the correct version and to check if we support this version.
In order to resolve this issue, use the `appendZeroesToVersion` method in order to make the versin semver valid.
Change the return type of `validateJavacVersion` method - it does not require to return a Promise.
Add unit tests for `validateJavacVersion` method.
@rosen-vladimirov rosen-vladimirov merged commit e730cf4 into master Jan 3, 2018
@rosen-vladimirov rosen-vladimirov deleted the vladimirov/java-9 branch January 3, 2018 11:29
mbektchiev pushed a commit that referenced this pull request Jan 9, 2018
* Add check for JAVA 9

Currently Gradle cannot work with JAVA 9, so detect if it has been used and break the build for Android. Also the check will print warning when `tns doctor` is called.
Also update submodule, where the following change is applied:

Fix detection of Javac version

The command `javac -version` prints result to stderr when JAVA 8 is used and to stdout when JAVA 9 is used. Current check in CLI uses the stderr output, so when JAVA 9 is installed it fails to detect the correct version.
In order to support both JAVA 8 and JAVA 9, capture both stdout and stderr and get the version from there.
Also remove unneeded check for Java version - we care about JAVA Compiler, which is included in JDK.

* Handle case when Javac version is a single number (9 for example)

In some cases javac version is a single number, for example 9. In this case our validation fails to detect the correct version and to check if we support this version.
In order to resolve this issue, use the `appendZeroesToVersion` method in order to make the versin semver valid.
Change the return type of `validateJavacVersion` method - it does not require to return a Promise.
Add unit tests for `validateJavacVersion` method.
rosen-vladimirov added a commit that referenced this pull request Jan 9, 2018
* minor grammatical update (#3267)

* Add check for JAVA 9 (#3294)

* Add check for JAVA 9

Currently Gradle cannot work with JAVA 9, so detect if it has been used and break the build for Android. Also the check will print warning when `tns doctor` is called.
Also update submodule, where the following change is applied:

Fix detection of Javac version

The command `javac -version` prints result to stderr when JAVA 8 is used and to stdout when JAVA 9 is used. Current check in CLI uses the stderr output, so when JAVA 9 is installed it fails to detect the correct version.
In order to support both JAVA 8 and JAVA 9, capture both stdout and stderr and get the version from there.
Also remove unneeded check for Java version - we care about JAVA Compiler, which is included in JDK.

* Handle case when Javac version is a single number (9 for example)

In some cases javac version is a single number, for example 9. In this case our validation fails to detect the correct version and to check if we support this version.
In order to resolve this issue, use the `appendZeroesToVersion` method in order to make the versin semver valid.
Change the return type of `validateJavacVersion` method - it does not require to return a Promise.
Add unit tests for `validateJavacVersion` method.

* release notes 3.4.0 (#3297)

* release notes 3.4.0

* add implemented item

* remove a fixed issue from changelog

* Update submodule to release branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants