TheInfiniteKind / appbundler

72 stars 24 forks source link

JDK lookup breaks after homebrew openjdk change #66

Open joewiz opened 4 years ago

joewiz commented 4 years ago

Homebrew recently added an openjdk recipe, which is installed automatically as a dependency of Java-based command-line utilities like ant and maven. Specifically, the new homebrew recipe installs OpenJDK 13 from openjdk.net and symlinks this version to /Library/Java/JavaVirtualMachines/openjdk.jdk.

This change causes appbundler-based apps to fail to discover the JDK, and raise the dreaded error:

Screen Shot 2020-02-13 at 10 38 53 PM

I believe this issue is caused by the absence of a version number in the JDK's filename in /Library/Java/JavaVirtualMachines/openjdk.jdk. Appbundler expects to find a version number in the JDK's filename, e.g., jdk-12.0.1.jdk or zulu-8.jdk; see https://github.com/TheInfiniteKind/appbundler/blob/master/appbundler/native/main.m#L837-L938.

Renaming the homebrew JDK symlinked filename to openjdk-13.0.2.jdk fixes the issue, but ideally we could adjust appbundler to recognize openjdk.jdk as a valid JDK.

To reproduce, run brew install openjdk, brew cask install exist-db, and then start /Applications/eXist-db.app. If eXist starts, be sure no other symlinked JDKs are in /Library/Java/JavaVirtualMachines. (I'm even able to reproduce the error with zulu-8.jdk present, so it seems appbundler's code really trips up on openjdk.jdk when it tries to parse it.)

// cc: @reitermarkus

reitermarkus commented 4 years ago

Using the java_home command definitely works correctly:

$ sudo ln -sfn /usr/local/opt/openjdk/libexec/openjdk.jdk /Library/Java/JavaVirtualMachines/openjdk.jdk
$ ls /Library/Java/JavaVirtualMachines
openjdk.jdk
$ /usr/libexec/java_home -v '1.8+'
/Library/Java/JavaVirtualMachines/openjdk.jdk/Contents/Home
FranklinYu commented 4 years ago

I think we should update the function findJREDylib() to use java_home instead of the deprecated “Internet Plug-Ins”. java_home should also be able to resolve #59.

@reitermarkus Does Apple recommend tools to make use of java_home or is it meant to be used only by Apple internally? Is it documented anywhere?

FranklinYu commented 4 years ago

Wait, by default jrePreferred is false:

https://github.com/TheInfiniteKind/appbundler/blob/98113669e127c828941947d85f2e4cef5eefbf19/appbundler/src/com/oracle/appbundler/AppBundlerTask.java#L84

So JDK should have been searched:

https://github.com/TheInfiniteKind/appbundler/blob/98113669e127c828941947d85f2e4cef5eefbf19/appbundler/native/main.m#L740-L752

And findJDKDylib() does use java_home. What went wrong? It should have found the path.

reitermarkus commented 4 years ago

For the formula it is irrelevant what Apple recommends regarding java_home, since we cannot install to the location java_home uses without sudo.

reitermarkus commented 4 years ago

If you create the /Library/Java/JavaVirtualMachines/openjdk.jdk symlink as mentioned in the caveats, java_home will find it, however.

FranklinYu commented 4 years ago

Found more by reading the source. It is already using java_home, but it thinks Java path looks like this:

https://github.com/TheInfiniteKind/appbundler/blob/98113669e127c828941947d85f2e4cef5eefbf19/appbundler/native/main.m#L890-L892

In addition, Java version is decided by folder name (like jdk-12.0.1.jdk). Is there a better way to tell the Java version?

reitermarkus commented 4 years ago

You can run e.g. /usr/libexec/java_home -v '12' to get version 12.

rschmunk commented 4 years ago

I'm the one who a few years ago submitted some of the changes to IK's appbundler for treating jrepreferred vs jdkpreferred, and also trying to parse the JDK version. And yes, it needs to be cleaned up, whether due to Oracle's change in Java numbering but more likely due to the way the numerous alternative Javas name their folders.

As @FranklinYu notes, trying to parse the Java version based on folder name has become progressively more difficult. So as @reitermarkus suggests, it seems that calling java_home -v NN could provide an easier to manage result.