[MRELEASE-851]: javaHome argument is now honored in InvokerMavenExecutor

[prof-schnitzel]

I don't know why this bug does not get fixed. The documentation clearly says that you can use the argument -DjavaHome= to set the path to your jdk, but this parameter is not used anywhere in the code, thereby completely ignored. This is a huge pain for people who are using CI/CD servers (for example jenkins) with multiple jdks installed. In that case the release plugin is not usable.

So I adjusted the InvokerMavenExecutor so that it honors the javaHome argument.

[prof-schnitzel]

@olamy Is there a chance to get this quickly merged and released?

[michael-o]

@prof-schnitzel I will check the issue and then the code and let you know.

[olamy]

@olamy Is there a chance to get this quickly merged and released?

yes there is definitely a chance! I was just waited for other folks to review just in case.

[michael-o]

I have a slight problem with this PR, this it is correct, it is also incomplete. the actual issue says: "javaHome is ignored and inherited unexpected". So this should apply to both invoker (InvokerMavenExecutor) and forked-path (ForkedMavenExecutor).

For the ForkedMavenExecutor the JAVA_HOME env var isn't set.

Please make it consistent.

WDYT?

[prof-schnitzel]

@michael-o Yes, you are right. I never use forked-path, thats why I skipped that. But I added the relevant code lines, so that ForkedMavenExecutor now also respects -DjavaHome.

[michael-o]

@prof-schnitzel , I have another problem, maybe you can share your opinion: The mojo parameters use ${java.home} which does not necessarily is ${env.JAVA_HOME}. I'd either change the documention not to mention JAVA_HOME or use the env. Using the env does not guarantee the value to be present. WDYT?

[prof-schnitzel]

@michael-o Thats correct. ${java.home} is a dynamic property showing you which JRE is running your maven build and is not necessarily the same as ${env.JAVA_HOME}. I would suggest to change the documentation not to mention JAVA_HOME. Using the env would be a breaking change. Using the env wouldn't be consistent since all other default maven plugins are using ${java.home}.

[extern-matthias-kampmeyer]

@michael-o Do you have any idea when this will be released?

[michael-o]

@michael-o Do you have any idea when this will be released?

Not before May. I am waiting for fixes from @slawekjaranowski .

[slawekjaranowski]

@michael-o Do you have any idea when this will be released?

Not before May. I am waiting for fixes from @slawekjaranowski .

Which one?

[michael-o]

@michael-o Do you have any idea when this will be released?

Not before May. I am waiting for fixes from @slawekjaranowski .

Which one?

Maven Shared Utils with the StreamFeeder/Pumper.

[slawekjaranowski]

Maven Shared Utils with the StreamFeeder/Pumper.

Was done apache/maven-shared-utils#113

[michael-o]

Maven Shared Utils with the StreamFeeder/Pumper.

Was done apache/maven-shared-utils#113

Now we need a patch release of m-s-utils?