Add multi-release support to aQute.bnd.osgi.Jar

[laeubi]

Currently the aQute.bnd.osgi.Jar is not a capable of processing
multi-release jars in a convenient way, especially if one likes to
support the multi-release modules support as well as the multi-release
OSGi manifest part.

This adds first-class support of multi-release jar to the
aQute.bnd.osgi.Jar other modules can build on top in a way as the
JarFile#getJarEntry would behave, that is a resource that is requested
is returned for the most specific selected release.

Relates to:

• OSGi Java 9 support: Multi-release JAR #2227
• Calculate moduleName for multi-release-jar-dependencies #5327
• Use BND for generating of Use bnd-jar-plugin to generate a proper ClickHouse/clickhouse-java#1009

[laeubi]

@bjhargrave (or any other committer) would it be possible to approve the workflow run to see if this breaks anything?

[laeubi]

The rerun currently fail for some test-cases with:

java.lang.NoClassDefFoundError: org/junit/Assert
	at aQute.xlaunchpad.LaunchpadConfigurationTest.testConfiguration(LaunchpadConfigurationTest.java:50)
Caused by: java.lang.ClassNotFoundException: org.junit.Assert not found by biz.aQute.launchpad.tests [9]
	at org.apache.felix.framework.BundleWiringImpl.findClassOrResourceByDelegation(BundleWiringImpl.java:1597)

any hint whats going on?

The CodeQL seem to complain about lines that where not changed by this PR are there any config files that need to be adjsuted because of changed code-positions?

[laeubi]

@rotty3000 can you maybe take a look at this as this would fix some of the issues with JPMS generation as well and would be the basis for other enhancements.

[rotty3000]

This is the kind of low level change I really would like @bjhargrave to have a pass over. Let's please wait until he's available to review.

[laeubi]

@rotty3000 maybe you can approve the workflow run in the meanwhile and maybe help with the CodeQL and rerun issue?

[rotty3000]

@rotty3000 maybe you can approve the workflow run in the meanwhile and maybe help with the CodeQL and rerun issue?

Done!

[laeubi]

@rotty3000 Thanks! Any idea where/what need to be adjusted to make the CodeQL happy? DO you have a hint what might be wrong with the "rerun" job? I don't really understand what it does and how the failure could be related to my changes.

[rotty3000]

Code QL infractions are here https://github.com/bndtools/bnd/pull/5331/checks?check_run_id=7727025812

The rerun job builds bnd with the just-built snapshot of bnd. Bnd must be able to build itself. We do that to prove, among other things, that we haven't broken consumers.

[laeubi]

Code QL infractions are here

I have never used Code QL before so these messages are a bit mysterious for me, is there a config file or something for CodeQL? Looking at the messages some might be because of changed code locations ("5 analyses not found").

[laeubi]

@bjhargrave can you review this? I have at least two external libs that would require multi-release support (and will see probably more in the future).

[bjhargrave]

I took a quick look at this PR and I don't see how a user can build a multi release jar using a bnd file. The change seems to be just API changes (which include a breaking change/major version bump which is an issue).

As far as I can see, the proposed change sets a "release" state which is used so that subsequent puts end up in the release folder. We need a solution that enables the user to build a multi-release jar using instructions in a bnd file. The user should not have to write code to call bnd api.

So I don't approve of merging this PR in it current state.

[laeubi]

@bjhargrave please see the linked PR that shows how one can build a multi-release jar:

https://github.com/laeubi/clickhouse-jdbc/blob/a75fec67450a9c9d1af4cdd58faee24258102c21/pom.xml#L583-L622
https://github.com/laeubi/clickhouse-jdbc/blob/a75fec67450a9c9d1af4cdd58faee24258102c21/pom.xml#L1006-L1022

[bjhargrave]

please see the linked PR that shows how one can build a multi-release jar

I still don't see any bnd file instructions. That PR also uses bnd-process mojo. What about all other places using bnd? This is why using bnd file instructions are preferred. They can be used by all drivers of bnd (eclipse, maven, gradle).

For example, configuring bnd for multi-release mode should be done by the bnd file entry Multi-Release: true, not by a new maven (or gradle) configuration.

[laeubi]

I still don't see any bnd file instructions.

You don't see any because they are not required.

That PR also uses bnd-process mojo. What about all other places using bnd?

What do you mean by "all other places"? The advantage of this aproach is that nothing else needs change/adjusted as every other BND code only see whats relevant for this release, this also fixes the "class at wrong folder" problem.

This is why using bnd file instructions are preferred. They can be used by all drivers of bnd (eclipse, maven, gradle).

Sure if anyone likes to add a BND instruction as well and we can do it in one step, why not, this adds the basic API required to correctly process multi-release jars (currently BND do not process them correctly and throws errors instead) and enhances the maven-bnd- plugin so it can be used to create a multi-release jar with standard maven techniques to demonstrate the usage.

eclipse

Eclipse currently can't build multi-release jars anyways.

For example, configuring bnd for multi-release mode should be done by the bnd file entry Multi-Release: true, not by a new maven (or gradle) configuration.

Why not? This is how maven supports building multi-release jars, and is like javac handles the --release flag, if additional there processing are required/desired it could be of course added later on.

[laeubi]

Please also note that the <release> option might even be used if you not build a multi-release jar, but want to compile code for a specific release. Currently BND simply ignores any other release than the default release leading to wrong manifests and/or metadata generated when it comes to analyze library jars that are multi-release jars see for example:

• Calculate moduleName for multi-release-jar-dependencies #5327

[bjhargrave]

I still don't see any bnd file instructions.

You don't see any because they are not required.

My point is that all bnd configuration should be done through bnd instructions so that it can work in all places bnd is used.

That PR also uses bnd-process mojo. What about all other places using bnd?

What do you mean by "all other places"?

Gradle plugin, Bndtools in Eclipse, maven-bundle-plugin, etc.

The advantage of this aproach is that nothing else needs change/adjusted as every other BND code only see whats relevant for this release, this also fixes the "class at wrong folder" problem.

This is why using bnd file instructions are preferred. They can be used by all drivers of bnd (eclipse, maven, gradle).

Sure if anyone likes to add a BND instruction as well and we can do it in one step, why not, this adds the basic API required to correctly process multi-release jars (currently BND do not process them correctly and throws errors instead) and enhances the maven-bnd- plugin so it can be used to create a multi-release jar with standard maven techniques to demonstrate the usage.

eclipse

Eclipse currently can't build multi-release jars anyways.

Bndtools in Eclipse builds jars. So if bnd is updated to build multi-release jars, then Bndtools must be able to take advantage of this.

For example, configuring bnd for multi-release mode should be done by the bnd file entry Multi-Release: true, not by a new maven (or gradle) configuration.

Why not? This is how maven supports building multi-release jars, and is like javac handles the --release flag, if additional there processing are required/desired it could be of course added later on.

Because this is how bnd rolls. We configure through bnd instructions so that that many drivers (Bndtools in Eclipse, bnd maven plugins, bnd gradle plugins, maven-bundle-plugin, ...) can take advantage of new function.

I will also note this PR is a breaking change to the Jar API in that some public methods have changed return types and other public methods no longer return mutable objects. So this alone is a reason to not merge this PR.

[laeubi]

My point is that all bnd configuration should be done through bnd instructions so that it can work in all places bnd is used.

I can add a -relaseflag to the BND instructions, but multi-release jar is about packing jars and not really about instructions to generate metadata/files.

Bndtools in Eclipse builds jars. So if bnd is updated to build multi-release jars, then Bndtools must be able to take advantage of this.

It can do as soon as BNDTools allows compiling for multiple releases, but BNDTools is out of scope of this ... thats why I add a simple API to the analyzer so everyone who thinks can process this can use this and thats what the change to the bnd-maven-plugin do, it calls the API in a maven way :-)

Because this is how bnd rolls. We configure through bnd instructions so that that many drivers

Well that's not really true, just take a look at https://github.com/bndtools/bnd/blob/master/maven/bnd-maven-plugin/README.md#configuration-parameters there is a total of nine parameters not configured through a BND file...

I will also note this PR is a breaking change to the Jar API in that some public methods have changed return types and other public methods no longer return mutable objects.

Sorry but thats a bit confusing, I though versioning things is to evolve things, if you never change things why version things?
Beside that, the API nowhere states anything about mutability (actually it is not documented at all) so anyone depending on that is relying on implementation details.

So this alone is a reason to not merge this PR.

Feel free to prose a better way then, the fact is that Its over 5(!) years now that this support is missing and actually there are even bugs in BND caused by its wrong handling of multi-release jars, so what else could qualify for a major change?

This is completely hilarious, when one asks for a feature its said "please add it your own" and when one is proposing a PR its said "we can't merge it because it has changes" ...

[bjhargrave]

My point is that all bnd configuration should be done through bnd instructions so that it can work in all places bnd is used.

I can add a -relaseflag to the BND instructions, but multi-release jar is about packing jars and not really about instructions to generate metadata/files.

There are already bnd instruction for the packaging such as compression, signing, etc.

Bndtools in Eclipse builds jars. So if bnd is updated to build multi-release jars, then Bndtools must be able to take advantage of this.

It can do as soon as BNDTools allows compiling for multiple releases, but BNDTools is out of scope of this

