-
Notifications
You must be signed in to change notification settings - Fork 57
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
OSGi import/export headers #242
Comments
I'm not sure we have someone amongst our maintainers that is an OSGi expert. I'm reluctant to these additions as we have not means to properly test such metadata nor do we have the expertise to maintains OSGi declarations. Such arrangements typically ask for constant trouble and maintenance. |
Yeah - I get that, it is a bit of a fringe area these days. In fact, looking at what the BND tool produces, I don't think it's too hard in your case for just the SPI JAR. Here's how the Manifest comes out once we push it through the "auto BND" in our library re-build stage
Most of the above is boilerplate our re-build creates - the active parts really are the Import-Package / Export-Package headers. We made the javax.annotation.meta optional which may not be strictly correct for your code, but since we only need the bundle to resolve (we're not using the underlying classes) it handles things our side. |
I've added these meta data to jOOQ early on using the Apache Felix Maven plugin, and surprisingly, I hardly ever had any need to maintain them, which can only mean two things:
|
OSGi is a rarer usage case for sure - I couldn't speak to the wider usage, but Apache Felix is certainly used and maintained which is how we use our OSGi framework. I believe the Eclipse IDE itself was OSGi framework based at one stage, although we mostly used Netbeans or VScode these days. I guess the imports must have changed with jOOQ 3.15, since this issue only arises then. Presumably r2dbc-spi was either not an import in 3.14, or was an optional one. |
It was not |
BTW - I'm happy to submit a pull request with the extra headers if of help. Not a MVN expert, as we're gradle users, but pretty sure it's a small change to the current pom.xml:
The essentials as per the following example
Optional, but not really required being:
|
A PR sounds good, with having One thing to note, R2DBC SPI is a component that interacts with Java's |
Cool - I'll take a look. I'm not a Git expert, so forgive me if I fumble a bit on the local fork / PR creation process. Always have to remind myself on the GitHub process for that
You have me there - I'll have to go digging into some Docs. Haven't looked into how OSGi and ServiceLoader co-operate and if additional metadata is needed. Update: this document looks like the relevant spec, and seems to be involve extra manifest declarations on the provider side and consumer side. I'm guessing r2dbc will need the consumer side ones? |
I think so. We call |
Fix for r2dbc#242 - Inclusion of basic OSGI headers. Note, further work may be needed for full OSGi ServiceLoader handling : https://blog.osgi.org/2013/02/javautilserviceloader-in-osgi.html
Submitted pull request for the basic headers. Might be handy if feasible to also merge into the 0.8.x release branch. |
Thanks a lot, the PR looks pretty decent. Can you add the |
Exactly the same here, even worse, I never really got in touch with OSGi details.
We don't have many attempts to get things right. We should do it in a single step. I cannot oversee how much or little impact we will have with the metadata already so I'm inclined to wait with the addition until we can deliver the full set of metadata. |
Let me see if OSGi supports the concept of an "optional" ServiceLoader requirement on the consumer side. If not "right" becomes a bit debatable. With the present headers, the bundle will load and not prevent the OSGi framework from starting if no service providers are present. That would be right for any bundle that needs the SPI classes to resolve, but may not actually use the underlying service - which often times with OSGi SPI or API bundles, is as many or more cases than those who use the provided service. |
Inclusion of optional ServiceLoader provider requirement
OK - added an extra commit to the PR for the ServiceLoader headers. |
…iceLoader provider requirement.
…iceLoader provider requirement. Signed-off-by: RobWalker <[email protected]>
- <developers><developer><id> element added to silence a complaint from bnd about well formed entries - let bnd generate default manifest spec entries, as well as setting all other headers - added BND SPI annotations to add the necessary parts to make ServiceLoader work in OSGi (passively) fixes r2dbc#242 Signed-off-by: Raymond Augé <[email protected]>
- <developers><developer><id> element added to silence a complaint from bnd about well formed entries - let bnd generate default manifest spec entries, as well as setting all other headers - added BND SPI annotations to add the necessary parts to make ServiceLoader work in OSGi (passively) fixes r2dbc#242 Signed-off-by: Raymond Augé <[email protected]>
Realise this is a very niche/uncommon area these days. Would be handy though to have OSGi import/export declarations in the Manifest so that the library can be used in an OSGi runtime framework with other libs that create these. Can supply examples if it is of interested to be added.
The text was updated successfully, but these errors were encountered: