Skip to content
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

Java version check does not print detected version to console #623

Closed
fabiante opened this issue Jan 17, 2019 · 7 comments
Closed

Java version check does not print detected version to console #623

fabiante opened this issue Jan 17, 2019 · 7 comments

Comments

@fabiante
Copy link
Contributor

Hi!

Disclaimer: I am unsure if this is the correct repository, feel free to guide me to the correct spot if I am lost 😉

So, When using Cordova and having Java 11 and Java 8 installed on my Windows machine, running cordova emulate android will output this error many people know:

ANDROID_HOME=C:\Users\fabit\AppData\Local\Android\Sdk
JAVA_HOME=C:\Program Files\Java\jdk1.8.0_202
Requirements check failed for JDK 1.8

I then thought "hmm, but my JAVA_HOME is pointing to the correct JDK, version 8.
Running the command with --verbose showed my that in fact Cordova checks not the ENV variable but some other. This happens in this function:

This can be found here:

// Returns a promise.
module.exports.run = function () {
return Q.all([this.check_java(), this.check_android()]).then(function (values) {
console.log('ANDROID_HOME=' + process.env['ANDROID_HOME']);
console.log('JAVA_HOME=' + process.env['JAVA_HOME']);
if (!String(values[0]).startsWith('1.8.')) {
throw new CordovaError(`Requirements check failed for JDK 8 ('1.8.*')`);
}
if (!values[1]) {
throw new CordovaError('Requirements check failed for Android SDK');
}
});
};

As you can see, the check is happening on values[0], not the JAVA_HOME ENV variable.

This is misleading and caused me (and propably others) to uninstall the correct Java version and still getting the error since I didn't suspect Cordova picking up Java 11.

@fabiante
Copy link
Contributor Author

I added a console.log command in the linked PR.

@janpio
Copy link
Member

janpio commented Jan 17, 2019

Ugh, why does it output that ENV var there then at all? Do you have a suggestion for better output here that would actually help users to understand what is going on? (besides just outputting the compared value like in your PR?)

@fabiante
Copy link
Contributor Author

Well, I could rewrite it to display "Checking your Java and Android SDK version". Printing the ENV variable isn't correct.

Looks like in my case, there was more to it, i fixed my error:

As mentioned above I have Java 11 and 8 installed. What I just now found was, that I hat a PATH ENV entry pointing to my Java 11 installation. In some way this ended up in the called function, not my JAVA_HOME ENV variable. Figuring this out will take a while, but isn't related to this issue.

@brodycj
Copy link

brodycj commented Jan 17, 2019

I think it is good to log both the JAVA_HOME setting and the result of javac -version to help the user figure out what is going wrong.

@fabiante
Copy link
Contributor Author

@brodybits Theoretically it is. But what I have discovered is that these may not correspond to what Cordova checks. Both my ENV variables and the javac --version showed the 1.8 installation. Yet somehow Cordova used my OpenJDK 11 installation.

@brodycj
Copy link

brodycj commented Jan 17, 2019

Both my ENV variables and the javac --version showed the 1.8 installation. Yet somehow Cordova used my OpenJDK 11 installation.

Got it now. I think that comment belongs on PR #624 where we are reviewing the actual changes.

@fabiante
Copy link
Contributor Author

Thank you for your feedback!
I'd be happy to clean up the check_req.js once Cordova 9 is out. But that should be another issue & PR I think 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants