-
Notifications
You must be signed in to change notification settings - Fork 393
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
WICKET-7090: add outputTimestamp property to enable reproducible builds #758
Conversation
In addition to reprocible builds, this should also result in timestamps being set on files in jars.
Ok, unfortunately, this does not seem to give the desired effect. The files in the jars still don't have timestamps. |
@@ -24,7 +24,7 @@ | |||
<relativePath>../pom.xml</relativePath> | |||
</parent> | |||
<artifactId>wicket-auth-roles</artifactId> | |||
<packaging>bundle</packaging> | |||
<packaging>jar</packaging> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this break the OSGi support ?!
CC @mattrpav
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know, it shouldn't. A jar is an OSGi bundle, when it's manifest contains OSGi related information, but it's still a jar. The documentation for the maven-bundle-plugin even describes this setup as a valid solution: https://felix.apache.org/documentation/subprojects/apache-felix-maven-bundle-plugin-bnd.html#_adding_osgi_metadata_to_existing_projects_without_changing_the_packaging_type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, type should be ‘bundle’ to activate the bundle plugin when generating the jar.
ActiveMQ had changes to support reproducible builds and maintained the bundle plugin.
I recommend we should know exactly what is the miss or change needed to support reproducible builds, and not just change the project type to jar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattrpav the bundle plugin is activated by binding one of its goals to the maven execution phase (in this case manifest
in process-classes
). This will generate the MANIFEST.MF, which will then be used by the maven-jar-plugin. I can see the correct MANIFEST.MF in the produced jar.
The problem with the maven-bundle-plugin is that it generates jars without timestamps on its entries. This is valid (and considered reproducible), but very inconvenient. There is no way to change this when using packaging bundle
. I think the produced jars are fine. They do contain the correct MANIFEST.MF. However, we do not use OSGi, so I have no way of testing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@papegaaij the bundle plugin can add any field to the MANIFEST.MF. What field are you looking for in the MANIFEST.MF?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattrpav This change is to get correct timestamps on files (also called entries) in the jar files. The maven-bundle-plugin does not set these timestamps at all, causing them to revert to the jar-epoch (02-01-1980). The maven-jar-plugin honors project.build.outputTimestamp
. If it also gives us reproducible builds, that's a bonus but not the reason for this change.
We do however not want to break OSGi support. However, none of the Wicket core developers uses OSGi, so this is very hard for us to test. With this change, the produced jars should still be valid OSGi bundles, but we have no good way to verify this. It be really great of you could check this.
As far as I know OSGi, the only thing that makes a normal jar an OSGi bundle is a set of entries in the MANIFEST.MF. These all seem unaffected by this change (I compared before and after), so I think the change is correct. But maybe you can point to something else that must be in place for an OSGi bundle?
In addition: one of the commits uses |
…plugin for packaging
Doh, TKH is the prefix we use for Topicus KeyHub :) I've fixed the commit. |
@@ -129,6 +129,8 @@ | |||
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | |||
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding> | |||
|
|||
<project.build.outputTimestamp>2024-01-01T00:00:00Z</project.build.outputTimestamp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a static value seems difficult to manage. I suggest looking at using the git commit timestamp.
Benefits:
- Automatically changes
- Changes with every commit
- Ensures reproducible builds get the same timestamp when rebuilt from any commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maven-release-plugin updates this field during release .....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the maven-release-plugin should update this field automatically during a release. We do need to verify this at the next release though.
@martin-g are you ok with merging this change? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Hopefully the change will be tested by an OSGi user during the vote for 10.0.0!
If there is a regression we can work on a fix in 10.0.x!
@martin-g @papegaaij I'll test the v10 release in OSGi and report any issues. Thanks. |
Thank you @mattrpav |
In addition to reprocible builds, this should also result in timestamps being set on files in jars.