Update check_reqs.js - allow JDK > 1.8 #928

dkotrada · 2020-03-13T09:47:34Z

Checking JDK should not fail when there is another version installed like 11.0.6 for example.
Redme file declares clearly

Requires

> Java JDK 1.8 or greater

Motivation and Context

I have another verison installed. It fails but shouldn't.

Checking JDK should not fail when there is another version installed like 11.0.6 for example. Redme file declare clearly > Requires > Java JDK 1.8 or greater

Indentation fix

fix trailing spaces

breautek · 2020-03-13T12:35:37Z

Java JDK 1.8 or greater

Is actually wrong. Android requires 1.8, not anything lower, not anything greater. So if the readme states this, it probably should be changed. But I would favour the README change in a separate PR.

jcesarmobile · 2020-03-13T13:03:49Z

As Norman said, Android requires Java 8, so the docs should be changed, not the code.

dkotrada · 2020-03-13T15:51:50Z

Java JDK 1.8 or greater

Is actually wrong. Android requires 1.8, not anything lower, not anything greater. So if the readme states this, it probably should be changed. But I would favour the README change in a separate PR.

I am not native speaker so you may not understood me what I mean with this PR.

With my code you can use any java version. Who has today lower than 1.8? Next LTS version is 11. So my code works well with both 1.8 and greather current code is wrong because it checks only for 1.8. But cordova works with greather versions.

But you can do what ever you want. Have a nice day.

jcesarmobile · 2020-03-13T16:01:10Z

I'm reopening so can be tested, but supposedly Android only works with Java 8.
But maybe, as long as we don't use newer Java API features, it could still compile with newer SDK versions.

breautek · 2020-03-13T16:18:49Z

But maybe, as long as we don't use newer Java API features, it could still compile with newer SDK versions.

I'm pretty sure this is the case. The android documentation says that the target compile needs to be set to jdk 8, which is what we define in gradle.

https://developer.android.com/studio/write/java8-support

So we cannot use features that only appear in later versions. I still think it's safer to stick with java8, but perhaps the checks can be less restrictive, which I believe is what this PR tries to do.

I would like to see some tests added to ensure the checks returns appropriately when it finds older versions of java and when it finds newer versions of java.

And if we go that route, then I guess this will make #929 obsolete.

breautek · 2020-04-14T00:33:22Z

I just tested openJDK 11 on ubuntu and the tests was passed.. so I think was indeed wrong about the android sdk strictly requiring java 8.

If we are going to open up that requirements check though, I think we should have a suite of tests for both java8 and java11 in the github actions.

GuilleW · 2020-05-11T21:00:23Z

Hi !
Same problem here. I'm on Debian GNU/Linux bullseye/sid with openjdk-11-jdk and openjdk-8-jdk installed.
In my .bashrc, I have :
export JAVA_HOME=/usr/lib/jvm/java-1.8.0-openjdk-amd64
and my JDK 8 is not used by cordova-android build.

Actually, I patch this file manually to
[197] return Q.denodeify(child_process.exec)(process.env['JAVA_HOME'] + '/bin/javac -version')

Or ... can we set a ['CORDOVA_JAVA'] env variable so everyone can use last update of java, and a specific version for cordova ?

GuilleW · 2020-05-11T21:50:05Z

Sorry, I just saw that the NPM package cordova-android : 8.1.0 is not updated since 8 months, so ... my suggestion is already outdated.

breautek · 2020-05-11T22:14:23Z

Nope, i dont think you're suggestion is out dated.

I'm not sure which route we should take, but there are two different routes imo:

this route this PR addresses, which opens up the check to allow java 8+, including java11. Android sdk specifically requires 8, but I don't think that means you need jdk8... because we actually do explicitly say to target/compile against java 8 in the gradle configs. In this route, I think we should have tests with both jdk8 and jdk11 environments. I'm personally not aware of any cons of using the jdk11.
we keep the restrictive jdk check (that is, must require jdk 8), but provide a cordova environment variable like you suggested so users can have both jdk versions installed at the same time. Cordova will check for the cordova java home variable, then fall back to the generic JAVA_HOME variable. This route is probably the safer option.

GuilleW · 2020-05-11T22:41:34Z

IMO, option 2 is safer and doesn't introdruce new bugs if it works great with JDK8.
With this option, we have time to test JDK11.

breautek · 2020-05-15T00:38:16Z

On this note, I did add java 11 to the test CI on my fork, and it all appears to pass: https://github.com/breautek/cordova-android/runs/676508506?check_suite_focus=true

Laubeee · 2020-05-29T16:01:41Z

is there any quick fix to this? I have multiple JREs/JDKs and even if my JAVA_HOME points to jdk1.8, java -version gives me 1.8, I still get the error

Requirements check failed for JDK 8 ('1.8.*')! Detected version: 11.0.4

breautek · 2020-05-29T16:48:45Z

even if my JAVA_HOME points to jdk1.8

Improvements were made with how java is detected, so this may be fixed in cordova-android@nightly. Can you test? We are looking to release cordova-android@9 soon, but I can't give any specific ETA.

GuilleW · 2020-05-29T20:19:32Z

I can't build with nightly.
TL;DR look at the last $ cordova build

See below...

`$ cordova create hello com.example.hello HelloWorld`

Creating a new cordova project.

`$ cd hello`

`$ cordova platform add android`

Using cordova-fetch for cordova-android@^8.0.0
Adding android project...
Creating Cordova project for the Android platform:
	Path: platforms/android
	Package: com.example.hello
	Name: HelloWorld
	Activity: MainActivity
	Android target: android-28
Subproject Path: CordovaLib
Subproject Path: app
Android project created with cordova-android@8.1.0
Plugin 'cordova-plugin-whitelist' found in config.xml... Migrating it to package.json
Discovered saved plugin "cordova-plugin-whitelist". Adding it to the project
Installing "cordova-plugin-whitelist" for android
Adding cordova-plugin-whitelist to package.json

`$ cordova requirements`

Requirements check results for android:
Java JDK: installed 11.0.7
Android SDK: installed true
Android target: installed android-28
Gradle: installed /home/guillew/App/gradle/gradle-6.4/bin/gradle

`$ cordova build`

Checking Java JDK and Android SDK versions
ANDROID_SDK_ROOT=/home/guillew/App/android-sdk (recommended setting)
ANDROID_HOME=/home/guillew/App/android-sdk (DEPRECATED)
Requirements check failed for JDK 8 ('1.8.*')! Detected version: 11.0.7
Check your ANDROID_SDK_ROOT / JAVA_HOME / PATH environment variables.

Update to nightly

`$ npm i cordova-android@nightly`

npm WARN com.example.hello@1.0.0 No repository field.

+ cordova-android@9.0.0-nightly.2020.5.29.a830145f
added 39 packages from 40 contributors, removed 6 packages, updated 12 packages and audited 88 packages in 6.784s

2 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

`$ cordova build`

Unable to load PlatformApi from platform. Error: Cannot find module 'shelljs'
Require stack:
- /home/guillew/hello/platforms/android/cordova/lib/pluginHandlers.js
- /home/guillew/hello/platforms/android/cordova/lib/AndroidProject.js
- /home/guillew/hello/platforms/android/cordova/Api.js
- /usr/lib/node_modules/cordova/node_modules/cordova-lib/src/cordova/util.js
- /usr/lib/node_modules/cordova/node_modules/cordova-lib/src/platforms/platforms.js
- /usr/lib/node_modules/cordova/node_modules/cordova-lib/src/plugman/install.js
- /usr/lib/node_modules/cordova/node_modules/cordova-lib/src/plugman/plugman.js
- /usr/lib/node_modules/cordova/node_modules/cordova-lib/cordova-lib.js
- /usr/lib/node_modules/cordova/src/help.js
- /usr/lib/node_modules/cordova/src/cli.js
- /usr/lib/node_modules/cordova/bin/cordova
Unhandled error. ('The platform "android" does not appear to be a valid cordova platform. It is missing API.js. android not supported.')

Laubeee · 2020-06-02T07:28:19Z

even if my JAVA_HOME points to jdk1.8

Improvements were made with how java is detected, so this may be fixed in cordova-android@nightly. Can you test? We are looking to release cordova-android@9 soon, but I can't give any specific ETA.

Hey, sorry, I started editing my original post but it seems I forgot to actually post it (or didnt work)

I got it to work. there was still a ref to javac 11 in the path before 1.8 (but not for java), so when I rearranged some more path vars it worked.
Looking forward to see this handled in a better way in the next release! thx

timbru31 · 2020-06-08T12:42:56Z

