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

SLING-11836 - Add Jakarta JSON support to the Sling Starter #155

Closed
wants to merge 1 commit into from

Conversation

rombert
Copy link
Contributor

@rombert rombert commented Apr 25, 2023

Add the glassfish osgi resource locator to the boot feature. This shim is explicitly invoked by the JsonProvider to properly function in OSGi environments 1.

Add the glassfish osgi resource locator to the boot feature. This shim is explicitly
invoked by the JsonProvider to properly function in OSGi environments [1].

[1]: https://github.com/jakartaee/jsonp-api/blob/a75c9ab8455c4d04b787a6eb18fde5b855ba632c/api/src/main/java/jakarta/json/spi/JsonProvider.java#L616-L645
@rombert rombert requested a review from kwin April 25, 2023 15:46
@enapps-enorman
Copy link
Contributor

Perhaps they would accept a patch that uses the osgi service loader mediator instead of that glasssfish workaround? We already have the org.apache.aries.spifly.dynamic.bundle for dealing with those things.

@rombert
Copy link
Contributor Author

rombert commented Apr 26, 2023

@enapps-enorman - I did try using the SPIFly bundle locally, but I think I did not do it properly. There is some discussion about it at osgi/osgi#434 and also a larger discussion about ServiceLoader at osgi/osgi#372.

I will try again the SPIFly approach.

@enapps-enorman
Copy link
Contributor

@enapps-enorman - I did try using the SPIFly bundle locally, but I think I did not do it properly. There is some discussion about it at osgi/osgi#434 and also a larger discussion about ServiceLoader at osgi/osgi#372.

I will try again the SPIFly approach.

I haven't done much testing, but I think something like this should work: eclipse-ee4j/parsson@master...enapps-enorman:parsson:use-osgi-serviceloader-mediator

@rombert
Copy link
Contributor Author

rombert commented Apr 27, 2023

@enapps-enorman - thanks for looking into this. I built your patched version of Parsson, applied it to the starter master branch

git diff
diff --git a/src/main/features/boot.json b/src/main/features/boot.json
index fa62734..fe939d7 100644
--- a/src/main/features/boot.json
+++ b/src/main/features/boot.json
@@ -89,7 +89,7 @@
             "start-order":"1"
         },
         {
-            "id":"org.eclipse.parsson:parsson:1.1.1",
+            "id":"org.eclipse.parsson:parsson:1.1.2-SNAPSHOT",
             "start-order":"1"
         },
         {

but the build failed with

0]]
[ERROR] [requirements-capabilities] org.eclipse.parsson:parsson:1.1.2-SNAPSHOT: Artifact org.eclipse.parsson:parsson:1.1.2-SNAPSHOT requires [org.eclipse.parsson/1.1.2.SNAPSHOT] osgi.extender; filter:="(osgi.extender=osgi.serviceloader.registrar)" in start level 1 but no artifact is providing a matching capability in this start level.

What am I missing?

@rombert
Copy link
Contributor Author

rombert commented Apr 27, 2023

I went down the SPIFly route and it seems to work fine. I filed alternate PR #156 so we can compare the two and decide on a solution on the short-term.

Of course, removing the need for dynamic weaving by patching Parsson would be even better.

@enapps-enorman
Copy link
Contributor

What am I missing?

Looks like you figured this out in #156 by moving the org.apache.aries.spifly.dynamic.bundle bundle (and related bundles) to start level 1.

@rombert
Copy link
Contributor Author

rombert commented Apr 28, 2023

Closing in favour of the other PR.

@rombert rombert closed this Apr 28, 2023
@rombert rombert deleted the issue/SLING-11836 branch April 28, 2023 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants