Skip to content

Update FlatLaf from 2.4 to 2.5#4696

Closed
DevCharly wants to merge 3 commits into
apache:masterfrom
DevCharly:flatlaf-2.5
Closed

Update FlatLaf from 2.4 to 2.5#4696
DevCharly wants to merge 3 commits into
apache:masterfrom
DevCharly:flatlaf-2.5

Conversation

@DevCharly

Copy link
Copy Markdown
Member

@DevCharly DevCharly added Upgrade Library Library (Dependency) Upgrade Look and Feel labels Sep 28, 2022
@DevCharly DevCharly added this to the NB16 milestone Sep 28, 2022
…rsion

This allows modules that need access to the internal packages and
accept, that they will be version locked to declare an implementation
dependency on this module.
@matthiasblaesing

Copy link
Copy Markdown
Contributor

Change looks sane to me, however I suggest to include this in this change set (if in doubt, feel free to squash it into your commit):

66716b5

The intention is to fix the issue described in #4619. @neilcsmith-net it would be nice if you could have a look.

@DevCharly

Copy link
Copy Markdown
Member Author

@matthiasblaesing Done. Thanks.

@DevCharly

Copy link
Copy Markdown
Member Author

@mbastian could you try whether the added implementation version works with your FlatLaf-SwingX module?

@neilcsmith-net

Copy link
Copy Markdown
Member

@matthiasblaesing that change looks a good idea.

I wonder if we should be pre-extracting the native libs similarly to how JNA module does it here?

Aside @DevCharly - while looking at FlatLaf code for native loading noticed the comments on libjawt. Similar to #4441 and java-native-access/jna#1422 Probably good to always preload.

@matthiasblaesing

Copy link
Copy Markdown
Contributor

@matthiasblaesing that change looks a good idea.

Thanks for having a look.

I wonder if we should be pre-extracting the native libs similarly to how JNA module does it here?

In theory a good idea, in practice extracting libraries needs a way to force loading through System#loadLibrary with the basename only. In FlatLaf the loading always goes through extraction and loading target with full path. We'd need to modify this.

@neilcsmith-net

neilcsmith-net commented Oct 2, 2022

Copy link
Copy Markdown
Member

In FlatLaf the loading always goes through extraction and loading target with full path.

Yes, I saw. We'd need to set this in the module installer.

Let's merge as is, but keep this in mind if we get any linking error reports.

@DevCharly an option in FlatLaf to prefer trying to load via System::loadLibrary without full path would be great in future!

@matthiasblaesing

Copy link
Copy Markdown
Contributor

@neilcsmith-net thank you for the pointer - I looked one level to deep. What do you think about this:

6781772

@DevCharly the referenced commit should change the library loading, so that the library is loaded from the pre-extracted version in the modules directory. The issue with self extracting libraries is two fold: The first problem is cleanup - windows only lets you remove libraries after the users are closed, the second problem is on linux the temp directory could be mounted noexec, which also prevends library loading. As NetBeans needs to be executed, it can be expected, that the modules are located in an executable location.

@neilcsmith-net

neilcsmith-net commented Oct 2, 2022

Copy link
Copy Markdown
Member

Thanks @matthiasblaesing Possibly should go directly in modules/lib rather than modules/lib/flatlaf though? The lib folder has a defined structure. Also, InstalledFileLocator can handle directories.

@matthiasblaesing

Copy link
Copy Markdown
Contributor

@neilcsmith-net valid point updated to:

be4dba6

@DevCharly if you consider this, please use this commit, not the previous one, I replaced it based on Neils comments.

@DevCharly

Copy link
Copy Markdown
Member Author

@matthiasblaesing have added your commit (removed an unused import)

@DevCharly an option in FlatLaf to prefer trying to load via System::loadLibrary without full path would be great in future!

I wonder where Java/NetBeans searches for libraries if System::loadLibrary is used?

@neilcsmith-net

Copy link
Copy Markdown
Member

I wonder where Java/NetBeans searches for libraries if System::loadLibrary is used?

In NetBeans, from modules/lib where the commit puts it. The classloader is used for locating native libraries, and the NetBeans classloaders look there.

@DevCharly

Copy link
Copy Markdown
Member Author

I wonder if we should be pre-extracting the native libs similarly to how JNA module does it here?

Is it possible that this is broken?
When I start NB 15 (on Windows), JNA creates jna14122183419442360685.dll in C:\Users\abc\AppData\Local\Temp\jna--1361632413.

Looking at the JNA code, it seems that JNA uses System.loadLibrary() only if system property jna.nosys is set to false, which seems not the case.
https://github.com/java-native-access/jna/blob/bf60e51eace6dffa18548019e2ba398ff84904ef/src/com/sun/jna/Native.java#L1003

@DevCharly

Copy link
Copy Markdown
Member Author

@neilcsmith-net

Copy link
Copy Markdown
Member

Well, jna.nosys used to default to false. Changed in JNA 5.x I think (@matthiasblaesing ?). Possible this should have been updated in the installer here - https://github.com/apache/netbeans/blob/master/platform/libs.jna/src/org/netbeans/libs/jna/Installer.java#L29 Take to another issue?

@matthiasblaesing

Copy link
Copy Markdown
Contributor

@DevCharly: You are right - it currently hits the "load from classpath" case
@neilcsmith-net You are also right. The issue comes from the fix for java-native-access/jna#384. I think the change was sane, but indeed NetBeans needs to be updated to handle this.

@matthiasblaesing

Copy link
Copy Markdown
Contributor

The jna issue should be fixed by #4736

@neilcsmith-net

Copy link
Copy Markdown
Member

@DevCharly sorry, looks like our PRs conflicted in changes to Installer. Are you able to update and resolve?

@neilcsmith-net

Copy link
Copy Markdown
Member

Opened #4803 with conflicts resolved instead.

@neilcsmith-net neilcsmith-net removed this from the NB16 milestone Oct 17, 2022
@DevCharly
DevCharly deleted the flatlaf-2.5 branch January 23, 2023 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Look and Feel Upgrade Library Library (Dependency) Upgrade

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants