Skip to content
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

[WIP] First attempt for JDK11 compilation - #Java11 #3421

Merged
merged 132 commits into from
Aug 25, 2019
Merged

[WIP] First attempt for JDK11 compilation - #Java11 #3421

merged 132 commits into from
Aug 25, 2019

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented Nov 9, 2017

This addresses #2594.

I played around a bit with the jigsaw module stuff.
Does not yet compile complete, but it's a step in the right direction.
If anyone is interested in this stuff, please move ahead.

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@koppor
Copy link
Member

koppor commented Nov 10, 2017

It seems, we are going to ship different jars for different JREs. Think, we have to continue working on #3180 to get this done?!

@Siedlerchr
Copy link
Member Author

Yes, however, there is also the experimental gradle stuff to create multi release jar. https://github.com/FrVaBe/Java9/tree/master/multi-release-jar

@koppor koppor changed the title [WIP] First attempt at jdk9 compilation [WIP] First attempt for JDK9 compilation - #Java9 Nov 22, 2017
@koppor koppor added the jabcon label Nov 22, 2017
@lenhard
Copy link
Member

lenhard commented Dec 11, 2017

One of our core problems here at the moment seems to be a split package problem, i.e. several of the modules we use (open office stuff, jgoodies, ...) export the same package from different modules. When that happens, Java will report an error of the form

Error:java: the unnamed module reads package com.sun.star.util from both ridl and unoil

I am still looking for a solution to this that is not too much effort. One seems to be to package the two modules together manually into a single module that JabRef could depend upon. That is absolutely no solution for us, because it means that we would have to repackage tons of outdated and no longer maintained libraries.

@Siedlerchr
Copy link
Member Author

Well, at least jgoodies will be removed by your other PR.
Stay some other moduels. According to the jdk9 documentaiton, when they are unnamed modules, they can still be split packages.
http://www.logicbig.com/tutorials/core-java-tutorial/modules/split-packages/

A better toolfor identifying split classes:
https://github.com/AdoptOpenJDK/jsplitpkgscan

In general:
https://dzone.com/articles/the-legacy-developers-guide-to-java-9

@lenhard
Copy link
Member

lenhard commented Dec 11, 2017

A hands-on guide that has helped me to resolve some problems with internal accesses:

https://blog.codefx.org/java/java-9-migration-guide

@koppor koppor mentioned this pull request Feb 15, 2018
8 tasks
@florian-beetz
Copy link
Contributor

Hey, I'm a student of @stefan-kolb and I'm planning to write my Bachelor's Thesis on Java 9 Migration. I'd like to have a look on this.

@lenhard
Copy link
Member

lenhard commented Feb 16, 2018

I am glad to hear that! Just a remark: Although we try to add module files here, the module system is not our first priority when it comes to Java 9. Getting rid of all accesses to internal libraries is the initial obstacle. So, as a start, you might want to look at that problem.

@koppor koppor mentioned this pull request Feb 20, 2018
5 tasks
@Siedlerchr
Copy link
Member Author

With the removal of our customjfx component in the future #3779 we will be one step closer to no internal dependencies. I think we should consider basing this branch on the maintable beta?

build.gradle Outdated
@@ -24,7 +24,8 @@ plugins {
id "de.sebastianboegl.shadow.transformer.log4j" version "2.1.1"
id "com.simonharrer.modernizer" version '1.5.0-1'
id 'me.champeau.gradle.jmh' version '0.4.3'
id 'net.ltgt.errorprone' version '0.0.13'
id 'com.zyxist.chainsaw' version '0.1.3'
//id 'net.ltgt.errorprone' version '0.0.13'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we support error prone any more? This should be commented inside build.gradle

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at that time it did not work. Therefore I uncommented that. But have no clue if it's just the version or module stuff.

@Siedlerchr
Copy link
Member Author

Another interesting aspect: Dependencies on JAVA EE modules ( java.mail for example) etc are soon no longer part of JAVA SE. http://openjdk.java.net/jeps/320 The good news: Nearly all EE modules are available as reference implementation on github and can be integrated as an example.
https://javaee.github.io/javaee-spec/Specifications

@florian-beetz
Copy link
Contributor

I currently have a version that compiles with Java 9 in my fork, where I temporarily removed the dependencies with split packages.

However I've got some new problems:

  • When I try to run JabRev with the gradle run goal, it seems like no resources can be read. Log4j prints a warning that it is unconfigured and the ResourceBundles are null.

  • When I run the assemble goal and run the JAR manually JabRef starts, but opening FX dialogs does not work, because the PostConstruct annotation can not be found. It works with the --add-modules=java.xml.ws.annotation flag, but I can't add it to the module-info, because gradle depends on a library also providing these annotations.

I'd really appreciate if someone could have a look into this.

@Siedlerchr
Copy link
Member Author

Hi, I am not that deep into that, but I think in such cases you need to specify
But this might help:
https://discuss.gradle.org/t/java-9-build-9-ea-170-issues/22767/8

@florian-beetz
Copy link
Contributor

Hey, thanks for your help.

I just tried that, but this seems to just add the modules to the module path during compilation, but not actually at runtime.

@tobiasdiez
Copy link
Member

@florian-beetz That are already good news. Nice work!
Which class or library uses PostConstruct? Is it only afterburner.fx? If yes, then I'll have a look at it in the coming days since I'm currently in the progress of partly rewriting that library.

@florian-beetz
Copy link
Contributor

@tobiasdiez Thank you.
As far as I could tell it is afterburner.fx and Guava, that have a dependency on com.google.code.findbugs:jsr305.

@tobiasdiez
Copy link
Member

tobiasdiez commented Mar 18, 2018

At least for guava, this should be fixed/fixable if I understand google/guava#2782 (comment) correctly.

@florian-beetz
Copy link
Contributor

I was able to fix the JSR 305 problem now.
Guava's dependency on JSR 305 is actually optional, but was made a runtime dependency due to some special use case (google/guava#2721), so explicitly excluding the dependency helped.

Also there is currently an open Guava issue to replace JSR 305 completely google/guava#2960

@stefan-kolb
Copy link
Member

@florian-beetz Nice 👍 Would you like to share your current progress via a PR with us?

@florian-beetz
Copy link
Contributor

@stefan-kolb Sure, should I just open a new one? I can't seem to add my changes to this one.

@stefan-kolb
Copy link
Member

stefan-kolb commented Mar 24, 2018

I guess so, we can close this one then in favor of your new one! You have all the changes of this one in your branch?!

@Siedlerchr
Copy link
Member Author

@stefan-kolb Maybe we can give him push rights for the branches here?

@stefan-kolb
Copy link
Member

Is that possible? Would also be fine but I guess it complicates things unecessesarily.

@florian-beetz florian-beetz mentioned this pull request Mar 24, 2018
6 tasks
@Siedlerchr
Copy link
Member Author

@Siedlerchr Siedlerchr closed this Aug 25, 2019
@Siedlerchr Siedlerchr reopened this Aug 25, 2019
@koppor
Copy link
Member

koppor commented Aug 25, 2019

@Siedlerchr We already have that in: https://github.com/JabRef/jabref/blob/jdk9/build.gradle#L111

The issue currently is:

Module org.testfx contains package org.assertj.core.error.future, module org.assertj.core exports package org.assertj.core.error.future to org.testfx

In other words:

  • package A is contained in package B
  • package A is exported by package C

with A being org.assertj.core.error.future, B being org.testfx, C being org.assertj.core

This is a contradition

@koppor
Copy link
Member

koppor commented Aug 25, 2019

New dependencies can introuce spooky issues: #5217

@koppor
Copy link
Member

koppor commented Aug 25, 2019

All tests green. GuiTests are properly ignored now.

Before that, we had || true do allow failures, but due to some gradle magic for forking, it was hanging:

To honour the JVM settings for this build a new JVM will be forked. Please consider using the daemon: https://docs.gradle.org/5.6/userguide/gradle_daemon.html.

Starting process 'Gradle build daemon'. Working directory: /home/travis/.gradle/daemon/5.6 Command: /home/travis/oraclejdk11/bin/java --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.lang.invoke=ALL-UNNAMED --add-opens java.prefs/java.util.prefs=ALL-UNNAMED -XX:MaxMetaspaceSize=256m -XX:+HeapDumpOnOutOfMemoryError -Xms256m -Xmx512m -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant -cp /home/travis/.gradle/wrapper/dists/gradle-5.6-bin/ez4di095eg6cfomw17rqphhsg/gradle-5.6/lib/gradle-launcher-5.6.jar org.gradle.launcher.daemon.bootstrap.GradleDaemon 5.6

Successfully started process 'Gradle build daemon'

An attempt to start the daemon took 1.717 secs.

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received

The build has been terminated

@koppor koppor merged commit f85c081 into master Aug 25, 2019
@koppor koppor deleted the jdk9 branch August 25, 2019 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants