Java 9 support for FXLauncher

abhinayagarwal commented 6 years ago

This PR makes sure FXLauncher works on both 8 and 9 without Multi-Release Jar support

edvin commented 6 years ago

Thank you! Will merge after testing :)

jmerle commented 6 years ago

Do you have an estimate on when we can expect this PR to be merged?

abhinayagarwal commented 6 years ago

@mordechaim You may be right since this isn't tested.

@edvin seems to be busy. If someone is willing to test the changes and come back with feedback, we could move things faster.

mordechaim commented 6 years ago

Take a look at my spinoff from this project. Fully compatible with Java 9.

https://github.com/update4j/update4j

xzel23 commented 5 years ago

This is already 11 months old, and the current LTS version Java 11 is not yet supported. Any chance to get this in? I'd volunteer to resolve conflicts if there's a chance to get this in.

edvin commented 5 years ago

Absolutely, resolve the conflicts and I'll merge :) Sorry, this fell off my radar. Too busy with other projects these days, but if you'll resolve the conflicts I'll merge and do a new release right away.

abhinayagarwal commented 5 years ago

I can most probably look at it sometime this week.

xzel23 commented 5 years ago

I have looked into this and I think there are some things to consider:

Removing use of internal API should be done anyway and shouldn't break compatibility with Java 8.
To have a launcher that runs on Java 9+, both JAXB and JavaFX have to be on the classpath/module path.
This pull request in its current form produces a jar that's not compatible with JDK 8 anymore, so a little more work is needed.
IMHO it should be enough to ensure that fxlauncher works on JDK 8 and JDK 11+ given that Java 9 and 10 are already obsolete and not LTS. Java 8 will still be around for a long time, so it should be supported, and Java 11 is the actual version and the first LTS release to include the module system.

I think the first point should in any case already be adresses. I might find the time in the next days to factor this out so that it is ready for merging. There is however still a small problem: I have not found a solution to set the application name as there doesn't seem to be any public API that exposes this functionality.

edvin commented 5 years ago

OK, good work :) If there isn't a public API to set the app name (sounds strage) we could give it a shot using reflection in a try/catch block perhaps?

xzel23 commented 5 years ago

@edvin I think there's no publich API because it's called from the Application Launcher and it seems no one thought there'd be a need to use a custom launcher.

I have just come home from my holiday. I will try to find the time to create a patch containing those parts of the pull request that IMHO should be merged anyway and will work for both JDK 8 and 9+ (i.e. remove use of private API where a public API is available).

I'd then look at the remainder of the patch to make sure it is compatible with 8 and 9+.

edvin commented 5 years ago

@xzel23 OK, super :)

abhinayagarwal commented 5 years ago

Since @xzel23 is already looking into this, can this PR be declined?

edvin / fxlauncher

Java 9 support for FXLauncher #103