It is not out of scope. While an individual eclipse project can have only one compiler target release, Bnd can assemble a jar out of the output of many projects. So one project can make Java 8 class and another can make Java 11 classes. And these can be packaged in to a single jar by Bndtools.

... thats why I add a simple API to the analyzer so everyone who thinks can process this can use this and thats what the change to the bnd-maven-plugin do, it calls the API in a maven way :-)

Because this is how bnd rolls. We configure through bnd instructions so that that many drivers

Well that's not really true, just take a look at https://github.com/bndtools/bnd/blob/master/maven/bnd-maven-plugin/README.md#configuration-parameters there is a total of nine parameters not configured through a BND file...

Those are maven plugin config not bnd config. If you want to configure the exports, you need to use bnd config.

I will also note this PR is a breaking change to the Jar API in that some public methods have changed return types and other public methods no longer return mutable objects.

Sorry but thats a bit confusing, I though versioning things is to evolve things, if you never change things why version things? Beside that, the API nowhere states anything about mutability (actually it is not documented at all) so anyone depending on that is relying on implementation details.

We take backwards compatibility fairly seriously. The next breaking changes may come with Bnd 7 when we move to Java 17 as a base language level. But changing the API to change the return values may still be a bridge too far. It would have to be agreed to by the broader set of bnd maintainers.

So this alone is a reason to not merge this PR.

Feel free to prose a better way then, the fact is that Its over 5(!) years now that this support is missing and actually there are even bugs in BND caused by its wrong handling of multi-release jars, so what else could qualify for a major change?

This is completely hilarious, when one asks for a feature its said "please add it your own" and when one is proposing a PR its said "we can't merge it because it has changes" ...

I think you know how open source works :-)

[laeubi]

The next breaking changes may come with Bnd 7 when we move to Java 17

Then it seems a good opportunity to start with BND 7 right now.

and other public methods no longer return mutable objects

Just to make clear, the current implementation:

1. Allows to put the jar in an inconsistent state because I can modify one of two close related collections
2. Allows to circumvent close() checks, might be irrelevant but why the add all those close checks?
3. Allows to circumvent path checks ("Zip Slip"), might be a security risk

so actually these are all bugs, beside that it is bad to expose internal state of an object to the outside.

If you want to configure the exports, you need to use bnd config.

I can simply use OSGi bundle annotations so no BND config required, even though BND warns me if I do not use one, anyways the point it that some things are controlled by maven and still influence the result.

It is not out of scope. While an individual eclipse project can have only one compiler target release, Bnd can assemble a jar out of the output of many projects. So one project can make Java 8 class and another can make Java 11 classes. And these can be packaged in to a single jar by Bndtools.

Well then the only think left would be to adjust Bndtools to set for each compiled java version the appropriate release flag and generate a manifest for each of those, but this is different from how maven works here where all sources, classes and generated code are part of one project. So however it handles that, it could be added if someone cares enough to enable multi-release support for BNDTools.

I think you know how open source works :-)

I'm not sure that I understand, do you want to say:

• It is usual property of open source project to not solve relevant bugs for years but reject any fix that finally would enables to solve them?
• BND is open source but not open to contributions?
• I should just fork BND and simply release my own version?
• ...?

[timothyjward]

adjust Bndtools to set for each compiled java version the appropriate release flag and generate a manifest for each of those

For proper multi-release support we do need to generate a partial manifest for each release folder, adding in dependency information for Import-Package and/or Require-Bundle. See https://docs.osgi.org/specification/osgi.core/8.0.0/framework.module.html#framework.module-multireleasejar

This is actually done in Apache Aries JAX-RS Whiteboard to manage a breaking change between Java 8 and Java 9.

[laeubi]

For proper multi-release support we do need to generate a partial manifest for each release folder, adding in dependency information for Import-Package and/or Require-Bundle

This change actually support exactly this but not only for a single jar but also for transitive dependencies.

[laeubi]

This jar (need to rename to jar because of file type restrictions) is generated by BND with this change. It uses java 8, java 9 and java 11 alternative versions, each with a proper maninifest, just note that the 9 version currently has a small bug see

• osgi.ee computation needs to take modules-info.class into account #5334

and thus should actually have EE=9 in this case...
clickhouse-http-client-0.3.3-SNAPSHOT.zip

[laeubi]

It seems I was to enthusiastic here and more smaller steps are required, I therefore created:

• [bnd] Support processing of Multi-Release-Jars #5346
• [jar] the data integrity of bnd Jar at risk because it exposes internal datastructures and allows bypassing sanity checks #5344

to track two aspects of this PR so they might be solved in smaller chuncks than I could provide with this PR

[rotty3000]

thank you for being patient with us @laeubi . The approach of smaller chunks is really appreciated.