Respect JAVA_HOME environment variable

[andreoss]

Closes #26, also related to #6

[katrinsharp]

Hi @andreoss. How is your PR different from #43 ? Thank you.

[cstroe]

@katrinsharp This PR does an additional explicit lookup for the JAVA_HOME environment variable if the java.home system property is not set.

@andreoss To my knowledge, looking at the JAVA_HOME environment variable is not required. The java.home system property is always set by the JVM, so if your JVM program is running, the location of the JVM should be available from the java.home, no need to look elsewhere.

Maven launcher scripts handle JAVA_HOME to find which JVM to execute, but once scala-maven-plugin is launched, it should only look at the java.home system property.

[cstroe]

As it was mentioned in another PR, we should be using ToolchainManager lookups to find the JVM: #43 (comment)

More info on Maven Toolchains.

The main benefit:

With Maven Toolchains applied to JDK toolchain, a project can now be built using a specific version of JDK independent from the one Maven is running with.

This might help when doing JDK upgrades of projects that use scalatest-maven-plugin.

[andreoss]

@andreoss To my knowledge, looking at the JAVA_HOME environment variable is not required. The java.home system property is always set by the JVM, so if your JVM program is running, the location of the JVM should be available from the java.home, no need to look elsewhere.

Maven launcher scripts handle JAVA_HOME to find which JVM to execute, but once scala-maven-plugin is launched, it should only look at the java.home system property.

You might be right. But surefire seems to query for JAVA_HOME before java.home
https://github.com/apache/maven-surefire/blob/master/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/SurefireLauncher.java#L77
My guess is that this approach is chosen because it's less bug-prone, since System properties is a mutable collection, and value for java.home can be altered. Unlike immutable environment variables.

Also there could be cases when using that the same JVM which was used for maven is not always desirable. The standard mvn script seems to always set JAVA_HOME, but in case maven was started from IDE it may not be so.

[cstroe]

@andreoss Yep, it doesn't hurt to use JAVA_HOME as a backup as far as I know. 👍

[katrinsharp]

LGTM

[cerveada]

Hello, there seems to be some confusion

You might be right. But surefire seems to query for JAVA_HOME before java.home
https://github.com/apache/maven-surefire/blob/master/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/SurefireLauncher.java#L77

This is an integration test code, not code used by surefire itself. In fact surefire uses java.home. I can share a link to simple project that I used to test that if anyone is interested.

@andreoss Yep, it doesn't hurt to use JAVA_HOME as a backup as far as I know. 👍

But that is not what is done in the PR. JAVA_HOME is the defualt and the java.home is a backup.

Please take a look at #80 where I described why it seems to me the default should be java.home.