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

[jsscripting] Use OSGi-ified GraalVM dependencies #18053

Merged
merged 7 commits into from
Feb 13, 2025

Conversation

florian-h05
Copy link
Contributor

@florian-h05 florian-h05 commented Jan 6, 2025

Fixes #18054 by using a OSGi-ified Truffle Runtime, which is loaded only once.
It also reduces the add-on size from 34 MB to 16 MB: Truffle ICU4j was pretty big and now is provided as OSGi bundle and hence not bundled with the add-on anymore.

GraalVM Polyglot and Truffle API dependencies cannot be provided as OSGi bundles, because those use the Java ServiceLoader to load the available polyglot and truffle language implementations, which depend on on the individual add-on, and we cannot configure the service loading for those if they are shared OSGi bundles.

Signed-off-by: Florian Hotze <dev@florianhotze.com>
…endency

Signed-off-by: Florian Hotze <dev@florianhotze.com>
Signed-off-by: Florian Hotze <dev@florianhotze.com>
…ndency

Signed-off-by: Florian Hotze <dev@florianhotze.com>
@florian-h05 florian-h05 changed the title [not working] [jsscripting] Use OSGI-ified GraalVM dependencies [jsscripting] Use OSGI-ified GraalVM dependencies Feb 4, 2025
Signed-off-by: Florian Hotze <dev@florianhotze.com>
@florian-h05 florian-h05 changed the title [jsscripting] Use OSGI-ified GraalVM dependencies [jsscripting] Use OSGi-ified GraalVM dependencies Feb 4, 2025
Signed-off-by: Florian Hotze <dev@florianhotze.com>
Signed-off-by: Florian Hotze <dev@florianhotze.com>
@florian-h05 florian-h05 marked this pull request as ready for review February 5, 2025 09:16
@ccutrer
Copy link
Contributor

ccutrer commented Feb 6, 2025

Question: is there a reason you didn't OSGi-ify the polyglot dependency? That also seems like it's going to be shared with the pythonscripting add-on.

@florian-h05
Copy link
Contributor Author

Yeah, I didn’t do this because I couldn’t get if work and I think it might not be able to work at all.

Both the Polyglot and the Truffle API dependencies use the Java ServiceLoader to load the available engines and languages, which depend on the actual language you use.

OSGi-ifying those two dependencies would require to be able to modify the service loading bevahiour of those two depending on the bundle requiring them. I googled a bit (I am no OSGi expert either) and everywhere it was recommended to build a fat JAR including those two deps.

@florian-h05 florian-h05 requested a review from a team February 10, 2025 10:07
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

@lsiepel lsiepel merged commit 8c68fb7 into openhab:main Feb 13, 2025
2 checks passed
@lsiepel lsiepel added this to the 5.0 milestone Feb 13, 2025
@florian-h05 florian-h05 deleted the jsscripting-osgi branch February 13, 2025 08:37
@florian-h05 florian-h05 added the enhancement An enhancement or new feature for an existing add-on label Feb 13, 2025
@HolgerHees
Copy link
Contributor

@florian-h05 works for the pythonscripting binding too

HolgerHees@efd1e87

matgroe pushed a commit to matgroe/openhab-addons that referenced this pull request Feb 24, 2025
* [jsscripting] Use OSGI-ified org.graalvm.sdk:* dependencies

Signed-off-by: Florian Hotze <dev@florianhotze.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
4 participants