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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

added graalvm native image reflect config for a seamless experience #578

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cmdjulian
Copy link

Description

This changes adds the required graalvm reflection config for the two missing fields. This config is picked up by graalvm during native compilation and registers the two fields for reflective access on the compiled native image java app.
See here for more details.

Related Issue

#448

Type of Change

  • 馃摎 Examples / docs / tutorials / dependencies update
  • 馃敡 Bug fix (non-breaking change which fixes an issue)
  • 馃 Improvement (non-breaking change which improves an existing feature)
  • 馃殌 New feature (non-breaking change which adds functionality)
  • 馃挜 Breaking change (fix or feature that would cause existing functionality to change)
  • 馃攼 Security fix

Checklist

  • I've written tests (if applicable) for all new methods and classes that I created.
  • I've added documentation as necessary so users can easily use and understand this feature/fix.

@cla-bot
Copy link

cla-bot bot commented Jun 24, 2023

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @cmdjulian on file. In order for us to review and merge your code, please sign our Contributor License Agreement to get yourself added. You'll find the CLA and more information here: https://github.com/hivemq/hivemq-community/blob/master/CONTRIBUTING.adoc#contributor-license-agreement

@cmdjulian
Copy link
Author

@cla-bot check

@cla-bot cla-bot bot added the cla-signed label Jul 1, 2023
@cla-bot
Copy link

cla-bot bot commented Jul 1, 2023

The cla-bot has been summoned, and re-checked this pull request!

@SgtSilvio
Copy link
Member

Thank you for your contribution @cmdjulian!
In general, it would be best if JCTools would ship with the right native image configuration. As JCTools is only a dependency, the user of the hivemq-mqtt-client can update the JCTools version transitively which might need a change in the native image configuration as well.
But I agree that we can at least make the user experience better.
I am unsure if reflective access is the right thing here. We have the hivemq-mqtt-client running in a native image as part of the mqtt-cli, where we --initialize-at-build-time the JCTools classes, see https://github.com/hivemq/mqtt-cli/blob/master/build.gradle.kts#L416.
Maybe we can just include these settings instead of the reflection config?

@cmdjulian
Copy link
Author

Yeah you right. Thanks for pointing this out.
I had a look at JCTools, and is seems like they use a lot of reflection and Unsafe. I was thinking about contributing a PR over there as well, maybe I find some time in the future to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants