Skip to content

Rename folder with native binaries from 'lib' to 'lib-native' #290

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.

Sign up for GitHub

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

Jump to bottom
Merged
jflo merged 2 commits into from
Jan 13, 2026
Merged

Rename folder with native binaries from 'lib' to 'lib-native' #290

jflo merged 2 commits into from
Jan 13, 2026
+52 −45

Conversation

@jjohannes
Copy link
Contributor

@jjohannes jjohannes commented Sep 3, 2025

A folder with a - in its name is not considered a package by Java and is then allowed to exist in multiple Jars without causing a split-package issue. See description of #274 for a detailed explanation.

Fixes #274

@jjohannes jjohannes mentioned this pull request Sep 3, 2025
Closed
@garyschulte
Copy link
Contributor

garyschulte commented Sep 9, 2025

I will check the library loading in besu with these changes. TBH we have not seen any gradle packaging problems across modules, so this is news to me. I hadn't seen #274 until this morning.

@lukasz-hashgraph lukasz-hashgraph mentioned this pull request Sep 24, 2025
Closed
@jjohannes
Copy link
Contributor Author

jjohannes commented Oct 15, 2025 •
edited
Loading

There is another issue when loading on the module path. This code leads to the attempt to find a resource that is in another Module:

BesuNativeLibraryLoader.registerJNA(Library.class, "besu_native_ec_crypto");
  • Library.class is inside com.sun.jna
  • besu_native_ec_crypto is in org.hyperledger.besu.nativelib.secp256r1

The loading is later attempted via classResource.getResourceAsStream(libraryResourcePath). That does not work if class and resource are not in the same module.

I am not sure if the code is correct in general. Is it correct that the besu_native_ec_crypto implementation is bound to the generic com.sun.jna.Library?
If that is intended, registerJNA should probably take two Class arguments:

  1. The interface to bind the implementation to
  2. A class/interface as a reference to the module the ressource is loaded from via Class.getResourceAsStream

@lukelee-sl lukelee-sl mentioned this pull request Oct 16, 2025
Merged
2 tasks
@jwagantall jwagantall requested review from a team, garyschulte, lu-pinto and macfarla October 20, 2025 20:53
@jjohannes jjohannes force-pushed the 274-split-package-fix branch from 0ef1b17 to 12492c9 Compare October 23, 2025 13:57
@ryjones ryjones force-pushed the 274-split-package-fix branch from 12492c9 to c6f6b3a Compare October 31, 2025 13:49
@stoyanov-st
Copy link

stoyanov-st commented Nov 21, 2025

Hello besu-native team!
Could we have an ETA for when this PR could be reviewed/merged and at what version would it be included?
This fix is crucial for us and will unblock us to deliver Pectra features for our clients!
Thanks for looking into it 🙏

@Ferparishuertas
Copy link

Ferparishuertas commented Jan 8, 2026 •
edited
Loading

@lu-pinto @macfarla @garyschulte Is there anyway we can bump this PR, and give it priority?

@jjohannes
Rename folder with native binaries from 'lib' to 'lib-native'
7bde0f4 
A folder with a '-' is not considered a package by Java and is then
allowed to exist in multiple Jars without causing a split-package
issue.

Signed-off-by: Jendrik Johannes <jendrik@onepiece.software>
@jjohannes
Use calling class as resource access context in BesuNativeLibraryLoader
ceb4d8a 
When running on the --module-path, the native lib resource is only
accessible from within the module if it is not located a folder
that represents an explicitly exported java package.

Signed-off-by: Jendrik Johannes <jendrik@onepiece.software>
@jjohannes jjohannes force-pushed the 274-split-package-fix branch from dfb79f9 to ceb4d8a Compare January 9, 2026 12:28
jjohannes
jjohannes commented Jan 9, 2026
View reviewed changes
common/src/main/java/org/hyperledger/besu/nativelib/common/BesuNativeLibraryLoader.java
try {
final Optional<Path> libPath = extract(jnaClass, libraryName);
String moduleContextClassName = Thread.currentThread().getStackTrace()[2].getClassName();
Class<?> moduleContextClass = Class.forName(moduleContextClassName);
Copy link
Contributor Author

@jjohannes jjohannes Jan 9, 2026

Choose a reason for hiding this comment

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

This requires the method registerJNA to be called directly from a Class inside the Module that contains the native lib file to be loaded. This is currently the case for all besu native modules.

A more explicit solution would be to add another Class argument to the method. More details:
#290 (comment)

jjohannes added a commit to hiero-ledger/hiero-consensus-node that referenced this pull request Jan 9, 2026
@jjohannes
build: update besu beyond 25.2.2
1054b00 
This requires additional patching of the besu-native Jars, to integrate
the fixes in hyperledger/besu-native#290
until they are released upstream.

Signed-off-by: Jendrik Johannes <jendrik@onepiece.software>
@jjohannes jjohannes mentioned this pull request Jan 9, 2026
Draft
jjohannes added a commit to hiero-ledger/hiero-consensus-node that referenced this pull request Jan 9, 2026
@jjohannes
build: update besu beyond 25.2.2
a7b6054 
This requires additional patching of the besu-native Jars, to integrate
the fixes in hyperledger/besu-native#290
until they are released upstream.

Signed-off-by: Jendrik Johannes <jendrik@onepiece.software>
jjohannes added a commit to hiero-ledger/hiero-consensus-node that referenced this pull request Jan 9, 2026
@jjohannes
build: update besu beyond 25.2.2
2111b13 
This requires additional patching of the besu-native Jars, to integrate
the fixes in hyperledger/besu-native#290
until they are released upstream.

Signed-off-by: Jendrik Johannes <jendrik@onepiece.software>
garyschulte
garyschulte approved these changes Jan 13, 2026
View reviewed changes
Copy link
Contributor

@garyschulte garyschulte left a comment

Choose a reason for hiding this comment

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

It wasn't obvious to me why this was needed, because besu is using classpath instead of module path. In the context of module-path deps, this change makes sense, and the '-' fix is a nice workaround.

I built locally and confirmed it works without any changes required in besu main, LGTM.

@garyschulte
Copy link
Contributor

garyschulte commented Jan 13, 2026

I will give @macfarla and @lu-pinto a chance to review before merging, but I suspect this is uncontroversial now that we understand the necessity.

@jflo jflo merged commit 18d2b7a into hyperledger:main Jan 13, 2026
12 checks passed
stoyanov-st pushed a commit to hiero-ledger/hiero-consensus-node that referenced this pull request Jan 15, 2026
@jjohannes @stoyanov-st
build: update besu beyond 25.2.2
9418ad9 
This requires additional patching of the besu-native Jars, to integrate
the fixes in hyperledger/besu-native#290
until they are released upstream.

Signed-off-by: Jendrik Johannes <jendrik@onepiece.software>
@Ferparishuertas Ferparishuertas mentioned this pull request Jan 26, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@garyschulte garyschulte garyschulte approved these changes

@macfarla macfarla Awaiting requested review from macfarla

@lu-pinto lu-pinto Awaiting requested review from lu-pinto

Assignees

@garyschulte garyschulte

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Split package issue when using multiple besu-native Jars with the Java Module System

5 participants

@jjohannes @garyschulte @stoyanov-st @Ferparishuertas @jflo