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

#242 add OSGi headers #244

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

ascertrobw
Copy link

Fix for #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

@ascertrobw
Copy link
Author

Believe this to be right syntax for ServiceLoader headers - certainly loads as an optional requirement my side

@martin-g
Copy link

martin-g commented Sep 9, 2021

@rotty3000 Could you please review this PR ? Thanks in advance!

@mp911de
Copy link
Member

mp911de commented Sep 9, 2021

@ascertrobw Right now, there are no drivers with metadata. Would you like to provide PR's for Postgres, H2, R2DBC Pool (makes use of service loader), and SQL Server? Having metadata there would make this effort actually usable without further metadata adjustments within OSGi containers.

@mp911de
Copy link
Member

mp911de commented Sep 9, 2021

One last thing: Can you squash your commits and run a git rebase with --signoff? Doing so adds a signature line to commits that is required by the Developer Certificate of Origin for the Reactive Foundation.

@mp911de mp911de added the type: enhancement A general enhancement label Sep 9, 2021
@mp911de mp911de added this to the 0.8.6.RELEASE milestone Sep 9, 2021
…iceLoader provider requirement.

Signed-off-by: RobWalker <[email protected]>
@ascertrobw
Copy link
Author

One last thing: Can you squash your commits and run a git rebase with --signoff? Doing so adds a signature line to commits that is required by the Developer Certificate of Origin for the Reactive Foundation.

Not really an area I'm used too - hopefully I got that right!

@ascertrobw
Copy link
Author

@ascertrobw Right now, there are no drivers with metadata. Would you like to provide PR's for Postgres, H2, R2DBC Pool (makes use of service loader), and SQL Server? Having metadata there would make this effort actually usable without further metadata adjustments within OSGi containers.

Certainly an area I'd be happy to help with at some stage down the line - not sure I'll get bandwidth at the present on those though.

@rotty3000
Copy link

My first thought on this change is that the work was done manually rather than using a build plugin that can simplify things.
For example, like the fact that Bundle-Version currently uses a raw project.version value, but OSGi does not accept maven version encoding. If someone were to make a snapshot build for testing X.Y.Z-SNAPSHOT (or any other qualifier) the bundle wouldn't work in OSGi because of incorrect version encoding.

Furthermore, nothing will warn about metadata skew, since the work was done by hand there is nothing to validate it nor will anyone notice when it breaks until someone from the outside reports it (never an ideal scenario).

The intent is admirable and I'm willing to help. However, I wouldn't move forward unless there is buy-in to use a build plugin to handle the heavy lifting. Doing OSGi manually is why most of the OSGi hate exists. The cognitive load on the inexperienced is too high. Robots however (plugins like bnd-maven-plugin) are ideally suited to handle it and I would gladly help adopting that strategy if that is desired :)

@mp911de
Copy link
Member

mp911de commented Sep 9, 2021

I see your point, especially the concern with a snapshot version is perfectly valid and we do not have the means to easily influence that aspect. I guess we will have to switch to a suggested and maintained build plugin.

I'd ask specifically for help to set up an example (likely something outside of this repository) where we can verify that our OSGi setup is correct. Since I know next to nothing about how to get started with OSGi I'd appreciate a set of dependencies (a pom.xml or so) and a bit of hello world code. Later on, we can host that in the R2DBC Pool or R2DBC H2 repositories.

@rotty3000
Copy link

I'm willing to help with that. In fact I find it rather exciting challenge to help projects adopt OSGi, provided it proves not to be a burden for ongoing development. Is there any problem keeping this issue open for the medium term? It might take me some time to get to this and/or I can collaborate with @ascertrobw and anyone else interested.

@mp911de
Copy link
Member

mp911de commented Sep 9, 2021

Is there any problem keeping this issue open for the medium term?

No need to rush here. We have regular releases about every two months so we can include this change once it is done. I don't have a strong opinion on reusing this PR vs. creating a new one.

@ascertrobw
Copy link
Author

Happy to collaborate with @rotty3000 when there's time both ways. Any chance we can commit the current PR as an interim? At present, it's impossible to use jOOQ 3.15 in an OSGi environment using the standard r2dbc-spi build. At least this would get over that for now.

@mp911de
Copy link
Member

mp911de commented Sep 9, 2021

FWIW, isn't there a tool that allows the injection of OSGi metadata into existing jars?

@ascertrobw
Copy link
Author

FWIW, isn't there a tool that allows the injection of OSGi metadata into existing jars?

In our local Gradle based build env we use BND (in fact the same tool the maven plugin uses I think). As of yesterday, I have that setup to create our own internal r2dbc-spi build so our guys can upgrade to and use jOOQ 3.15. But being purely internal, that doesn't really help anyone else out there. Fully agree with @rotty3000 comments on manual manifest creation being less than ideal - and the MVN version format not really being OSGi compatible (although Apache Felix is quite happy to ignore that in practice). But until there's time for someone who knows the maven bundle plugin this would at least provide something usable.

@mp911de mp911de removed this from the 0.8.6.RELEASE milestone Sep 20, 2021
@rotty3000
Copy link

rotty3000 commented Oct 5, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants