-
Notifications
You must be signed in to change notification settings - Fork 94
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
Supplemented JAR manifest with headers required to use library in OSGi environment #89
Supplemented JAR manifest with headers required to use library in OSGi environment #89
Conversation
- Supplement JAR manifest with headers required to use library in OSGi environment
Thanks for your contribution! I wonder if there's enough interest from the FastCSV user base that justifies the effort of supporting and maintaining this. I can't provide any resources to support OSGI related questions or tasks. |
@osiegmar There is nothing to support beyond what's already done / part of this commit. The You will not need to support anything / no additional work on your part will be needed - this takes care of itself. |
Every tiny bit needs care! Build warningsThe build plugin you added throws this build warning:
You need to update the build plugin to version 7.0.0. How to verify that after this plugin update everything still works? There's no automatic test for that. OSGi manifestThe OSGi manifest also exports the package SupportWhat if someone rises an OSGi related issue/question? Who will take care of that? |
- Supplement JAR manifest with headers required to use library in OSGi environment - Exclude 'de.siegmar.fastcsv.util' package from public API
Would you be OK to live with this warning you mentioned, until you move project to Java 17, or should I go ahead and upgrade your project to Java 17 then? Warning cannot be even seen without all warnings turned on.. There is no intermediate version of
By default,
I cannot imagine why they would bother you about OSGi part of your project once this is done / merged with trunk. However, you can ping me then / ping @juergen-albert (Juergen Albert, Data in Motion), or simply refer them to documentation: https://github.com/bndtools/bnd/blob/master/gradle-plugins/README.md, https://bnd.bndtools.org/. Does that address your concerns? |
Thanks for your comments.
|
@osiegmar What you ask for should be no Problem. I suggest to go a bit further, as it might give you some extra value: As already mentioned by @ideas-into-software we have a bunch of build time annotations (no extra requirement at runtime) you can use to define what the outside world can see in a modular environement. So, with this, there is no need to modify any build files when you e.g. rename or remove a package as the truth is in the code. We can additionally also instruct the bnd plugin to create a proper module-info for you, as JPMS as an OSGi like requires similar data then OSGi does. As the Annotations are part of an official Specification you can also assume that this will be supported and not change in the future. |
Sounds interesting, @juergen-albert. Could you open a separate PR for that in order to see and compare? |
@osiegmar Please clarify your preference, i.e. should I go ahead with both what you (#89 (comment)) and @juergen-albert (#89 (comment)) mentioned ? @juergen-albert I presume you're busy and I should take care of both ? If so, is it OK I can supplement this PR @osiegmar, instead of opening new PR ? |
Right now I cannot express a preference as I haven't seen an annotation based implementation. That's why I asked for a separat PR in order to compare the two solutions. |
working on it |
- Supplement JAR manifest with headers required to use library in OSGi environment - Whitelist instead of blacklist for public API packages - Upgrade 'biz.aQute.bnd.builder' plugin to latest available version (7.0.0)
Most recent commit contains whitelist instead of blacklist for public API packages and
|
Thanks! Just merged #92 |
Proposed Changes
Currently released version of FastCSV library cannot be used in OSGi environment due to lack of required manifest headers - i.e.
MANIFEST.MF
having only the following:Therefore,
biz.aQute.bnd.builder
gradle plugin was introduced into the project with most basic configuration, and resulting JAR now has all required headers - i.e.MANIFEST.MF
contains the following:Merging this PR will enable us and others who develop OSGi apps to use FastCSV library out-of-the box (i.e. without additional manual steps, so called wrapping https://bnd.bndtools.org/chapters/390-wrapping.html).
Performance
(output of
lib/build/results/jmh/results.txt
)