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

add OSGi metadata and reproducible build #428

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

chrisrueger
Copy link

@chrisrueger chrisrueger commented Dec 11, 2023

This PR generates OSGi metadata during the build the following META-INF/MANIFEST.MF during a gradle build and should address #362 . In other words it creates a proper OSGi bundle.
It uses the bndtools Gradle Plugin (see here and here)

An additional side effect of this PR is that the build is made reproducible.

Why?

Simplifies adoption for people using an OSGi environment.

image
Manifest-Version: 1.0
Bnd-LastModified: 1702332812312
Bundle-Description: An artifact of well-specified annotations to power
  static analysis checks and JVM language interop.
Bundle-DocURL: https://jspecify.dev/docs/start-here
Bundle-ManifestVersion: 2
Bundle-Name: jspecify
Bundle-SymbolicName: org.jspecify.jspecify
Bundle-Vendor: jspecify.dev
Bundle-Version: 0.0.0.SNAPSHOT
Created-By: 17.0.9 (Eclipse Adoptium)
Export-Package: org.jspecify.annotations;version="0.0.0.SNAPSHOT"
Import-Package: java.lang,java.lang.annotation
Multi-Release: true
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
Tool: Bnd-7.0.0.202310060912

More Detailed View using bndtools Jar-Viewer:

[MANIFEST]

Bnd-LastModified                        1702332812312
Bundle-Description                      An artifact of well-specified annotations to power static analysis checks and JVM language interop.
Bundle-DocURL                           https://jspecify.dev/docs/start-here
Bundle-ManifestVersion                  2
Bundle-Name                             jspecify
Bundle-SymbolicName                     org.jspecify.jspecify
Bundle-Vendor                           jspecify.dev
Bundle-Version                          0.0.0.SNAPSHOT
Created-By                              17.0.9 (Eclipse Adoptium)
Export-Package                          org.jspecify.annotations;version="0.0.0.SNAPSHOT"
Import-Package                          java.lang
                                        java.lang.annotation
Manifest-Version                        1.0
Multi-Release                           true
Require-Capability                      osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
Tool                                    Bnd-7.0.0.202310060912


[IMPEXP]

Import-Package
  java.lang                              
  java.lang.annotation                   

Export-Package
  org.jspecify.annotations               {version=0.0.0.SNAPSHOT}

[CAPABILITIES]

Require-Capability
  osgi.ee                                {filter:=(&(osgi.ee=JavaSE)(version=1.8))}

[COMPONENTS]



[METATYPE]



[API USES]


[USES]

org.jspecify.annotations                


[USEDBY]



[LIST]

META-INF
  MANIFEST.MF
META-INF/versions
META-INF/versions <no contents>
META-INF/versions/9
  module-info.class
META-INF/versions/9/OSGI-INF
  MANIFEST.MF
org
org <no contents>
org/jspecify
org/jspecify <no contents>
org/jspecify/annotations
  NonNull.class
  NullMarked.class
  NullUnmarked.class
  Nullable.class

The bundle-Version is automatically taken from the version inside gradle.properties.

Copy link

google-cla bot commented Dec 11, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@chrisrueger chrisrueger marked this pull request as ready for review December 11, 2023 22:07
@chrisrueger chrisrueger marked this pull request as draft December 11, 2023 22:08
This generates the following META-INF/MANIFEST.MF during a gradle build

Manifest-Version: 1.0
Bnd-LastModified: 1702331529321
Bundle-Description: An artifact of well-specified annotations to power
  static analysis checks and JVM language interop.
Bundle-DocURL: https://jspecify.dev/docs/start-here
Bundle-ManifestVersion: 2
Bundle-Name: org.jspecify
Bundle-SymbolicName: jspecify
Bundle-Vendor: jspecify.dev
Bundle-Version: 0.0.0.SNAPSHOT
Created-By: 17.0.9 (Eclipse Adoptium)
Export-Package: org.jspecify.annotations;version="0.0.0.SNAPSHOT"
Import-Package: java.lang,java.lang.annotation
Multi-Release: true
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
Tool: Bnd-7.0.0.202310060912

specify Bundle-SymbolicName

to org.jspecify.jspecify
@chrisrueger chrisrueger marked this pull request as ready for review December 11, 2023 22:17
@chrisrueger
Copy link
Author

Not entirely sure about Bundle-SymbolicName

Do we want:
a) Bundle-SymbolicName: org.jspecify.jspecify
or
b) Bundle-SymbolicName: org.jspecify.annotations

a) is currently implemented. It is based on the maven artifact GAV (org.jspecify:jspecify:jar:0.3.0 e.g. see here

@ben-manes
Copy link

As a ignorant developer supporting a library with OSGi users, I found adding a unit test helpful comfort, wdyt?
(example using pax-exam)

This project might want reproducible jars? I don't believe bnd does that by default (my build snippet if helpful).

@chrisrueger chrisrueger marked this pull request as draft December 12, 2023 19:32
@agentgt
Copy link

agentgt commented Dec 12, 2023

As a ignorant developer supporting a library with OSGi users, I found adding a unit test helpful comfort, wdyt?
(example using pax-exam)

I'm going to steal that 😄 for my own project.

Also I think making this project reproducible is more important than OSGi so hopefully that will be the case. I didn't see it in the list: https://github.com/jvm-repo-rebuild/reproducible-central#readme

@ben-manes do you know if it is currently reproducible (jspecify jars)?

@chrisrueger chrisrueger marked this pull request as ready for review December 12, 2023 20:48
@chrisrueger
Copy link
Author

chrisrueger commented Dec 12, 2023

@ben-manes thanks a lot for the pointers. I tried adding your suggestions about reproducible builds - although I must admit I have no experience with it, so could you double check it?

Regarding unit test: generally good idea. What would you test for here?

Last point:

As a ignorant developer

Sorry, german here, don't get it ;) What's is with this "ignorant"?

@ben-manes
Copy link

ben-manes commented Dec 12, 2023

do you know if it is currently reproducible (jspecify jars)?

Prior to @chrisrueger's change, those jar look non-reproducible because it included the timestamps (those gradle flags were not set).

$ unzip -l jspecify-0.3.0.jar
Archive:  jspecify-0.3.0.jar
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  12-13-2022 12:34   META-INF/
       46  12-13-2022 12:34   META-INF/MANIFEST.MF
       ...

When set it would look more like,

unzip -l caffeine-3.1.8.jar
Archive:  caffeine-3.1.8.jar
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  02-01-1980 00:00   META-INF/
      792  02-01-1980 00:00   META-INF/MANIFEST.MF
    11358  02-01-1980 00:00   META-INF/LICENSE
    ...

What would you test for here?

I suppose maybe a reflection query if the annotation is found on a dummy test class? I believe since it runs in a custom classloader the Caffeine test is just ensuring that its library can be loaded by the runtime.

What's is with this "ignorant"?

haha, I just meant that I have not actually ever used OSGi and only know of it as some magic flags that I set to satisfy other developers. I don't trust myself to not break them by an innocent refactor, so I can stay naive but more assured.

@ben-manes
Copy link

could you double check it?

lgtm (but just an observer)

Inspection
Downloads gh repo clone jspecify/jspecify
fatal: destination path 'jspecify' already exists and is not an empty directory.
failed to run git: exit status 128Downloads cd jspecifyjspecify git:(add-osgi-metadata) gh pr checkout 428
From https://github.com/jspecify/jspecify
 * branch            refs/pull/428/head -> FETCH_HEAD
Already up to date.jspecify git:(add-osgi-metadata) ./gradlew -q jarjspecify git:(add-osgi-metadata) cd build/libslibs git:(add-osgi-metadata) unzip -l jspecify-0.0.0-SNAPSHOT.jar
Archive:  jspecify-0.0.0-SNAPSHOT.jar
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  02-01-1980 00:00   META-INF/
      626  02-01-1980 00:00   META-INF/MANIFEST.MF
        0  02-01-1980 00:00   META-INF/versions/
        0  02-01-1980 00:00   META-INF/versions/9/
        0  02-01-1980 00:00   META-INF/versions/9/OSGI-INF/
      145  02-01-1980 00:00   META-INF/versions/9/OSGI-INF/MANIFEST.MF
      187  02-01-1980 00:00   META-INF/versions/9/module-info.class
        0  02-01-1980 00:00   org/
        0  02-01-1980 00:00   org/jspecify/
        0  02-01-1980 00:00   org/jspecify/annotations/
      434  02-01-1980 00:00   org/jspecify/annotations/NonNull.class
      498  02-01-1980 00:00   org/jspecify/annotations/NullMarked.class
      488  02-01-1980 00:00   org/jspecify/annotations/NullUnmarked.class
      436  02-01-1980 00:00   org/jspecify/annotations/Nullable.class
---------                     -------
     2814                     14 fileslibs git:(add-osgi-metadata) unzip jspecify-0.0.0-SNAPSHOT.jar
Archive:  jspecify-0.0.0-SNAPSHOT.jar
   creating: META-INF/
  inflating: META-INF/MANIFEST.MF
   creating: META-INF/versions/
   creating: META-INF/versions/9/
   creating: META-INF/versions/9/OSGI-INF/
  inflating: META-INF/versions/9/OSGI-INF/MANIFEST.MF
  inflating: META-INF/versions/9/module-info.class
   creating: org/
   creating: org/jspecify/
   creating: org/jspecify/annotations/
  inflating: org/jspecify/annotations/NonNull.class
  inflating: org/jspecify/annotations/NullMarked.class
  inflating: org/jspecify/annotations/NullUnmarked.class
  inflating: org/jspecify/annotations/Nullable.classlibs git:(add-osgi-metadata) cat META-INF/MANIFEST.MF
Manifest-Version: 1.0
Bundle-Description: An artifact of well-specified annotations to power
  static analysis checks and JVM language interop.
Bundle-DocURL: https://jspecify.dev/docs/start-here
Bundle-License: https://www.apache.org/licenses/LICENSE-2.0
Bundle-ManifestVersion: 2
Bundle-Name: jspecify
Bundle-SymbolicName: org.jspecify.jspecify
Bundle-Vendor: jspecify.dev
Bundle-Version: 0.0.0.SNAPSHOT
Export-Package: org.jspecify.annotations;version="0.0.0.SNAPSHOT"
Import-Package: java.lang,java.lang.annotation
Multi-Release: true
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"

@chrisrueger
Copy link
Author

lgtm (but just an observer)

Thanks a lot.

@agentgt
Copy link

agentgt commented Dec 13, 2023

@ben-manes Sorry I didn't mean for you to go actually check 😄 just if you knew offhand. Thanks for checking though!

The reason is because it is missing from : https://github.com/jvm-repo-rebuild/reproducible-central#readme

I will see if I can add it to the list today. I think you can add an artifact without being a project owner.

I will say checking the timestamp in the zip contents sometimes isn't enough because some Maven plugins have (had) bugs like Moditect that accidentally put something like timestamp somewhere in the zip that for some reason isn't visible in the zip manifest (e.g. checksum changes on each build). However this project uses Gradle (and I guess Blaze internally) so probably not a problem.

@chrisrueger chrisrueger changed the title add OSGi metadata add OSGi metadata and reproducible build Dec 29, 2023
bnd.bnd Outdated
@@ -0,0 +1,10 @@
Bundle-SymbolicName: org.jspecify.jspecify
Bundle-Name: jspecify
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about to make it more human readable by JSpecify annotations
Also pom.xml 0.3.0 states: <name>JSpecify annotations</name>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. Added your suggestion @bjmi

@chrisrueger
Copy link
Author

Regarding failing JDK 11 build

I forgot that bnd 7.0.0 requires JDK 17 as a minimum.
I'll later decrease bnd to 6.4.0 which should also work and requires JDK 8 asa minimum.

@chrisrueger
Copy link
Author

I'll later decrease bnd to 6.4.0 which should also work and requires JDK 8 asa minimum.

@cpovirk could you trigger a rebuild to check if JDK11 build now runs?

@cpovirk
Copy link
Collaborator

cpovirk commented May 24, 2024

Done. Looks like it's unhappy with the module-info under the multi-release jar root. If that's a significant obstacle, feel free to move it back to the main root (#484).

dilemma: unfortunatelly bnd in 6.4.0 does not support multi-release jars. It starts being support in 7.0.0 but this requires min.JDK 17. 
So the only way is to suppress the warning via -fixupmessage (see bndtools/bnd#3514 and https://bnd.bndtools.org/instructions/fixupmessages.html)
@chrisrueger
Copy link
Author

Done. Looks like it's unhappy with the module-info under the multi-release jar root. If that's a significant obstacle, feel free to move it back to the main root (#484).

Thanks. I have applied a fix.
it is kind of a dilemma: unfortunatelly bnd in 6.4.0 does not support multi-release jars. It starts being support in bnd 7.0.0 but this requires min. JDK 17.
So the only way is to suppress the warning via -fixupmessage (see bndtools/bnd#3514 and https://bnd.bndtools.org/instructions/fixupmessages.html)

I try to investigate if there are other options like doing different things depending on JDK version.

@ben-manes
Copy link

You can run gradle on 17 independently from the execution tasks. In the latest version, you can add the gradle-daemon-jvm.properties tool version and use bnd, but build on a lower jdk using the java.toolchain.languageVersion. Does that help?

@chrisrueger
Copy link
Author

You can run gradle on 17 independently from the execution tasks. In the latest version, you can add the gradle-daemon-jvm.properties tool version and use bnd, but build on a lower jdk using the java.toolchain.languageVersion. Does that help?

Thanks. Not sure.

My first thought was to try something like this: https://discuss.gradle.org/t/conditional-dependency-based-on-toolchains-version/46369

if (JavaVersion.current().isJava17Compatible()) {
    id "biz.aQute.bnd.builder" version "7.0.0"
  }
else {
    id "biz.aQute.bnd.builder" version "6.4.0"
  }

Is this what you mean?

@ben-manes
Copy link

ben-manes commented May 24, 2024

Nope. Bnd only needs to run in gradle so as long as the daemon is 17+ then everything else will work. You can now force that with a daemon toolchain.

The Java version for tasks like compile, test, exec are from the language toolchain. Instead of inferring it by the daemon jvm, it should be set explicitly. An external env variable can be used, which then lets you cross build. The jdk release flag is also useful as it will ensure that the output matches a desired version as well.

Caffeine’s build uses this, but it’s a few parts to set up.

@cpovirk
Copy link
Collaborator

cpovirk commented May 24, 2024

(I'd be happy for someone to make that happen, especially after the fun in #517. I don't expect to get to it myself on any particular timeline, unfortunately.)

@chrisrueger
Copy link
Author

Thanks for the pointers @ben-manes
I reverted my changes back to bnd 7.0.0 because this would be the right thing once JDK 17 is used to support Multi-Release jars too.

In the latest version, you can add the gradle-daemon-jvm.properties tool version and use bnd, but build on a lower jdk using the java.toolchain.languageVersion.

It seems that it is currently in (unreleased) Gradle 8.8.-rc-1 and I first need to get familiar with the whole toolchain stuff. No gradle expert.
So eventually somebody more familiar with the matter can take over or someday I'll find the time to wrap my head around it.

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

Successfully merging this pull request may close these issues.

None yet

5 participants