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

[bnd] Support processing of Multi-Release-Jars #5346

Closed
laeubi opened this issue Aug 19, 2022 · 7 comments
Closed

[bnd] Support processing of Multi-Release-Jars #5346

laeubi opened this issue Aug 19, 2022 · 7 comments
Assignees

Comments

@laeubi
Copy link
Contributor

laeubi commented Aug 19, 2022

This is related to

But only handles a subset, that is, support processing of Multi-Release-Jars.

Currently when one tries to process a Multi-Release-Jar with BND, the bnd-maven-plugin complains with the following message:

[ERROR] Failed to execute goal biz.aQute.bnd:bnd-maven-plugin:6.4.0-SNAPSHOT:bnd-process (bnd-process) on project clickhouse-client: Classes found in the wrong directory: {META-INF/versions/9/module-info.class=module-info}

The problem is, that BND currently do not read Multi-Release-Jar correctly leading to issues else where

One problem is, that Multi-Release-Jar processing requires to know what are the java release, this usually is taken from either the running VM or by specify it directly, e.g by a compiler switch.

Therefore I propose, as a first step, to allow specify a new directive in a bnd file -release that takes an integer and is interpreted as the desired target release.

BND should then work in the following way:

  • if no -release is specified, it should behave as of today, e.g. Multi-Release-Jars processing is disabled and might even lead to the current problems
  • if -release is specified and it is smaller than 0 it is interpreted as if it was not specified
  • if -release is specified and it is smaller than 9 it is interpreted as processing a jar with the default jar content that is ignoring anything from META-INF/versions and thus also don't find any modules info, or classes and do not complain
  • if -release is specified and it is larger or equal to 9 then it is interpreted as processing a jar with the given release, that is any class/resource in META-INF/versions/... that are smaller or equal to <release> will shade any class/resource from the default jar content

This will ensure:

  1. Current users of BND won't be affected and it would be 100% backward compatible
  2. Users that demand Multi-Release-Jar can at least read and process jars with the desired release option
laeubi added a commit to laeubi/bnd that referenced this issue Aug 19, 2022
Currently bnd is not a capable of processing multi-release jars, this
adds support of multi-release processing by a new directive -release
where one can select what release version should be processed by bnd.

Fixes bndtools#5346
laeubi added a commit to laeubi/bnd that referenced this issue Aug 19, 2022
Currently bnd is not a capable of processing multi-release jars, this
adds support of multi-release processing by a new directive -release
where one can select what release version should be processed by bnd.

Fixes bndtools#5346
laeubi added a commit to laeubi/bnd that referenced this issue Aug 19, 2022
Currently bnd is not a capable of processing multi-release jars, this
adds support of multi-release processing by a new directive -release
where one can select what release version should be processed by bnd.

Fixes bndtools#5346

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
laeubi added a commit to laeubi/bnd that referenced this issue Aug 19, 2022
Currently bnd is not a capable of processing multi-release jars, this
adds support of multi-release processing by a new directive -release
where one can select what release version should be processed by bnd.

Fixes bndtools#5346

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
laeubi added a commit to laeubi/bnd that referenced this issue Aug 19, 2022
Currently bnd is not a capable of processing multi-release jars, this
adds support of multi-release processing by a new directive -release
where one can select what release version should be processed by bnd.

Fixes bndtools#5346

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
@rotty3000
Copy link
Contributor

I generally like this direction, however, I wonder if bnd could be even smarter than that.

My question then would be @laeubi do you think there is a scenario where if bnd were able to automatically determine the applicable releases given a Jar's contents would that break something unexpectedly?

@laeubi
Copy link
Contributor Author

laeubi commented Aug 22, 2022

I wonder if bnd could be even smarter than that.

Sure, the current approach is just as minimal and "dumb" as possible,mainly to not change too much and thus not requiring any (major) version bumps and my previous attempt even contained a list of discovered versions inside a jar, so one can think about the following:

  1. find the minimal EE of the jar to be analyzed, lets assume it is 11
  2. find any versioned classes in the jar, lets assume it is 17
  3. No check all jars on the analyzer classpath, lets say it includes 9 and 14

So a "smart" way would be to assume we need:

  • one computation with release = 11 for the default manifest
  • a partial one with release = 14
  • a partial one with release = 17

but such an automatic approach would that requires much more attention e.g. for the analyzer it needs some kind of loop in "analyze", one for each release to be computed, and probably reset any cached states I don't know how good e.g. Analyzer plugins are prepared for multiple invocations with slightly different content returned.

laeubi added a commit to laeubi/bnd that referenced this issue Aug 23, 2022
Currently bnd is not a capable of processing multi-release jars, this
adds support of multi-release processing by a new directive -release
where one can select what release version should be processed by bnd.

Fixes bndtools#5346

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
laeubi added a commit to laeubi/bnd that referenced this issue Aug 23, 2022
Currently bnd is not a capable of processing multi-release jars, this
adds support of multi-release processing by a new directive -release
where one can select what release version should be processed by bnd.

Fixes bndtools#5346

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
laeubi added a commit to laeubi/bnd that referenced this issue Aug 23, 2022
Currently bnd is not a capable of processing multi-release jars, this
adds support of multi-release processing by a new directive -release
where one can select what release version should be processed by bnd.

Fixes bndtools#5346

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
laeubi added a commit to laeubi/bnd that referenced this issue Aug 24, 2022
Currently bnd is not a capable of processing multi-release jars, this
adds support of multi-release processing by a new directive -release
where one can select what release version should be processed by bnd.

Fixes bndtools#5346

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
@HannesWell
Copy link
Contributor

Is there any progress planned on this issue?
This is for example a blocker to use the bnd-maven-plugin in qos-ch/slf4j#327.

@bjhargrave
Copy link
Member

It is still on my work queue. I have a local feature branch but the work is still in progress.

@laeubi
Copy link
Contributor Author

laeubi commented Dec 15, 2022

@laeubi laeubi closed this as completed Dec 15, 2022
@laeubi laeubi reopened this Dec 15, 2022
@stbischof
Copy link
Contributor

This multi Release jar thing is also a Blocker on our site. Some os project's love the multi release jars. They would accept bnd als build tool but its blocked by multi release support.

pkriens added a commit to pkriens/bnd that referenced this issue Feb 21, 2023
There has been a long history how to support MRJs (Multi Release Jars). I kind of get it since it seems a giant magnet for complications, but alas, progress.

The main problem with the current approach is that the support was focused on the Jar file. Imho, (and I think BJ's) this is the wrong place. The Jar abstracts working with the ZIP file but should imho not contain any semantics of the class path. We already placed the Bundle-Classpath (which is a very similar problem) in the Analyzer.

This morning I had an interesting discussion with Christopher :-)  He really helped me to see the problem more clear (I hope). I think we need to separate this problem strictly in 2 parts. The first is the MJR read support. This is this PR. Write will come later.

During the discussion it became clear to me that for our build (remember, I am ignoring writing MJRs) we need a _view_ on the MJR that flattens the namespace of the resources. To be able to flatten, we need a max release for the view. I am a bit fuzzy on the use cases but I am convinced you need to be able specify this max release per JAR if you need to but for convenience, we need a setting per builder/project and default default. I've come up with the following rules:

* In a -*path (e.g. -buildpath) you can set a MRJ.
* You can add an attribute like `-java.release.max=10` to the file in the buildpath. This will create a view up to and including release 10. I.e. what the VM would see if it was a Java 10 VM.
* In absence of this attribute, a default can be set in the workspace or project with the same property `-java.release.max=17`
* In absence of a default, the Jar is flattened for all the release it contains. This may mean that the compiler cannot read certain class files.

I am open to another strategy if someone has ideas. I am not sure I like the property name. We could use javac.target but I feel a tad uncomfortable with that. However, during the discussion it became crystal clear to me that this was not a Jar quality because the view that is used depends on the triplet of MRJ layout, max release, and the Jar.

So I've implemented this accordingly in this PR. Quick feedback is appreciated because Christopher has been hanging in there for a very long time.

---
 Signed-off-by: Peter Kriens <Peter.Kriens@aQute.biz>

Signed-off-by: Peter Kriens <Peter.Kriens@aQute.biz>
@pkriens pkriens self-assigned this Feb 24, 2023
@pkriens pkriens removed the waiting label Feb 24, 2023
@pkriens
Copy link
Member

pkriens commented Mar 17, 2023

I am closing this as done. The test case for maven does not work yet but that is an open PR and does not seem to affect the current implementation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants