bndtools / bnd

Bnd/Bndtools. Tooling to build OSGi bundles including Eclipse, Maven, and Gradle plugins.
https://bndtools.org
Other
528 stars 304 forks source link

OSGi Java 9 support: Multi-release JAR #2227

Closed njbartlett closed 1 year ago

njbartlett commented 6 years ago

This issue was raised against Felix (maven-bundle-plugin) but it needs to be addressed in bnd: https://issues.apache.org/jira/browse/FELIX-5592

njbartlett commented 6 years ago

See also https://issues.apache.org/jira/browse/FELIX-5527

bjhargrave commented 6 years ago

Some more detail on what "support" for multi release jars would look like in OSGi is needed here.

Bnd could certain stop objecting to classes in META-INF/versions/9 but that would not make them available at runtime IF the runtime is Java 9.

This topic was discussed in a recent OSGi expert group call and the OSGi "way" here would be to use a fragment to supply the types for a java version with the host bundle carving out a space in the Bundle-Classpath.

But if Bnd is being fed a target/classes folder with classes in META-INF/versions/9, I am not sure how Bnd can make a bundle which will work as expected for multi release jars.

bjhargrave commented 6 years ago

Also, if classes in META-INF/versions/9 exist, should Bnd process them for imports? The imports for the bundle may vary depending upon whether the classes in META-INF/versions/9 are "in effect". And there is no way to have imports conditional on the version of Java at runtime. This is why fragments to supply these types is more appropriate in OSGi since the fragments can supply additional imports (and exports) for the types.

bjhargrave commented 6 years ago

How do JPMS modules and multi-release jars interact? I assume they are mutually exclusive since JPMS modules would have the same issues with varying requires/exports depending upon the runtime version of Java. Yes, this is not a practical issue yet for JPMS since it is only Java 9 for now. But with Java 10, then what?

njbartlett commented 6 years ago

Unfortunately the version-specific part under META-INF/versions can override the non-version-specific classes. It seems that a multi-release JAR would have to result in at least a base bundle plus one fragment for Java 9 and a further fragment for the pre-Java-9 code. We might be able to optimise this if we detect that there is actually no overlap.

njbartlett commented 6 years ago

You can have module-info.class under META-INF/versions. So you could have entirely different dependencies for Java 9, 10, etc.

bjhargrave commented 6 years ago

We don't need a fragment for pre-Java 9 code. The host bundle would have all the pre-9 code and Bundle-Classpath: versioned, . and the fragment would have all its code in the versioned folder.

njbartlett commented 6 years ago

Okay nice idea with Bundle-ClassPath.

bjhargrave commented 6 years ago

You can have module-info.class under META-INF/versions. So you could have entirely different dependencies for Java 9, 10, etc.

Hmm, so bnd could put different manifests under META-INF/versions and the framework would pick a manifest at runtime? (sigh)

META-INF/MANIFEST.MF (java 8 manifest)
META-INF/versions/9/META-INF/MANIFEST.MF (java 9 manifest)
karlpauls commented 6 years ago

The problem with the bundle classpath is that it needs to be conditional. So you would need an entry for each existing java version.

njbartlett commented 6 years ago

Yes I think that might need to be the way forward... runtime support for fragments nested inside a bundle. That way projects like log4j can just release a single JAR, which is what they want to do.

bjhargrave commented 6 years ago

The problem with the bundle classpath is that it needs to be conditional. So you would need an entry for each existing java version.

No. There will be multiple fragments which only resolve for the specific java version.

karlpauls commented 6 years ago

No. There will be multiple fragments which only resolve for the specific java version.

But they need to override each other.

bjhargrave commented 6 years ago

Yes I think that might need to be the way forward... runtime support for fragments nested inside a bundle. That way projects like log4j can just release a single JAR, which is what they want to do.

I think there are many issues here. Like how does Bundle.getEntry/getEntries treat this?

bjhargrave commented 6 years ago

But they need to override each other.

No. At most one of the fragments is attached and it supplies classes in the versioned folder.

