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

ADR for managing native libraries in GraalVM native images #43984

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

cescoffier
Copy link
Member

This PR introduces an ADR detailing the process for managing native libraries in Quarkus extensions when performing native compilation with GraalVM.

Key points include:

  • Using GraalVM's Feature mechanism to select and include platform-specific native libraries.
  • Centralizing native library management and runtime initialization configuration within the Feature.
  • Examples for implementing a Feature to include native libraries such as brotli.so.
  • Discussion of considered options and consequences of using the Feature approach.

Copy link
Contributor

@zakkak zakkak left a comment

Choose a reason for hiding this comment

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

Thank you for the write down @cescoffier! It looks good to me. Just added a few comments that I leave up to you to decide if they are worth including or not.

adr/0006-native-compilation-with-binary-libraries.adoc Outdated Show resolved Hide resolved
adr/0006-native-compilation-with-binary-libraries.adoc Outdated Show resolved Hide resolved
adr/0006-native-compilation-with-binary-libraries.adoc Outdated Show resolved Hide resolved
adr/0006-native-compilation-with-binary-libraries.adoc Outdated Show resolved Hide resolved
Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

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

Looks good! good to have written down.

just minor suggestions on clarification but otherwise +1

@maxandersen
Copy link
Member

Just realized - should we document pattern to use for when libraries have all binaries included in one artifacts vs using multiple classifers (multiple artifacts) ?

@zakkak
Copy link
Contributor

zakkak commented Oct 22, 2024

Just realized - should we document pattern to use for when libraries have all binaries included in one artifacts vs using multiple classifers (multiple artifacts) ?

I don't think we use any different pattern in this case (at least in native mode).

@cescoffier
Copy link
Member Author

Just realized - should we document pattern to use for when libraries have all binaries included in one artifacts vs using multiple classifers (multiple artifacts) ?

I've added a note.

This commit introduces an ADR detailing the process for managing native libraries in Quarkus extensions when performing native compilation with GraalVM.

Key points include:
- Using GraalVM's `Feature` mechanism to select and include platform-specific native libraries.
- Centralizing native library management and runtime initialization configuration within the `Feature`.
- Examples for implementing a `Feature` to include native libraries such as `brotli.so`.
- Discussion of considered options and consequences of using the `Feature` approach.

Co-authored-by: @zakkak
@cescoffier cescoffier force-pushed the adr-binary-in-native-executable branch from b6535c3 to af6a5e8 Compare October 24, 2024 06:45
Copy link
Contributor

@zakkak zakkak left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the additions.

@gsmet gsmet dismissed maxandersen’s stale review October 25, 2024 15:24

Comments have been addressed AFAICS. Let's merge!

@gsmet
Copy link
Member

gsmet commented Oct 25, 2024

AFAICS, Max was +1 and his comments were addressed so let's merge!

@gsmet gsmet merged commit da39c8b into quarkusio:main Oct 25, 2024
3 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.17 - main milestone Oct 25, 2024
@cescoffier cescoffier deleted the adr-binary-in-native-executable branch October 27, 2024 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants