Migrate to Java 12 - Githubissues

DoctorD1501 / JAVMovieScraper

Scrape XBMC and Kodi movie metadeta and automatically rename files for Japanese Adult Videos (JAV), American Adult DVDs, and American Adult Webcontent

GNU General Public License v2.0

751 stars 161 forks source link

Migrate to Java 12 #286

Closed rainux closed 5 years ago

rainux commented 5 years ago

I know it is a huge change, I originally did this for myself to resolve the annoying issue on HiDPI mode of Windows 10. But since migrate to Java 12 also give us extra benefits:

Support HiDPI mode on Windows 10. It is really annoying to workaround this with Java 8.
(Perhaps) make the project easier to contribute. May it's wrong since I'm not a native Java user, but new features of tools should always be attractive.

So I would like to submit this PR to open a discussion with you guys.

Note: Although gradle run works on Windows, but gradle shadowJar and gradle jar still doesn't work as the original master branch does. It looks like caused by a strange issue in Gradle itself, so we still need a Linux/macOS/WSL to create jar.

Wizell commented 5 years ago

First, thank you lot for the PR.

It's always nice to see other people getting involved

I read thought PR and it looks nice. But I haven't got enought time to test it yet.

I have a question, did you manage to test it with older JVM ?

If possible i would like to retain compatibility with Java 8 (packaged in lastest debian stable) as many linux distro are debian derivatives

I will try to give a try to the patches in the next few days

rainux commented 5 years ago

Hello Wizell, these changes meant to not work with older JVM, since OpenJFX was compiled as a newer class file version.

I see retain compatibility with Java 8 make sense for real world issue. Another approach I would suggest is:

Maintain both Java 8 and Java 12 support in build.gradle by using conditional build instructions.
Set sourceCompatibility to "8.0".

Then we can build the project with both JDK version, make users of older and newer system both happy, by using a single code base for Java 8.

Though I'm not very sure if this is possible in Gradle, but I would like to try later when I have time.

Wizell commented 5 years ago

I looked more into this matter.

For Ubuntu the lastest LTS is using Java 11 and Debian Buster will be shipped with Java 11 as well, for Windows, users have to download Oracle JRE anyway and for MacOS there is many solutions which ships from Java 9 to 12.

So I think raising the minimum version to 11 can be a good compromise as it would allow to move to javafx and retain a large compatibility.

I started the tests using jdk 11 (setting sourceCompatibility to 11.0 .

The build is successful and the app starts.

So far i only saw warnings about reflections used by the current json serializer. WARNING: An illegal reflective access operation has occurred WARNING: Illegal reflective access by com.cedarsoftware.util.io.MetaUtils to field java.lang.Enum.name Do you happen to see those problems as well ?

I will try to update the dependency. Else it will need to be replaced.

Wizell commented 5 years ago

Upgrading the dependency is not enough.

Do you know any good json serializer for java ? i'm thinking to switch to gson but don't know how it fares.

I will make a new branch with this work tomorrow so we can continue the work before merging in master