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

Making DbSetup osgi-ready #36

Closed
sarod opened this issue Mar 15, 2015 · 5 comments
Closed

Making DbSetup osgi-ready #36

sarod opened this issue Mar 15, 2015 · 5 comments

Comments

@sarod
Copy link

sarod commented Mar 15, 2015

DbSetup does not provide osgi metadata in it's MANIFEST.MF.
So it cannot be used in osgi runtimes as is.

That would be nice if it could include OSGI manifest information.

@jnizet
Copy link
Member

jnizet commented Mar 26, 2015

I don't know much about OSGI.
Could you please tell me what metadata, and in which format, should be in the manifest file?

@sarod
Copy link
Author

sarod commented Mar 28, 2015

The more important osgi manifest attributes are :

  • Bundle-SymbolicName that would probably be "com.ninja-squad.dbsetup"
  • Bundle-Vendor is optional but would be "Ninja Squad"
  • Bundle-Version: the bundle version
  • Import-Package: lists the dependencies needed at runtime. This is not needed for dbsetup
  • Export-Package: lists the exported packages. The exported packages are the package that are public to dependent bundles. In your case I guess only dbsetup.util is not public.

So in your case the osgi manifest could look like this:

Bundle-SymbolicName: com.ninja-squad.dbsetup
Bundle-Vendor: Ninja Squad
Bundle-Version: 1.5.0
Export-Package: com.ninja_squad.dbsetup,
 com.ninja_squad.dbsetup.bind,
 com.ninja_squad.dbsetup.destination,
 com.ninja_squad.dbsetup.generator,
 com.ninja_squad.dbsetup.operation

Gradle has an OSGI plugin (http://gradle.org/docs/current/userguide/osgi_plugin.html) that helps generating the osgi manifests attributes.

@jnizet
Copy link
Member

jnizet commented Mar 28, 2015

@sarod I created a PR (#38) for this, and pasted the generated manifest file in the PR comment. Could you please check if it looks OK to you?

Given my ignorance of OSGI, my concerns are

  • should I add those OSGI attributes in the manifest of the sources and javadoc jar as well? I currently don't do it, but I have no idea of the implications.
  • the gradle OSGI plugin generates a ;uses:=<list of used packages>" for each export package. I guess that's not a problem, except maybe that javax.annotation is always listed, although it's not actually needed at runtime: the annotations are used by FindBugs, at build time. What do you think?
  • the gradle OSGI plugin generates Import-Package: javax.annotation,javax.sql, although javax.sql is part of the JDK. What's the use of that? Should I care?

@sarod
Copy link
Author

sarod commented Mar 30, 2015

Hi,
The generated manifest is fine.

  • OSGI attributes are used for OSGI runtime so I don't think there is a need to add them to the source or javadoc jars.
  • the uses:= information are optional and painful to maintain manually. Howver since gradle OSGI/BND generates them for free you can keep them.
  • Regarding javax.sql it's optional to list the "javax.*" packages that are present in the jre however it's considered a best practice.
  • The javax.annotation is maybe more problematic. Some annotations (like Nullable) are defined with runtime retention that is why BND lists them. This should work because there is already a javax.annotation package in the jre that contains some annotation. However it's probably cleaner to exclude the annotation because you know they are not used at runtime. This should be doable using the bnd Import-Package instruction (http://www.aqute.biz/Bnd/Format#import-package).
instruction 'Import-Package', '!javax.annotation*'

@jnizet
Copy link
Member

jnizet commented Apr 3, 2015

Thanks.

FYI,

instruction 'Import-Package', '!javax.annotation*'

removes javax.annotation, but also removes javax.sql

Using

instruction 'Import-Package', '!javax.annotation.*', '*'

fixes it.

I'll merge this PR and release a new version today.

@jnizet jnizet closed this as completed in c0fd2b9 Apr 3, 2015
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

No branches or pull requests

2 participants