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

Upgrade JavaFX to v16 #5499

Merged
merged 4 commits into from
May 20, 2021
Merged

Conversation

ripcurlx
Copy link
Contributor

As we have now the possibility again to keep up with the latest bug fixes and updates I've upgraded JavaFX to v16.

It seems to be mainly a bug fix release. All details can be found here: https://github.com/openjdk/jfx/blob/master/doc-files/release-notes-16.md

I did some smoke tests on Mainnet and everything seems to be fine.

@ripcurlx ripcurlx requested review from cd2357 and sqrrm May 17, 2021 09:10
@ripcurlx ripcurlx changed the title Upgrade JavaFX to v16 [WIP] Upgrade JavaFX to v16 May 17, 2021
@ripcurlx ripcurlx changed the title [WIP] Upgrade JavaFX to v16 Upgrade JavaFX to v16 May 17, 2021
@ripcurlx ripcurlx changed the title Upgrade JavaFX to v16 [WIP] Upgrade JavaFX to v16 May 17, 2021
@ripcurlx
Copy link
Contributor Author

There seems to be something that needs to be looked into:

Mai 17, 2021 11:06:04 VORM. com.sun.javafx.application.PlatformImpl startup
WARNUNG: Unsupported JavaFX configuration: classes were loaded from 'unnamed module @51133c06'

This warning has now surfaced for the first time in JavaFX 16 as it implements https://bugs.openjdk.java.net/browse/JDK-8256362

@sqrrm
Copy link
Member

sqrrm commented May 17, 2021

This sounds good to me, it's not a fringe library so keeping it up to date makes sense to me.

@ripcurlx ripcurlx force-pushed the update-javafx-to-16 branch from 87e3e42 to e3842f2 Compare May 18, 2021 15:20
@ripcurlx
Copy link
Contributor Author

Using the javaFX plugin doesn't fix this issue, but it will as soon as this is fixed by the plugin itself (hopefully). See openjfx/javafx-gradle-plugin#108

As this is an issue that exists already on master I think this PR is still good to be merged IMO. @sqrrm WDYT?

@ripcurlx ripcurlx changed the title [WIP] Upgrade JavaFX to v16 Upgrade JavaFX to v16 May 18, 2021
cd2357
cd2357 previously approved these changes May 18, 2021
Copy link
Contributor

@cd2357 cd2357 left a comment

Choose a reason for hiding this comment

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

ACK. Tested on macOS and Linux.

@ripcurlx ripcurlx added this to the v1.6.5 milestone May 19, 2021
@cd2357
Copy link
Contributor

cd2357 commented May 19, 2021

Hmm packaging for linux fails for me with

> Task :desktop:packageInstallers
Executing command:
java -jar "/home/user/git/bisq/desktop/package/tools-1.0.jar" /home/user/git/bisq/desktop/build/libs/fatJar/desktop-1.6.4-SNAPSHOT-all.jar

Command output (stderr):
Exception in thread "main" java.io.IOException: Failed to rename /tmp/Bisq-stripped2587512738202276447.tmp to /home/user/git/bisq/desktop/build/libs/fatJar/desktop-1.6.4-SNAPSHOT-all.jar
at bisq.tools.Utils.renameFile(Utils.java:36)
at io.github.zlika.reproducible.StipZipFile.strip(StipZipFile.java:35)
at bisq.tools.DeterministicBuildTool.main(DeterministicBuildTool.java:24)

Can someone else please confirm? Just to check its not an issue with my local env.

@ripcurlx
Copy link
Contributor Author

Hmm packaging for linux fails for me with

> Task :desktop:packageInstallers
Executing command:
java -jar "/home/user/git/bisq/desktop/package/tools-1.0.jar" /home/user/git/bisq/desktop/build/libs/fatJar/desktop-1.6.4-SNAPSHOT-all.jar

Command output (stderr):
Exception in thread "main" java.io.IOException: Failed to rename /tmp/Bisq-stripped2587512738202276447.tmp to /home/user/git/bisq/desktop/build/libs/fatJar/desktop-1.6.4-SNAPSHOT-all.jar
at bisq.tools.Utils.renameFile(Utils.java:36)
at io.github.zlika.reproducible.StipZipFile.strip(StipZipFile.java:35)
at bisq.tools.DeterministicBuildTool.main(DeterministicBuildTool.java:24)

Can someone else please confirm? Just to check its not an issue with my local env.

Packaging on Ubuntu 20.04.2 worked just fine for me.

@ripcurlx
Copy link
Contributor Author

Ok - as I wasn't able to create a universal deterministic JAR I'll revert my changes back to OS specific jars.

Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

utACK

@sqrrm sqrrm merged commit 31d37db into bisq-network:master May 20, 2021
@ripcurlx ripcurlx deleted the update-javafx-to-16 branch July 16, 2021 07:14
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.

3 participants