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

Add export of com.sun.javafx.event to org.jabref #11195

Merged
merged 18 commits into from
Apr 15, 2024
Merged

Add export of com.sun.javafx.event to org.jabref #11195

merged 18 commits into from
Apr 15, 2024

Conversation

koppor
Copy link
Member

@koppor koppor commented Apr 15, 2024

Currently does not fix the issue (tried out locally using jPackage).

I have no clue how to continue.

It is also not about forceMerge, because the issue reporter says he used the release version (where our update of #11170) is not included.

Idea from sshahine/JFoenix#1133 (comment)

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@koppor
Copy link
Member Author

koppor commented Apr 15, 2024

"Brute force" adding to many places also does not help:

diff --git a/build.gradle b/build.gradle
index 553a0c29da..9667908ebc 100644
--- a/build.gradle
+++ b/build.gradle
@@ -394,7 +394,9 @@ compileJava {
         // TODO: Remove access to internal api
         addExports = [
                 // Fix for https://github.com/JabRef/jabref/issues/11188
+                'javafx.base/com.sun.javafx.event' : 'org.controlsfx.controls',
                 'javafx.base/com.sun.javafx.event' : 'org.jabref',
+                'javafx.base/com.sun.javafx.event' : 'org.jabref.merged.module',

                 'javafx.controls/com.sun.javafx.scene.control' : 'org.jabref',
                 'org.controlsfx.controls/impl.org.controlsfx.skin' : 'org.jabref'
@@ -406,6 +408,7 @@ run {
     // TODO: Remove access to internal api
     moduleOptions {
         addExports = [
+                'javafx.base/com.sun.javafx.event' : 'org.jabref',
                 'javafx.controls/com.sun.javafx.scene.control' : 'org.jabref',
                 'org.controlsfx.controls/impl.org.controlsfx.skin' : 'org.jabref',

@@ -424,6 +427,7 @@ run {
         ]

         addOpens = [
+                'javafx.base/com.sun.javafx.event' : 'org.jabref',
                 'javafx.controls/javafx.scene.control' : 'org.jabref',
                 'org.controlsfx.controls/org.controlsfx.control.textfield' : 'org.jabref',
                 'javafx.controls/com.sun.javafx.scene.control' : 'org.jabref',

@koppor
Copy link
Member Author

koppor commented Apr 15, 2024

The exception in line wrapped form:

  • java.lang.IllegalAccessError:
  • class org.controlsfx.control.textfield.AutoCompletionBinding (in module org.jabref.merged.module) cannot access
  • class com.sun.javafx.event.EventHandlerManager (in module javafx.base) because
  • module javafx.base does not export com.sun.javafx.event to module org.jabref.merged.module

@Siedlerchr
Copy link
Member

needs to be added to the run below as well

@koppor
Copy link
Member Author

koppor commented Apr 15, 2024

needs to be added to the run below as well

You mean the run{, which around line 405 and therefore not shown at the diff? I added it to that!

@koppor
Copy link
Member Author

koppor commented Apr 15, 2024

The secret was to use applicationDefaultJvmArgs, which is picked up by the jlink plugin as jvmArgs (see https://badass-jlink-plugin.beryx.org/releases/latest/).

@koppor koppor changed the title [WIP] Add export of com.sun.javafx.event to org.jabref Add export of com.sun.javafx.event to org.jabref Apr 15, 2024
@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 15, 2024
@Siedlerchr Siedlerchr mentioned this pull request Apr 15, 2024
2 tasks
build.gradle Outdated

applicationDefaultJvmArgs =
// Fix for https://github.com/JabRef/jabref/issues/11188
['--add-exports=javafx.base/com.sun.javafx.event=org.jabref.merged.module']
Copy link
Member

Choose a reason for hiding this comment

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

seems like we need to add all runStuff here
'> java.lang.reflect.InaccessibleObjectException: Unable to make protected javafx.collections.ObservableList javafx.scene.Parent.getChildren() accessible: module javafx.graphics does not "opens javafx.scene" to module org.jabref.merged.module

from #11198

@koppor
Copy link
Member Author

koppor commented Apr 15, 2024

And we seem to need more?!

java.lang.reflect.InaccessibleObjectException: Unable to make protected javafx.collections.ObservableList javafx.scene.Parent.getChildren() accessible: module javafx.graphics does not "opens javafx.scene" to module org.jabref.merged.module

@koppor koppor enabled auto-merge April 15, 2024 18:49
@koppor
Copy link
Member Author

koppor commented Apr 15, 2024

Last commit was because of

WARNING: package impl.org.controlsfx.skin not in org.controlsfx.controls
WARNING: package org.controlsfx.control.textfield not in org.controlsfx.controls
Messages are not initialized before accessing key: Display help on command line options

@koppor
Copy link
Member Author

koppor commented Apr 15, 2024

Current issue - when buld locally, it works, when build on the CI, it does not work.

Siedlerchr
Siedlerchr previously approved these changes Apr 15, 2024
Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

works now on mac as well

@koppor koppor added this pull request to the merge queue Apr 15, 2024
@koppor koppor removed this pull request from the merge queue due to a manual request Apr 15, 2024
@koppor koppor enabled auto-merge April 15, 2024 20:26
@koppor koppor disabled auto-merge April 15, 2024 20:26
Siedlerchr
Siedlerchr previously approved these changes Apr 15, 2024
@koppor koppor added this pull request to the merge queue Apr 15, 2024
@koppor koppor removed this pull request from the merge queue due to a manual request Apr 15, 2024
@koppor koppor added this pull request to the merge queue Apr 15, 2024
Copy link
Contributor

github-actions bot commented Apr 15, 2024

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

Merged via the queue into main with commit a30704f Apr 15, 2024
24 of 25 checks passed
@koppor koppor deleted the fix-11188 branch April 15, 2024 21:08
@LoayGhreeb
Copy link
Collaborator

Unable to compile on windows after this PR.

Log
> Task :run
WARNING: Unknown module: org.jabref.merged.module specified to --add-exports
WARNING: Unknown module: org.jabref.merged.module specified to --add-exports
WARNING: Unknown module: org.jabref.merged.module specified to --add-opens
WARNING: Unknown module: org.jabref.merged.module specified to --add-opens
WARNING: Unknown module: org.jabref.merged.module specified to --add-opens
Messages are not initialized before accessing key: Display help on command line options
2024-04-15 23:33:27 [main] org.jabref.logic.journals.JournalAbbreviationLoader.loadRepository()
WARN: There is no journal-list.mv. We use a default journal list


Exception in Application start method
2024-04-15 23:33:29 [main] org.jabref.Launcher.main()
ERROR: Unexpected exception: java.lang.RuntimeException: Exception in Application start method
	at javafx.graphics@22/com.sun.javafx.application.LauncherImpl.launchApplication1(LauncherImpl.java:893)
	at javafx.graphics@22/com.sun.javafx.application.LauncherImpl.lambda$launchApplication$2(LauncherImpl.java:196)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make private static void org.controlsfx.control.textfield.TextFields.setupClearButtonField(javafx.scene.control.TextField,javafx.beans.property.ObjectProperty) accessible: module org.controlsfx.controls does not "opens org.controlsfx.control.textfield" to module org.jabref
	at java.base/java.lang.reflect.AccessibleObject.throwInaccessibleObjectException(AccessibleObject.java:391)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:367)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:315)
	at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:203)
	at java.base/java.lang.reflect.Method.setAccessible(Method.java:197)
	at org.jabref@100.0.0/org.jabref.gui.groups.GroupTreeView.setupClearButtonField(GroupTreeView.java:589)
	at org.jabref@100.0.0/org.jabref.gui.groups.GroupTreeView.initialize(GroupTreeView.java:256)
	at org.jabref@100.0.0/org.jabref.gui.groups.GroupTreeView.<init>(GroupTreeView.java:121)
	at org.jabref@100.0.0/org.jabref.gui.sidepane.SidePaneContentFactory.create(SidePaneContentFactory.java:52)
	at org.jabref@100.0.0/org.jabref.gui.sidepane.SidePaneComponent.initialize(SidePaneComponent.java:42)
	at org.jabref@100.0.0/org.jabref.gui.sidepane.SidePaneComponent.<init>(SidePaneComponent.java:36)
	at org.jabref@100.0.0/org.jabref.gui.sidepane.GroupsSidePaneComponent.<init>(GroupsSidePaneComponent.java:26)
	at org.jabref@100.0.0/org.jabref.gui.sidepane.SidePaneViewModel.getSidePaneComponent(SidePaneViewModel.java:85)
	at org.jabref@100.0.0/org.jabref.gui.sidepane.SidePane.updateView(SidePane.java:61)
	at org.jabref@100.0.0/org.jabref.gui.sidepane.SidePane.<init>(SidePane.java:55)
	at org.jabref@100.0.0/org.jabref.gui.JabRefFrame.initLayout(JabRefFrame.java:272)
	at org.jabref@100.0.0/org.jabref.gui.JabRefFrame.init(JabRefFrame.java:371)
	at org.jabref@100.0.0/org.jabref.gui.JabRefFrame.<init>(JabRefFrame.java:144)
	at org.jabref@100.0.0/org.jabref.gui.JabRefGUI.start(JabRefGUI.java:72)
	at javafx.graphics@22/com.sun.javafx.application.LauncherImpl.lambda$launchApplication1$9(LauncherImpl.java:839)
	at javafx.graphics@22/com.sun.javafx.application.PlatformImpl.lambda$runAndWait$12(PlatformImpl.java:483)
	at javafx.graphics@22/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:456)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:400)
	at javafx.graphics@22/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:455)
	at javafx.graphics@22/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)
	at javafx.graphics@22/com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
	at javafx.graphics@22/com.sun.glass.ui.win.WinApplication.lambda$runLoop$3(WinApplication.java:184)
	... 1 more

@koppor koppor mentioned this pull request Apr 15, 2024
6 tasks
@koppor
Copy link
Member Author

koppor commented Apr 15, 2024

@LoayGhreeb I got it, too. Should be fixed at #11208. Please check latest main :)

@LoayGhreeb
Copy link
Collaborator

yes, it's fixed. Although those "WARNING" messages are still showing up, but I think it's fine.

@koppor koppor mentioned this pull request Apr 16, 2024
6 tasks
@koppor
Copy link
Member Author

koppor commented Apr 16, 2024

yes, it's fixed. Although those "WARNING" messages are still showing up, but I think it's fine.

I filed #11209 to avoid the messages.

@koppor koppor mentioned this pull request Jul 9, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InaccessibleObjectException Total Breakdown
4 participants