netbeans
netbeans copied to clipboard
Update FlatLaf from 2.4 to 2.5
Update FlatLaf to v2.5.
Changes: https://github.com/JFormDesigner/FlatLaf/releases/tag/2.5
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):
https://github.com/apache/netbeans/commit/66716b5506d91257f8ea136ffb11a669bac53f3a
The intention is to fix the issue described in #4619. @neilcsmith-net it would be nice if you could have a look.
@matthiasblaesing Done. Thanks.
@mbastian could you try whether the added implementation version works with your FlatLaf-SwingX module?
@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 https://github.com/java-native-access/jna/pull/1422 Probably good to always preload.
@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.
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!
@neilcsmith-net thank you for the pointer - I looked one level to deep. What do you think about this:
https://github.com/apache/netbeans/commit/6781772b899fdfeaf855ed6b20d071b8bcf5fe60
@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.
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.
@neilcsmith-net valid point updated to:
https://github.com/apache/netbeans/commit/be4dba61b9f6aa74089798095663fa3b917923d8
@DevCharly if you consider this, please use this commit, not the previous one, I replaced it based on Neils comments.
@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?
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.
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
The implementation in above line (in previous post) seems not fit to the Javadoc: https://github.com/java-native-access/jna/blob/bf60e51eace6dffa18548019e2ba398ff84904ef/src/com/sun/jna/Native.java#L83
Also hint at following line is IMHO not useful because jna.nosys=true is the default:
https://github.com/java-native-access/jna/blob/bf60e51eace6dffa18548019e2ba398ff84904ef/src/com/sun/jna/Native.java#L234
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?
@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 https://github.com/java-native-access/jna/issues/384. I think the change was sane, but indeed NetBeans needs to be updated to handle this.
The jna issue should be fixed by #4736
@DevCharly sorry, looks like our PRs conflicted in changes to Installer. Are you able to update and resolve?
Opened #4803 with conflicts resolved instead.