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

Calculated manifest attribute does not respect Java toolchain used in compilation #4779

Closed
pun-ky opened this issue Aug 4, 2021 · 6 comments

Comments

@pun-ky
Copy link

pun-ky commented Aug 4, 2021

Right now I've got a tricky situation. In Gradle AEM Plugin I am using BND tool to OSGify JAR compiled by Gradle. 

https://github.com/wttech/gradle-aem-plugin/blob/master/src/main/kotlin/com/cognifide/gradle/aem/bundle/tasks/BundleJar.kt#L157

Gradle jar task is using different Java (1.8) than it is running Gradle (11) as I am using Gradle toolchains feature.

https://docs.gradle.org/current/userguide/toolchains.html

 In the end classes in BND-produced JAR are compiled using Java 1.8, but MANIFEST.MF has a Created-By attribute with a value valid for Java 11. This is wrong.

As a workaround, as an author of the Gradle AEM Plugin, I need to handle this somehow to stop making people confused when they will see the wrong Java version in the manifest file (true negative situation). I am considering overriding it somehow from the GAP side, to enforce the correct Created-By value according to the Java version used in the compilation - respecting used toolchain / Java version.

Is anything else that could be improved? Maybe in BND itself to handle that scenario more properly? WDYT?

https://github.com/bndtools/bnd/blob/master/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java#L1011-L1012

Thanks, @szymon-owczarzak for helping with investigating this issue.

@szymon-owczarzak
Copy link

image

@bjhargrave
Copy link
Member

Bnd is getting the Java version from the Java runtime calling Bnd. How is this wrong? It has nothing to do with the class file versions in the jar.

You can use -noextraheaders: true to stop Bnd from adding the header if you wish.

@pun-ky
Copy link
Author

pun-ky commented Aug 4, 2021

It's a concept called toolchains available in both Maven and Gradle. This helps with having most recent Java installed globally and project-specific Java installed automatically by Gradle. Eliminates a problem with switching default Java from e.g 8 to 11 and vice versa when we are changing project that we work on.

Yep, I will use that flag. Maybe it's good enough.

@bjhargrave
Copy link
Member

It's a concept called toolchains available in both Maven and Gradle.

I am familiar with toolchains. The Bnd Gradle plugins support for bndrun execution in 6.0.

However, your code calls the buildBundle method in your TaskAction. So Bnd is being called by the Java runtime running your task which is the Java runtime used by gradle. If you want to call buildBundle from a different Java runtime, you will need to handle that in your task implementation.

@kriegfrj
Copy link
Contributor

kriegfrj commented Aug 4, 2021

It's also worth pointing out that, for the most part, Created By is informative rather than functional - it tells you the version of the toolchain that was used to assemble the jar. I could be wrong, but don't know of any tools or platforms that actually depend on the value stored in this header.

The more important functional header is the Require-Capability: osgi.ee directive. I couldn't see it from your screenshot, but this would have been set to Java SE 1.8 rather than 11. If this were not the case, that would be a bug. Bnd uses the maximum bytecode version of the classes in the bundle to calculate this value as it is assembling the bundle (not the version of the toolchain that is doing the assembly) which seems to be what you're expecting.

If you absolutely must have the correct value in the Created By header, you could do as @bjhargrave said and change your build so that the Java 8 toolchain is used to invoke the buildBundle method. Or possibly, as a bit of a hack (I haven't tested this), you could manually specify the Created By header that you want in your manifest as part of your Bnd instructions (Bnd may honor it if it is already set).

@pun-ky
Copy link
Author

pun-ky commented Aug 5, 2021

I will consider both options. Thanks @bjhargrave, @kriegfrj :)

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

No branches or pull requests

4 participants