karlpauls commented 6 years ago

No. At most one of the fragments is attached and it supplies classes in the versioned folder.

So you would duplicate all classes from the other versions that are not overriden in this version in each fragment in the chain?

rotty3000 commented 6 years ago

Could we add some new "effective"-ness' which map to Java version X?

Import-Package: \ com.foo,\ com.bar;effective=resolve-java9

Plus the Bundle-ClassPath?

On Wed, Dec 6, 2017 at 6:09 PM, BJ Hargrave notifications@github.com wrote:

You can have module-info.class under META-INF/versions. So you could have entirely different dependencies for Java 9, 10, etc.

Hmm, so bnd could put different manifests under META-INF/versions and the framework would pick a manifest at runtime? (sigh)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bndtools/bnd/issues/2227#issuecomment-349805690, or mute the thread https://github.com/notifications/unsubscribe-auth/AAI9TKR4CisE1GdlTDIRmODvyhf5jsqbks5s9x6ngaJpZM4Q4tuA .

-- Raymond Augé http://www.liferay.com/web/raymond.auge/profile (@rotty3000) Senior Software Architect Liferay, Inc. http://www.liferay.com (@Liferay) Board Member & EEG Co-Chair, OSGi Alliance http://osgi.org (@OSGiAlliance)

karlpauls commented 6 years ago

the thing is you can have:

/A.class /9/B.class /10/C.class /11/B.class

bjhargrave commented 6 years ago

So you would duplicate all classes from the other versions that are not overriden in this version in each fragment in the chain?

Why would you need to do that? Does multi release jars stack up all the versions? So that on Java 11, you search 11, then 10, then 9, then the base jar for a class?

karlpauls commented 6 years ago

Why would you need to do that? Does multi release jars stack up all the versions? So that on Java 11, you search 11, then 10, then 9, then the base jar for a class?

Yes

bjhargrave commented 6 years ago

Yes

That is in insane design. Good luck building such a jar and keeping it operational in all the versions of java...

And how do you build all the module-infos for all these versions of java (if you use JPMS)?

karlpauls commented 6 years ago

At least that is what I think http://openjdk.java.net/jeps/238 is saying

karlpauls commented 6 years ago

In the example above, when running on an MRJAR-aware Java 9 JDK, it would see the 9-specific versions of A and B and the general versions of C and D; on a future MRJAR-aware Java 10 JDK, it would see the 10-specific version of A and the 9-specific version of B;

So i would say yes, they stack.

njbartlett commented 6 years ago

I think the structure would look like this:

A.class (pre-Java 9 class)
B.class (pre-Java 9 class)
META-INF/
\- MANIFEST.MF:
      Bundle-SymbolicName: foo
      Bundle-ClassPath: versioned, .
      Require-Capability: osgi.ee; filter:='(osgi.ee=JavaSE-1.8)'
\- versions
  \- 9
      \- A.class (java 9 class)
         B.class (java 9 class)
         META-INF/MANIFEST.MF:
             Fragment-Host: foo
             Require-Capability: osgi.ee; filter:='(osgi.ee=JavaSE-9)'
  \- 10
      \- A.class (java 10 class)
         B.class (java 10 class)
         META-INF/MANIFEST.MF:
             Fragment-Host: foo
             Require-Capability: osgi.ee; filter:='(osgi.ee=JavaSE-10)'

A smaller problem is that the fragment for Java 9 would resolve on Java 10 because EEs include every previous EE. We would have to express a dependency on the EE of "exactly Java 9".

karlpauls commented 6 years ago

Part of me wonders if the solution is to really make them fragments inside the bundle. I.e., if a dir on the bundle-classpath has a META-INF/MANIFEST.mf we should just handle it like a fragment.

karlpauls commented 6 years ago

A smaller problem is that the fragment for Java 9 would resolve on Java 10 because EEs include every previous EE. We would have to express a dependency on the EE of "exactly Java 9".

No, I think we want exactely that.

karlpauls commented 6 years ago

Bundle-Classpath: META-INF/versions/10,META-INF/versions/9,.

and then what you just proposed as layout.

bjhargrave commented 6 years ago

I think we need to stop fixing this in this bug. This is sounding more than Bnd can address alone. Changes are need in the Core spec to address this. And then Bnd can support the Core changes.

karlpauls commented 6 years ago

I think we need to stop fixing this in this bug. This is sounding more than Bnd can address. Changes are need in the Core spec to address this. And then Bnd can support the Core changes.

This is what I told David last week and what made him bring it up in your call (and because I had the feeling that wasn't the conclusion of the call I pinged Neil about it today :-)

njbartlett commented 6 years ago

Bundle-Classpath: META-INF/10,META-INF/9,.

Yes I think this works, and bnd can generate that. It becomes tricky when the user wants to use Bundle-ClassPath for something else as well, but maybe that should just be disallowed.

Agree with BJ that bnd should follow CPEG.

karlpauls commented 6 years ago

Yes I think this works, and bnd can generate that. It becomes tricky when the user wants to use Bundle-ClassPath for something else as well, but maybe that should just be disallowed.

But that probably would still just work as you could keep adding stuff to it (just need to have the version dirs first).

Anyways, I agree this needs spec support.

njbartlett commented 6 years ago

In the short term can we disable the error when bnd finds classes under META-INF? In this case bnd produces no output, and projects like log4j and JBoss are already removing maven-bundle-plugin from their builds, which affects all of their users, not just the subset of users that have moved to Java 9.

bjhargrave commented 6 years ago

In the short term can we disable the error when bnd finds classes under META-INF?

Sure, we can do that. But they can do that themselves right now with a -fixupmessages instruction. No need to wait for Bnd 4.0.

karlpauls commented 6 years ago

In the short term can we disable the error when bnd finds classes under META-INF?

Yes please 👍

Sure, we can do that. But they can do that themselves right now with a -fixupmessages instruction.

Interesting, maybe that is something to include in the maven-bundle-plugin by default...

bjhargrave commented 6 years ago

The most rapid path to happiness is to use the proper -fixupmessages instruction to elide the error message. You can do that asap. Not sure I would bake that into the plugin, but rather tell people how to add it to their configuration.

bjhargrave commented 6 years ago

Also, eliding the error message just wallpapers over the problem. They will end up with a bundle which works on Java 8 but may not work on Java 9 since the Java 9 classes are not in effect.

njbartlett commented 6 years ago

Yes agreed, any advice we give about using fixupmessages should make that clear, and make the error into a warning rather than make it go away entirely.

bjhargrave commented 6 years ago
<_fixupmessages>"Classes found in the wrong directory"</_fixupmessages>

will just throw the error away. If you want to convert the error to a warning, then do

<_fixupmessages>"Classes found in the wrong directory";is:=warning</_fixupmessages>
njbartlett commented 6 years ago

The following works, I tested against the exemplar project posted by Mark Raynsford:

      <configuration>
        <instructions>
          <_fixupmessages>"Classes found in the wrong directory";is:=warning</_fixupmessages>
        </instructions>
      </configuration>
njbartlett commented 6 years ago

will just throw the error away. If you want to convert the error to a warning, then do

Yes sorry BJ, I noticed my mistake and deleted the comment.

rgoers commented 6 years ago

I had to use

<_fixupmessages>"Classes found in the wrong directory";is:=warning,"Invalid class file module-info.class";is:=warning,"Exception: java.lang.ArrayIndexOutOfBoundsException: 19";is:=warning</_fixupmessages>

to get around the problems.

njbartlett commented 6 years ago

@rgoers That suggests you are using an old version of bnd. Version 3.4+ supports processing the module-info.class file.

rgoers commented 6 years ago

Yes. The latest maven-bundle-plugin ships with version 3.3.0. I will override that and give it a try.

rotty3000 commented 6 years ago

it would be nice to have a link to the exemplar project referenced here so we can take a look.

bjhargrave commented 6 years ago

BTW, the OSGi Core R7 spec now includes spec support for multi-release JARs. See https://osgi.org/specification/osgi.core/7.0.0/framework.module.html#framework.module-multireleasejar and https://osgi.org/specification/osgi.core/7.0.0/framework.module.html#framework.module-multireleasecontainer.

Also see the OSGi blog post for an overview of the multi-release JAR support in OSGi Core R7.

gjoranv commented 6 years ago

Besides the error message about class files in illegal location, there is another and more acute problem. I'm using the maven-bundle-plugin (3.5.0), and when I include the latest jaxb-api (javax.xml.bind:jaxb-api:2.3.0), the produced bundle gets the following requirement:

Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=9))"

This despite the fact that jaxb-api itself has this requirement:

Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.7))"

Using javap to find the major version of all class files reveals:

javap -verbose ./META-INF/versions/9/javax/xml/bind/ModuleUtil.class | grep major
  major version: 53

All other classes in the jaxb-api bundle have major version 51 (Java 7), so I assume it's the class file in META-INF/versions that is analysed by bnd and found to require Java 9.

So my bundle ends up with the wrong required osgi.ee, and our OSGi container still running on JDK 1.8, refuses to install the bundle. (We are working on migrating to Java 9 , but it will take time.) I'm working around this by using the -noee flag, but I think there should be a short term fix such that class files in META-INF are disregarded when computing the required version.

Vampire commented 5 years ago

Besides that the OSGi spec now supports modular JARs, even for previous version it would make sense to support this, at least if only the module-info.class is present in version specific folders. In my use-case I have all classes compatible with Java 8, but wanted the JAR to be usable properly as JPMS module, so I have only the module-info.class as version-specific file, so it would work properly with OSGi no matter whether Java 8 or Java 9 as always all classes are available and in the same version except for the module-info.class. Just that I cannot build the bundle due to this error as I switched from the deprecated osgi Gradle plugin to biz.aQute.bnd.builder as recommended while it works perfectly fine with the old one.

I'll try to work-around this with the -fixupmessages trick mentioned here, anyone has done this with the Gradle plugin?

Vampire commented 5 years ago

Ok, got it, if someone needs it, here the Kotlin version:

tasks.jar {
    withConvention(BundleTaskConvention::class) {
        bnd(mapOf(
                "-fixupmessages" to "^Classes found in the wrong directory: \\\\{META-INF/versions/9/module-info\\\\.class=module-info}$"
        ))
    }
}
rotty3000 commented 4 years ago

For bnd implementors: It occurred to me that bnd can easily bucket the classspace in several ways: group by path:

Map<String, List<Clazz>> classspaceByPath = classspace.values()
    .stream()
    .collect(Collectors.groupingBy(Clazz::getAbsolutePath));

group by class version:

Map<Clazz.JAVA, List<Clazz>> classspaceVersion = classspace.values()
    .stream()
    .collect(Collectors.groupingBy(Clazz::getFormat));

Next, each group could be analyzed with an individual Analyzer (perhaps extending from the this analyzer) in order to collect up metadata specific to each group. These would form a hierarchy based on the lower order path (by version) or the lower format version depending on the grouping (maybe the groups could be merged by similar java version heuristic (i.e. lowest EE with the root path, and matching each higher EE to a META-INF/versions/# path.

Starting at the lowest found version you'd analyze as bnd does currently, each subsequently higher EE grouping would be analyzed using the previous analyzer output as part of it's classpath (essentially), and so on.

Each subsequent group would get distinct manifest entries written to appropriate path for that EE.

All of this logic would only obviously get applied if Multi-Release: true.

afester commented 3 years ago

For what its worth, here is the Groovy DSL version of the workaround posted by @Vampire above:

jar {
    bnd('-fixupmessages': '^Classes found in the wrong directory: \\\\{META-INF/versions/9/module-info\\\\.class=module-info}$')
}

Note: I have not tested this concrete line, since I had to adjust the regexp to match my packages - in particular, I was using

jar {
    bnd('-fixupmessages': '^Classes found in the wrong directory: .*')
}

to match the whole error message, independent of the reported packages.