-
Notifications
You must be signed in to change notification settings - Fork 399
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
Java 9 build targets Java 8 but does not link against Java 8 runtime #2803
Comments
Unable to repro #2801 with intellij on latest master (tried to connect to lobby, was success). We may want to check release artifacts, otherwise seems like everyone is good (?) 🤔 |
@DanVanAtta It only occurs with the actual release artifacts not building/running the code from an IDE. |
Just to clarify... It only occurs when building from Gradle with JDK 9. You should be able to reproduce it locally if you change The last paragraph in the issue description attempts to explain why one doesn't observe the problem when building from Eclipse even though Eclipse uses a Java 9 compiler. Presumably, IntelliJ has a similar setup where it's setting In fact, I was able to reproduce the problem in Eclipse by changing the project's JRE to JRE 9 and rebuilding it. Then I changed the launch configuration to run it using JRE 8. There's probably a way to do the same thing in IntelliJ if you really want to convince yourself. 😄 |
Just repeating an answer from a different thread to ensure this is followed-up... My understanding is that the Java 9 build was added to detect Java 9 migration issues early so we don't deal with them all at once. @RoiEXLab, please elaborate if I'm mistaken. |
I looked into this a bit more... It turns out that, as postulated above, adding However, there is one caveat. The To prototype this, I added jfxrt.jar to my fork of the diff --git a/game-core/build.gradle b/game-core/build.gradle
index b77fa20f4..2de5a1418 100644
--- a/game-core/build.gradle
+++ b/game-core/build.gradle
@@ -56,10 +56,15 @@ targetCompatibility = 1.8
tasks.withType(JavaCompile) {
options.compilerArgs += [ '-Xlint:all', '-Xmaxwarns', '1000' ]
-
+
// workaround for: https://github.com/google/error-prone/issues/780
options.compilerArgs += [ '-Xep:ParameterName:OFF' ]
-
+
+ // workaround for https://github.com/gradle/gradle/issues/2510
+ if (JavaVersion.current() >= JavaVersion.VERSION_1_9) {
+ options.compilerArgs += ['--release', '8']
+ }
+
options.incremental = true
options.encoding = 'UTF-8'
}
@@ -99,11 +104,14 @@ dependencies {
compile 'com.yuvimasory:orange-extensions:1.3.0'
compile 'commons-cli:commons-cli:1.4'
compile remoteFile('https://github.com/kirill-grouchnikov/substance/raw/f894ef784a2ac20acf13e6ed2c7c26123399787b/drop/8.0.02/substance-8.0.02.jar')
+ if (JavaVersion.current() >= JavaVersion.VERSION_1_9) {
+ // TODO: update link to point to master branch on triplea-game/assets repo
+ compile remoteFile('https://github.com/ssoloff/triplea-game-assets/raw/add-javafx-8-lib/javafx/jfxrt-1.8.0_181.jar')
+ }
compileOnly 'org.projectlombok:lombok:1.18.0'
runtime remoteFile('https://github.com/kirill-grouchnikov/substance/raw/f894ef784a2ac20acf13e6ed2c7c26123399787b/drop/8.0.02/trident-1.5.00.jar')
-
testCompile 'nl.jqno.equalsverifier:equalsverifier:2.4.5'
testCompile 'org.hamcrest:java-hamcrest:2.0.0.0'
testCompile 'org.mockito:mockito-core:2.19.1'
@@ -170,7 +178,6 @@ task downloadAssets(group: 'release') {
}
}
-
import com.install4j.gradle.Install4jTask
task generateInstallers(type: Install4jTask, dependsOn: [shadowJar, downloadAssets], group: 'release') {
projectFile = file('build.install4j')
@DanVanAtta @RoiEXLab @ron-murhammer Any better ideas other than storing jfxrt.jar in the NB: Hosting jfxrt.jar is only required until we drop support for Java 8. NB 2: This solution should also work with JDK 10 and 11. |
@ssoloff I wonder if installing openjfx and getting the jfxrt from the installation folder would already do the trick. In any case any solution will be redundant for any version after JDK 11, because oracle hands javafx over to the open source community, effectively making JavaFX just "an independent", but yet modern UI framework of many. |
I considered something like that, but whatever solution we employ should work from a local build, as well. It felt a bit convoluted to have to introduce something like an environment variable pointing to JDK 8 to do that. EDIT: But not ruling that out. 😄
Indeed. JavaFX 11 (early access) is already available via Maven Central. |
Reopening, as this is a Java 9+ blocker (and we have a proposed solution). |
This issue was originally raised in #2801.
Our Java 9 build targets Java 8, but is incorrectly linking against the Java 9 runtime rather than the Java 8 runtime. At a minimum, the Java 9 build must specify the
-bootclasspath
option tojavac
(or the Gradle Java plugin equivalent) to point to a Java 8 runtime.We should also investigate using the new
--release
option tojavac
added in Java 9. It basically takes the place of-source
,-target
, and-bootclasspath
. Gradle support for this option is unknown at this time.It would be nice if we can accomplish this in the Travis build without having to download JRE 8 with each build. JDK 8 should already be installed on the Travis build image even when JDK 9 is selected (I believe all that does is change the default JDK used by the build, i.e. what's on the PATH).
The solution also needs to account for dev environments, as devs may choose to use Java 9 to build locally.
Because the location of the Java 8 runtime may differ between Travis and different dev's environments, the solution may require the specification of an environment variable or a Gradle project property to point to the Java 8 runtime.
For devs using Eclipse, I believe there is nothing to be done. IIRC, Eclipse always uses the ECJ compiler it ships with (which is Java 9-compliant in Oxygen). It effectively sets the
-bootclasspath
option based on the project's selected JRE (which should be a Java 8 JRE or JDK). Eclipse devs probably already have their workspaces configured in this way. I'm not sure what, if anything, would need to be done for IntelliJ.The text was updated successfully, but these errors were encountered: