Fully automate OSGi metadata creation and implement minor fixes #327
Conversation
Signed-off-by: Hannes Wellmann <wellmann.hannes1@gmx.net>
HannesWell
force-pushed
the
automate_osgi_metadata
branch
from
ad083a6
to
e10feb8
Compare
|
Additionally I did a comparison of the Manifest headers in the new artifacts against the SLF4J-2.0.5 release using a little self-written Java-program. The following are the results (the headers slf4j-apislf4j-simpleslf4j-nopslf4j-jdk14slf4j-jdk-platform-loggingslf4j-reload4jslf4j-extjcl-over-slf4jlog4j-over-slf4jjul-to-slf4josgi-over-slf4jslf4j-migrator |
|
And the same baseline comparison against the SLF4J-2.0.6 release (again ignoring slf4j-apislf4j-simpleslf4j-nopslf4j-jdk14slf4j-jdk-platform-loggingslf4j-reload4jslf4j-extjcl-over-slf4jlog4j-over-slf4jjul-to-slf4josgi-over-slf4jslf4j-migrator |
log4j-over-slf4j/pom.xml
Outdated
| <artifactId>maven-bundle-plugin</artifactId> | ||
| <configuration> | ||
| <instructions> | ||
| <Import-Package>org.slf4j.*;version="$${range;[===,3);${slf4j.api.minimum.compatible.version}}"</Import-Package> |
There was a problem hiding this comment.
Is it really necessary for this artifact to specify a lower bound for the slf4j package.imports of 1.6?
|
I highly recommend to NOT use felix maven bundle plugin. The bnd maven Plugin gives much better support. And Felix just uses bnd. I also started a pr that uses bnd.this is closed. But the project seems to like management ot the osgi metadata by hand. |
Yes I would like to do that (and even suggested to use the bnd-maven-plugin in the initially mentioned PR), but the bnd-maven-plugin fails to process the multi-release jars for Java-9 that contain a separated modules-info: bndtools/bnd#5346 (comment) |
To be honest every-time I asked for "support" at BND it ended in disaster complaining that BND is doing all fine and it is just the projects fault not being "enlightened" enough by the glory of BND ... so I can absolutely understand if projects like to use plugins with "less support" that simply work and solve real world development issues.
I even created PRs with fully working code to fix this specific issue:
but they are delayed for probably NIH or whatever... but I think my idea of MR-JAR support can be applied to Felix as well (even though it would be a bit more to fiddle around to overcome the current limitations of the
as this is all just a few loops over in-memorry representation that's quite fast and then Felix could process MR-jars even more comfortable. |
|
As pointed out in the discussion, it would be usefully to really use bnd and baselining to NOT BREAK the API again. |
HannesWell
force-pushed
the
automate_osgi_metadata
branch
from
e10feb8
to
c6f8161
Compare
|
I managed to make the bnd-maven-plugin working, the
Im torn about that point. @laeubi, @stbischof what's your opinion about that? @ceki from #320 (comment) I assume that you would probably prefer to not use the bnd-annotations to let the |
| <artifactId>bnd-maven-plugin</artifactId> | ||
| <configuration> | ||
| <bnd><![CDATA[ | ||
| Automatic-Module-Name: ${bsn} |
There was a problem hiding this comment.
Instead of adding Automatic-Module-Name headers, wouldn't it be better to add a regular modules-info.java?
This question applies for the other artifacts, where I added that header as well.
| Import-Package: org.slf4j.*;${slf4j_self_import_version}, * | ||
| -exportcontents: | ||
| Bundle-Activator: org.slf4j.osgi.logservice.impl.Activator | ||
| Bundle-Category: osgi |
There was a problem hiding this comment.
Is that header necessary?
| X-Compile-Source-JDK: ${maven.compiler.source} | ||
| X-Compile-Target-JDK: ${maven.compiler.target} | ||
| Implementation-Version: ${project.version} | ||
| Implementation-Title: $[project.artifactId] |
There was a problem hiding this comment.
Does anybody know for what those are used?
This actually just hides the issue than "fix" it see:
No idea, actually BND should generate this automatically as far as I know, so I would just reuse that. |
That's right. But since BND (should) not do anything in this regard that let workarund lets the build suceed.
By default BND creats imports that only specify Major and minor Version bounds, which leaves room for smaller service Versions. But I would Say a Lob should not self-Import potentially older versions. |
Why? the contract is that is is compatible for providers, otherwise an import of API would not make much sense if it can only replace the exact version. |
I might be confused by as you wrote:
so I assumed you want to generate those |
Only use the maven-bundle-plugin to generate all OSGi metadata into slf4j's Manifest.MF files. This unifies the resulting MANIFEST.MFs that are currently partly generated and partly statically defined and fixes the following aspects: - Restore the Bundle-SymbolicName from SLF4J-2.0.5 and before - Removes Exported-Packages: META-INF.versions.9 - Restricts the slf4j self import versions to [<current-version>,3) - Add Automatic-Module headers for those artifacts that don't have a module-info.java - Replace the deprecated 'Bundle-RequiredExecutionEnvironment' by a corresponding required 'osgi.ee' capability. - Fix the package-export of slf4j-jdk-platform-logging (from 'slf4j.jdk.platform.logging' to 'org.slf4j.jdk.platform.logging') - Adds the 'Main-Class' header for the migrator artifact. Because the Manifest.MF did not have a terminating new-line it was invalid and its content was ignored! Signed-off-by: Hannes Wellmann <wellmann.hannes1@gmx.net>
... using bnd-annotation for all SLF4JServiceProvider implementations, which are loaded using the ServiceLoader mechanism. Signed-off-by: Hannes Wellmann <wellmann.hannes1@gmx.net>
HannesWell
force-pushed
the
automate_osgi_metadata
branch
from
c6f8161
to
7ecfc23
Compare
I was thinking about the case where a client explicitly imports But I think in that case the client should either ensure that only the intended version is present in the TP or that at least there are no other (version) restrictions that prevent the OSGi resolver from wiring to the latest version possible. Therefore I just stick with the defaults now.
No that was not my goal. But if @ceki is interested and BND supports multi-release jars, that's IMHO something to consider. |
|
@HannesWell Given new SLF4J packages are added infrequently, automation of OSGi metadata creation is not a priority at this time. Adding a new dependency on "biz.aQute.bnd.annotation" will not fly. As such I am closing this PR without merging. |
|
@HannesWell Notwithstanding the above, thank you very much for this PR. |
From #320 (which I noticed after creating this PR) I already expected your dislike of using the bnd-annotations in slf4j and therefore updated this PR and extracted the usage of said annotations in a separate commit of this PR. This is the full list of enhancements that come with this PR from my initial comment:
You may also want to consider #328 before this one, which contains the very first commit of this PR that only contains the formatting changes. |
|
@ceki is that an architectural or financial issue?
|
|
@stbischof Here is a fuller quote:
I don't see the need to fix something which is (at a cursory look) not broken. |
|
The main benefit for us here is to improve osgi metadata with an automatic build and to secure it with a baseline. Bnd annotations not required. And we may are able to sponsor this. |
It is currently broken as described in my last comment. Some things were broken recently (the Therefore I asked you to reopen this PR to discuss how to make this change fine for you. :) |
|
@HannesWell Can you please open a new minimal PR and propose changes to just one module, say slf4j-api. |
Sure I can create a new PR and strip the changes of this one down to the required minimum. But I think it requires more changes to apply it only to sl4j-api module instead of all, especially since almost all modules are affected by in some way. |
|
Created #330. |
This is a follow up PR to #324 to only use the
maven-bundle-pluginto generate all OSGi metadata into slf4j's Manifest.MF files. This unifies the resulting MANIFEST.MFs that are currently partly generated and partly statically defined and fixes the following aspects:Bundle-SymbolicNamefrom SLF4J-2.0.5 and beforeGenerate the OSGi 'Service Loader Mediator' capabilities for all SLF4JServiceProviders using BND-ToolsMETA-INF.versions.9[<current-version>,3)Require-BundlebyImport-PackageSome of the fixed issues were introduced with #324 (mainly the changed BSN), some existed before.
I'm currently unsure if it is sufficient to have Import-Package version ranges for the slf4j packages that only contain the minor version (like
version="[2.0,3)") or if it should be better also include the current service/micro version in the lower bound (likeversion="[2.0.7,3)". That latter one is what this PR is currently doing, but the former would be simpler because it is what BND-tools (which the maven-bundle-plugin uses under the hood) is doing by default. However my first impuls would be that a library should never (self-)import a lower version of itself.@stbischof and @timothyjward you made some OSGi related enhancements of SLF4J in the recent past and are maybe also interested in this change.
@ceki do I have to create a extra ticket for this?