bin/templates/cordova/lib/check_reqs.js

@@ -393,11 +393,13 @@ module.exports.run = function () {
        console.log('ANDROID_SDK_ROOT=' + process.env['ANDROID_SDK_ROOT'] + ' (recommended setting)');
        console.log('ANDROID_HOME=' + process.env['ANDROID_HOME'] + ' (DEPRECATED)');

-        if (!String(values[0]).startsWith('1.8.')) {
+        if (values[0] === undefined) {


Wouldn't this, theoretically, allow JDK <8, too? They would, e.g., start with 1.6 for a JDK6. We should maybe use something like this in order to check for >= 52

Agreed, we should still throw an error for JDK < 8. We might be able to use semver. We already depend on it IIRC.

The problem is that in Java <=1.8 it’s labeled as 1.x while Java 9 and newer dropped the 1.. This complicates the check a little bit.

Well, using semver it would still only entail checking that we satisfy >=1.8.

Indeed, 14 > 1.8, so this could work/be a sufficient check.

With semver the following should work:

Suggested change

if (values[0] === undefined) {

if (!semver.satisfies(semver.coerce(values[0]), '>= 1.8')) {

brody4hire · 2020-06-12T18:02:06Z

I would like to get this fixed, would definitely agree with @timbru31 to check for JDK < 8.

raphinesse · 2020-06-16T21:09:32Z

bin/templates/cordova/lib/check_reqs.js

            );
+        } else {
+            console.log('Detected JDK version: ' + values[0] + '\n');


I'm not sure if we want to add this output. If so, it needs to use events like everything else.

I would prefer this text to be verbose output, personally.

raphinesse · 2020-06-16T21:13:43Z

It would be great if we could finally allow builds using the latest SDKs!

I remember that I thought this was possible quite some time ago. But when I did some tests in preparation of a PR it appeared as if the newer SDKs did break the build after all. Maybe I just messed up something else instead. I really hope so.

brody4hire · 2020-06-16T21:32:21Z

As I said in this thread on the mailing list([1]), I was able to use OpenJDK 13 to build and install a couple of non-Cordova Android apps from the command line, including a React Native project. (OpenJDK 13 also worked for me with building and installing https://github.com/sqlcipher/sqlcipher-android-tests from the command line.)

A side point is that we are using Gradle a little differently than most other Android projects, as discussed in the same thread ([1]).

So yes, I think we should update to allow JDK > 1.8, properly, with proper review and testing. I took the liberty to update the title again to better reflect the actual change proposed here.

[1] https://lists.apache.org/thread.html/ra54447f23a6df77128dbec69ad3130281221e8a1b724c5ceb02052db%40%3Cdev.cordova.apache.org%3E

raphinesse · 2020-07-03T14:17:38Z

Here's the problem I encountered and still do when using Java > 8:

$ echo $JAVA_HOME
/usr/lib/jvm/java-11-openjdk-amd64

$ sdkmanager --version # same Exception for avdmanager and android
Exception in thread "main" java.lang.NoClassDefFoundError: javax/xml/bind/annotation/XmlSchema
	at com.android.repository.api.SchemaModule$SchemaModuleVersion.<init>(SchemaModule.java:156)
	at com.android.repository.api.SchemaModule.<init>(SchemaModule.java:75)
	at com.android.sdklib.repository.AndroidSdkHandler.<clinit>(AndroidSdkHandler.java:81)
	at com.android.sdklib.tool.sdkmanager.SdkManagerCli.main(SdkManagerCli.java:73)
	at com.android.sdklib.tool.sdkmanager.SdkManagerCli.main(SdkManagerCli.java:48)
Caused by: java.lang.ClassNotFoundException: javax.xml.bind.annotation.XmlSchema
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
	... 5 more

$ export JAVA_HOME=/usr/lib/jvm/java-8-openjdk-amd64

$ sdkmanager --version
26.1.1

An explanation of the Exception can be found at stackoverflow. Unfortunately there seems to be no workaround for Java 11. It might work with later Android SDK tools. But I don't know what we support exactly.

This issue does not have to block this PR, since users should be able to use latest JDK if it works for them and the SDKs they are using. But we should maybe have an idea of what will and what will not work together, so we can estimate the impact of allowing newer JDK versions and maybe warn users or catch errors appropriately.

breautek · 2020-07-03T14:32:45Z

Strange. Is it possible you're still using the old command line tools?

Android now offers command line tools as a separate package that you need to install. Perhaps this is updated for java 11+ support.

https://stackoverflow.com/a/58652345/4685664

raphinesse · 2020-07-03T14:56:26Z

@breautek Yeah, it's been some time since I did Android development, so I still have the tools that come with the SDK (or you install via the sdkmanager?). I did not know there were new tools. If this is an unsupported or deprecated setup we don't have to worry. Hopefully we call the right tools from cordova-android if there are multiple versions. Probably depends on the user's PATH setup etc.

breautek · 2020-07-03T14:58:47Z

I haven't been doing a whole lot of mobile development stuff at my workplace lately, but I'll install JDK 11 and see what kind of issues I run into with. My SDK environment is relatively up to date with everything as well.

Edit: Actually I'm seeing similar issues running JDK 11 as well, even Android Studio GUI doesn't work properly for me when I have JAVA_HOME pointing to JDK 11.

Not sure why my fork had all the tests passing, perhaps most of these android studio calls are mocked...?

https://github.com/breautek/cordova-android/runs/676508506?check_suite_focus=true

Edit 2:

Yah, quick glance at the unit tests, it looks like most android sdk calls are mocked. So it looks like my original interpretation of Android SDK strictly supports java 8 is correct I think...

breautek · 2020-07-08T12:59:17Z

Thank you for your PR, but I'm certain that Android SDK tools doesn't play nicely with any java version above 8.

You can get Java11 working with the android tools, but it requires some hackery. Depending on this is not an acceptable solution, it should work out of the box.

So unfortunately, I don't see supporting java11 in the foreseeable future. This is something we can return to once Android Studio/Android SDK Tools supports java11, or greater.

raphinesse · 2020-07-08T22:05:29Z

Just some more details from my side:

Even the latest (6609375) Android Command Line Tools did not work properly for me using Java 11. While avdmanager seemed to work, sdkmanager always only showed the usage help preceded by this error:

Warning: Could not create settings
java.lang.IllegalArgumentException
	at com.android.sdklib.tool.sdkmanager.SdkManagerCliSettings.<init>(SdkManagerCliSettings.java:428)
	at com.android.sdklib.tool.sdkmanager.SdkManagerCliSettings.createSettings(SdkManagerCliSettings.java:152)
	at com.android.sdklib.tool.sdkmanager.SdkManagerCliSettings.createSettings(SdkManagerCliSettings.java:134)
	at com.android.sdklib.tool.sdkmanager.SdkManagerCli.main(SdkManagerCli.java:57)
	at com.android.sdklib.tool.sdkmanager.SdkManagerCli.main(SdkManagerCli.java:48)

Benji1 · 2020-09-09T12:31:03Z

I wanted to switch from an old docker image to circleci/android:api-28-node but they recently switched to openjdk 11 and in that article there is a link to another article, which claims that:

Even Android Studio and the Android SDK — one of the last holdouts — works with OpenJDK 11 out of the box.

So I wanted to know if maybe @aahlenst (who wrote this article) has a trick up his sleeve and can show us what could be done to make it work.

aahlenst · 2020-09-09T13:49:49Z

To make things work:

You need the new commandlinetools, 6514223 or newer should work.
You need to place the tool directory inside in /opt/android/cmdline-tools (when /opt/android is the SDK root).
You need to update the paths, especially /opt/android/cmdline-tools/tools/bin (when /opt/android is the SDK root) must come before everything else. This is because there are 2 sdkmanager binaries in the complete SDK and the old one in tools/bin does not work. It produces the stacktrace quoted above.

Install instructions:

$ unzip commandlinetools-linux-6514223_latest.zip 
$ mkdir -p /opt/android/cmdline-tools
$ mv tools /opt/android/cmdline-tools
$ export PATH="/opt/android/cmdline-tools/tools/bin:$PATH"

Then, use sdkmanager to install all required dependencies.

PATH configuration:

export PATH="/opt/android/tools:$PATH"
export PATH="/opt/android/tools/bin:$PATH"
export PATH="/opt/android/platform-tools:$PATH"
export PATH="/opt/android/emulator:$PATH"
export PATH="/opt/android/cmdline-tools/tools/bin:$PATH"

export ANDROID_SDK_ROOT="/opt/android"

Benji1 · 2020-09-09T14:46:43Z

Thanks for the fast answer!

GuilleW · 2020-09-09T18:36:08Z

I use a faster way, just sed the check_reqs.js file to really follow the JAVA_HOME env variable:

Fix JAVA_HOME not detected: #928

sed -i 's/\x27javac -version/process.env\[\x27JAVA_HOME\x27\] + \x27\/bin\/javac -version/g' src-cordova/platforms/android/cordova/lib/check_reqs.js

I run this command from my VueJS project folder. You should probably fix the path.

dkotrada added 3 commits March 13, 2020 10:44

Update check_reqs.js

a008a10

Checking JDK should not fail when there is another version installed like 11.0.6 for example. Redme file declare clearly > Requires > Java JDK 1.8 or greater

Update check_reqs.js

27f8718

Indentation fix

Update check_reqs.js

0110759

fix trailing spaces

jcesarmobile closed this Mar 13, 2020

jcesarmobile reopened this Mar 13, 2020

timbru31 requested changes Jun 8, 2020

View reviewed changes

brody4hire added the bug label Jun 12, 2020

brody4hire changed the title ~~Update check_reqs.js~~ Update check_reqs.js - JDK 1.8 or greater Jun 12, 2020

raphinesse reviewed Jun 16, 2020

View reviewed changes

brody4hire changed the title ~~Update check_reqs.js - JDK 1.8 or greater~~ Update check_reqs.js - allow JDK > 1.8 Jun 16, 2020

breautek closed this Jul 8, 2020

breautek mentioned this pull request Nov 20, 2020

support openjdk 11 #1127

Closed

5 tasks

breautek mentioned this pull request Jan 16, 2021

refactor: java checks / get java version #1130

Merged

5 tasks

timbru31 mentioned this pull request Apr 9, 2021

Cannot use Java 11 (or any Java >10) #1195

Closed

	if (values[0] === undefined) {
	if (!semver.satisfies(semver.coerce(values[0]), '>= 1.8')) {

Update check_reqs.js - allow JDK > 1.8 #928

Update check_reqs.js - allow JDK > 1.8 #928

Conversation

dkotrada commented Mar 13, 2020

Motivation and Context

breautek commented Mar 13, 2020

jcesarmobile commented Mar 13, 2020

dkotrada commented Mar 13, 2020 • edited Loading

jcesarmobile commented Mar 13, 2020

breautek commented Mar 13, 2020 • edited Loading

breautek commented Apr 14, 2020

GuilleW commented May 11, 2020 • edited Loading

GuilleW commented May 11, 2020

breautek commented May 11, 2020

GuilleW commented May 11, 2020

breautek commented May 15, 2020

Laubeee commented May 29, 2020 • edited Loading

breautek commented May 29, 2020

GuilleW commented May 29, 2020

$ cordova create hello com.example.hello HelloWorld

$ cd hello

$ cordova platform add android

$ cordova requirements

$ cordova build

Update to nightly

$ npm i cordova-android@nightly

$ cordova build

Laubeee commented Jun 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brody4hire commented Jun 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raphinesse commented Jun 16, 2020 • edited Loading

brody4hire commented Jun 16, 2020

raphinesse commented Jul 3, 2020

breautek commented Jul 3, 2020

raphinesse commented Jul 3, 2020

breautek commented Jul 3, 2020 • edited Loading

breautek commented Jul 8, 2020

raphinesse commented Jul 8, 2020

Benji1 commented Sep 9, 2020

aahlenst commented Sep 9, 2020

Benji1 commented Sep 9, 2020

GuilleW commented Sep 9, 2020 • edited Loading

Fix JAVA_HOME not detected: #928

dkotrada commented Mar 13, 2020 •

edited

Loading

breautek commented Mar 13, 2020 •

edited

Loading

GuilleW commented May 11, 2020 •

edited

Loading

Laubeee commented May 29, 2020 •

edited

Loading

`$ cordova create hello com.example.hello HelloWorld`

`$ cd hello`

`$ cordova platform add android`

`$ cordova requirements`

`$ cordova build`

`$ npm i cordova-android@nightly`

`$ cordova build`

raphinesse commented Jun 16, 2020 •

edited

Loading

breautek commented Jul 3, 2020 •

edited

Loading

GuilleW commented Sep 9, 2020 •

edited

Loading