fix: JAVA_HOME points to JRE directory on OpenJDK8, should instead point to JDK directory

[EmilyFlarionIO]

This is in regards to the fact that in Java 8, the directory structure was such that the actual java binary, was in ${JAVA_HOME}/jre/bin/java instead of ${JAVA_HOME}/bin/java, which was instead just a symlink.

Due to the way we recursively follow the symlinks, we'd end up in the wrong directory for Java 8.
This is ok for locating libjvm.so, as it is found using glob and it is within the jre directory, but causes issues with other expected JDK files, such as headers etc.

I propose in this PR to just add a "legacy-java-compat" feature, which when enabled, will let us check if the final result is the "jre" directory, if it is - we'll just traverse one directory up.
This is feature-gated in case other projects using Java 8 have already made their own workarounds, so we don't make a breaking change.
I've added a test using the Temurin 8 release + clearing the automatic JAVA_HOME env, to ensure the behaviour is fixed.
(Note that this entire problem is only relevant on Java installation methods that don't set JAVA_HOME, such as, when installing openjdk8 using apt)

I've also updated the github actions for checkout and setup-java, but that is not mandatory at all and I can revert that if you prefer

[astonbitecode]

Thanks for the PR.

I will take a look at it, but after briefly checking the changes, I guess there is at least a small typo to fix.

[astonbitecode]

I guess this should be JAVA_HOME?

[EmilyFlarionIO]

Indeed, thank you!

[astonbitecode]

The way I see it, a directory like /usr/lib/jvm/java-8-oracle is the JDK directory and /usr/lib/jvm/java-8-oracle/jre is the JRE one.

Having the env var JAVA_HOME empty means that the Java used by the system is the one where the symlinks end up. For example:

$ which java
/usr/bin/java

$ ls -la /usr/bin/java
/usr/bin/java -> /etc/alternatives/java

$ ls -la /etc/alternatives/java
/etc/alternatives/java -> /usr/lib/jvm/java-8-oracle/jre/bin/java

The system uses the JRE instead of the JDK and thus, there are no headers, similarly, there is no javac:

$ export JAVA_HOME=

$ cargo run --features="build-binary" -- --file javac
Error: JavaLocatorError { description: "Could not find the javac library in any subdirectory of /usr/lib/jvm/java-8-oracle/jre" }

If the user wants to use the JDK instead, they need to set the JAVA_HOME pointing to the JDK:

$ export JAVA_HOME=export JAVA_HOME=/usr/lib/jvm/java-8-oracle

$ cargo run --features="build-binary" -- --file javac
/usr/lib/jvm/java-8-oracle/bin

$ cargo run --features="build-binary" -- --file jni.h
/usr/lib/jvm/java-8-oracle/include

Is there a reason why not to set the JAVA_HOME to the needed value?

[EmilyFlarionIO]

You are describing the issue precisely, I think the only thing missing is that JAVA_HOME is a variable that should point to the JDK directory, not the JRE directory, this just means that the locate_java_home function returns an invalid value for JDK8, and while indeed setting JAVA_HOME manually fixes this, this crate does expose locate_java_home() to the user, meaning(in my opinion only, ofc) it should guarantee a valid JAVA_HOME value, not just a valid dyn library
Had this crate only exposed the locate_jvm_dyn_library function, I wouldn't make this PR.

The way I encountered the issue, for example:

1. I installed openjdk8 via apt.
2. I tried to compile the hdfs-sys crate.
3. That crate depends on java-locator do provider the correct JAVA_HOME variable, so it can set the include path in order to compile libhdfs.
4. Compilation fails.

Up until now I've resolved it by setting JAVA_HOME manually, but this is really something I'd rather not do system-wide, and doing it per project requires me to modify the IDE settings both for the Rust build, the Terminal I use to run tests manually, and for the Run settings, which becomes an annoyance.

If you feel like this is something you don't want to get into that's fine as well, this PR is just a suggestion^

[astonbitecode]

I understand.

However, when the system is actually configured in a way to give the path /usr/lib/jvm/java-8-oracle/jre/bin/java for the java executable (it happens to be the java executable of the JRE in our case), java_locator should follow and return the JAVA_HOME that includes this executable (the JRE) instead of something else. It would not be correct to return the java executable of the JDK because this would not be according to what it is configured in the system...

The best solution IMO, would be to configure the system to use the java executable of the JDK instead. For example, using the update-alternatives:

• Install the java executable of the JDK

$ sudo update-alternatives --install /usr/bin/java java /usr/lib/jvm/java-8-oracle/bin/java 1

• Select the needed java alternative (number 2 in this example)

$ $ sudo update-alternatives --config java
There are 4 choices for the alternative java (providing /usr/bin/java).

  Selection    Path                                            Priority   Status
------------------------------------------------------------
* 0            /usr/lib/jvm/java-21-openjdk-amd64/bin/java      2111      auto mode
  1            /usr/lib/jvm/java-17-openjdk-amd64/bin/java      1711      manual mode
  2            /usr/lib/jvm/java-8-oracle/bin/java              1         manual mode
  3            /usr/lib/jvm/java-8-oracle/jre/bin/java          1081      manual mode

Now you should get:

$ ls -la /etc/alternatives/java
/etc/alternatives/java -> /usr/lib/jvm/java-8-oracle/bin/java

And if you execute java_locator for finding the JAVA_HOME, it should return the JDK:

$ cargo run --features="build-binary"
/usr/lib/jvm/java-8-oracle

$ cargo run --features="build-binary" -- --file jni.h
/usr/lib/jvm/java-8-oracle/include

However, I believe it is much easier if you just pass the JAVA_HOME to the cargo command. For example:

$ JAVA_HOME="/usr/lib/jvm/java-8-oracle" cargo run --features="build-binary"
/usr/lib/jvm/java-8-oracle

$ JAVA_HOME="/usr/lib/jvm/java-8-oracle" cargo run --features="build-binary" -- --file jni.h
/usr/lib/jvm/java-8-oracle/include

How does it sound?

[astonbitecode]

Thinking it twice, we could maybe add a feature like "locate-jdks-only". This way, we could change the implementation of do_locate_java_home to use javac instead of java if the feature is enabled.

Doing so, the JAVA_HOME would be the JDK (/usr/lib/jvm/java-8-oracle).

Would you care implementing this?

[EmilyFlarionIO]

I've moved to your suggestion implementation, let me know if this approach suffices, and on a sidenote thank you for being so responsive!

[astonbitecode]

Thanks for taking the time to implement this.

As you see, some tests failed. I was thinking of a much simpler solution, with no fallbacks: If the feature locate-jdks-only is enabled, then we should just search once, but using the javac instead of java.

It is better to be explicit and for example fail if JDK is not installed in the system, while the user explicitly wants a JDK (by enabling the feature).

[EmilyFlarionIO]

Sorry, missed the specific macos test, updated to use a feature gate for the binary name, with no fallback

[astonbitecode]

Looks good!

Let's merge this, even if the new feature does not have impact for macos. We can take care about this in documentation for now.

Thanks once again.