-
Notifications
You must be signed in to change notification settings - Fork 1
feat: embed graalvm libs #601
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR embeds GraalVM libraries directly into the javascript-modules-engine bundle instead of relying on them being provided by Jahia. The GraalVM SDK version 17.0.12 and languages version 23.0.5 are added as properties and used consistently across the module.
- Defines GraalVM version properties at the root POM level
- Changes graal-sdk dependency from provided scope to embedded, and adds additional GraalVM dependencies (truffle-api, js, regex) plus icu4j
- Updates the bundle configuration to embed the new dependencies and marks them as ignored for unused dependency analysis
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pom.xml | Adds GraalVM version properties (SDK and languages versions) |
| javascript-modules-engine/pom.xml | Updates Embed-Dependency configuration to include GraalVM libraries |
| javascript-modules-engine-java/pom.xml | Changes graal-sdk from provided scope to embedded with explicit version, adds new GraalVM dependencies, and updates dependency analysis configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jkevan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be we should export the packages like they were exported by framework system from jahia core:
<Export-Package>
org.graalvm.*;version="${graalvm.languages.version}",
com.oracle.truffle.*;version="${graalvm.languages.version}",
com.oracle.js.*;version="${graalvm.languages.version}"
</Export-Package>
I see two potential argument in favor:
- better release lifecycle since it's a module, we could upgrade library faster
- since they were already exported from jahia core before this refactoring, it could be seen a breaking change not to exposing them anymore.
3fcf91e to
839476f
Compare
839476f to
c3a04d2
Compare
fixes #556
Description
Embed graalvm libs with javascript module engine.
Provided version are the same as the one in Jahia.
Checklist
Source code
Tests
Tip
Documentation to guide the reviews: How to do a code review