renpy / renpy-build

Build system for the Ren'Py visual novel engine. (The engine itself, not games.)
76 stars 51 forks source link

JAVA_HOME is not recognized on Linux and macOS due to hardcoded javac.exe check in plat.py > maybe_java_home #104

Closed hsandt closed 10 months ago

hsandt commented 11 months ago

Context

I was testing Renpy nightly (in order to check https://github.com/renpy/renpy/issues/5137) which uses Java 17. instead of Java 8, and therefore needs to be run with Java 17 to avoid this version check error:

CalledProcessError: Command '['java', '-classpath', '/home/leyn/Applications/Game Engines/renpy-nightly-sdk/rapt/buildlib', 'CheckJDK', '17']' returned non-zero exit status 1.

This can be done with sudo update-alternatives --config java on Linux, but since I wanted to keep Java 8 as default for Renpy stable to work, I preferred running Renpy nightly with temporary env variable JAVA_HOME:

# example using OpenJDK 17, where renpy-nightly.sh is a symlink to renpy.sh in a nightly install of renpy-sdk
$ JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64 renpy-nightly.sh

then selected Install SDK again. However, JAVA_HOME was not recognized, therefore the default Java version 8 was used, and renpy-nightly failed on the JDK 17 check exactly as if I had not defined JAVA_HOME at all:

CalledProcessError: Command '['java', '-classpath', '/home/leyn/Applications/Game Engines/renpy-nightly-sdk/rapt/buildlib', 'CheckJDK', '17']' returned non-zero exit status 1.

Cause

I identified the issue in this file: renpy-nightly-sdk/rapt/buildlib/rapt/plat.py line 44:

def maybe_java_home(s):
    ...
    if "JAVA_HOME" in os.environ:
        if not os.path.exists(os.path.join(os.environ["JAVA_HOME"], "bin", "javac.exe")):

which is incorrectly checking a hardcoded Windows .exe. The change was introduced in commit https://github.com/renpy/renpy-build/commit/c820d9b14eb8ad083c33184e7c957d3a571d850e

As a quick fix, I replaced the line with:

        if not os.path.exists(os.path.join(os.environ["JAVA_HOME"], "bin", "javac")):

and it fixed the issue on Linux.

For a more generic fix that still works on Windows though, we could either check a prepare string variable that could be "javac.exe" or "javac" depending on the OS, or even simpler, reuse the binary filename passed as argument s:

        if not os.path.exists(os.path.join(os.environ["JAVA_HOME"], "bin", s)):

Since it's also used in the return line below:

        return os.path.join(os.environ["JAVA_HOME"], "bin", s)

we could also precompute some variable s_full_path = os.path.join(os.environ["JAVA_HOME"], "bin", s) and reuse it in both locations (personally I'd rename s to something like binary_name or executable_filename to make things clearer too).

That sounds like an easy PR but I haven't cloned the renpy-build repository (unlike renpy-sdk) so I may not be able to send it immediately. So if somebody else who already cloned the repo feels like it, they can also make that PR to be even faster.