-
Notifications
You must be signed in to change notification settings - Fork 266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use OpenJDK 11 as default JDK #846
Conversation
Eclair can be built and used on Oracle JDK 1.8 or OpenJDK 11. JavaFX is now embedded in eclair-node-gui and does not need to be installed separately. Limitation: the Splash Screen has been removed from the GUI.
Tell users to download java from https://jdk.java.net/11
Note that running the Jar with OpenJDK 11 will raise the following warning:
AFAICT this is a bit ugly but otherwise has no incidence. |
The capsule issue is known (puniverse/capsule#110). It is indeed ugly but "just" a warning.
which is arguable worse than the warnings we get with our current packaging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Capsule warnings are annoying but are fine for the time being, we will need to find a packaging solution eventually.
README.md
Outdated
You need to first install java, more precisely a [JRE 1.8](http://www.oracle.com/technetwork/java/javase/downloads/jre8-downloads-2133155.html). | ||
|
||
:warning: If you are using the OpenJDK JRE, you will need to build OpenJFX yourself, or run the application in headless mode (see below). | ||
You need to first install java. Eclair will run on [Oracle JRE 1.8](http://www.oracle.com/technetwork/java/javase/downloads/jre8-downloads-2133155.html), [Oracle JDK 11](https://www.oracle.com/technetwork/java/javase/downloads/jdk11-downloads-5066655.html) and [Open JDK 11](https://jdk.java.net/11/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's be more clear:
"You need to install Open JDK 11".... and then.... "Eclair will also run on X Y Z but this is not recommended.
README.md
Outdated
|
||
:warning: If you are using the OpenJDK JRE, you will need to build OpenJFX yourself, or run the application in headless mode (see below). | ||
You need to first install java. Eclair will run on [Oracle JRE 1.8](http://www.oracle.com/technetwork/java/javase/downloads/jre8-downloads-2133155.html), [Oracle JDK 11](https://www.oracle.com/technetwork/java/javase/downloads/jdk11-downloads-5066655.html) and [Open JDK 11](https://jdk.java.net/11/). | ||
We recommend that use use [Open JDK 11](https://jdk.java.net/11/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
We recommend that use use [Open JDK 11](https://jdk.java.net/11/). | |
We recommend that you use [Open JDK 11](https://jdk.java.net/11/). |
@@ -235,6 +235,12 @@ | |||
<version>0.9.8</version> | |||
<scope>test</scope> | |||
</dependency> | |||
<dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need this for our docker tests, I've added a comment in the pom
@@ -127,6 +127,7 @@ | |||
<arg>-unchecked</arg> | |||
<arg>-Xmax-classfile-name</arg> | |||
<arg>140</arg> | |||
<arg>-nobootcp</arg> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This removes the scala jar from the boot classpath and is need to compile scala code on JDK9+
BUILD.md
Outdated
@@ -1,11 +1,13 @@ | |||
# Building Eclair | |||
|
|||
## Requirements | |||
- [Java Development Kit](http://www.oracle.com/technetwork/java/javase/downloads/jdk8-downloads-2133151.html) 1.8u161 or newer | |||
- [Oracle JDK 1.8](http://www.oracle.com/technetwork/java/javase/downloads/jdk8-downloads-2133151.html) 1.8u161 or newer, or [OpenJDK 11](https://jdk.java.net/11/). | |||
- [Maven](https://maven.apache.org/download.cgi) 3.5.4 or newer | |||
- [Inno Setup](http://www.jrsoftware.org/isdl.php) 5.5.9 (optional, if you want to generate the windows installer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inno Setup and the rest of the related build instructions should be removed.
This PR addresses 2 different issues:
From a licensing point of view: using Oracles JDK8 is fine for personal use but not for commercial use, and switching OpenJDK8 is not seamless because of JavaFX.
We could recommend that users install OpenJDK 8, but it is not functionally identical to Oracle's JDK 8 and for example does not include JavaFX which is a problem for GUI users.
See also https://blog.jetbrains.com/idea/2018/09/using-java-11-in-production-important-things-to-know/
From a maintenance point of view: switching to JDK11 makes more sense
Since Java11, Oracle's commercial JDK and Oracle's OpenJDK are functionally the same. JavaFX is now a "clean" separate package, which can be added as a dependency when building the app (which is done in this PR, user's don't have to worry about installing/compiling it).
Java 11 is also a LTS (see Java Support Roadmap), 9 and 10 were not.
For Scala development, it is actually recommended to target JDK11 and not 9 or 10, see Scala JDK compatibility overview
This PR:
java -jar