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

Fix #233 reproducible builds #234

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

agentgt
Copy link

@agentgt agentgt commented May 28, 2023

This just makes the maven artifact compare work. To really test you will need to try https://github.com/jvm-repo-rebuild/reproducible-central with different machines and docker images (yes all of these can make a difference).

I also recommend if you use any plugins to require -Duser.timezone=UTC at release time.

You can check out JStachio root pom in the enforcers and profiles to see how I did it as this PR does not have all the safety guards in.

@@ -40,7 +40,8 @@
<maven.compiler.target>1.8</maven.compiler.target>
<junit.jupiter.version>5.9.0</junit.jupiter.version>
<jacoco.version>0.8.5</jacoco.version>
<argLine>-Dfile.encoding=UTF-8</argLine>
<argLine>-Dfile.encoding=UTF-8</argLine>
<project.build.outputTimestamp>2023-01-01T00:00:00Z</project.build.outputTimestamp>
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's best to use the commit timestamp here IMO.

Copy link
Author

Choose a reason for hiding this comment

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

What is suppose to happen when you release that property is updated.

I don’t use the release plugin but it does that.

Thus I did not put a meaningful timestamp.

In fact I would do the opposite of what you think and have a maven enforcer plugin validate that it is not the hardcoded default on release.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally, every build should be reproducible, not just releases.

Copy link
Owner

Choose a reason for hiding this comment

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

https://maven.apache.org/guides/mini/guide-reproducible-builds.html

instead of explicitely writing a timestamp in their pom.xml, some people tend to prefer using last Git commit timestamp, like ${git.commit.time} from git-commit-id-maven-plugin.

Sounds pretty good, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, we can also take a look at our approach at work, it already does something similar IIRC.

Copy link
Author

@agentgt agentgt May 29, 2023

Choose a reason for hiding this comment

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

The problem with git.commit.time is that you are relying on that tool to make the build reproducible.

That is some one should be able to take a zip or tgz of your source and build w/o git.

For example in my company we used Hg but now switched over to git. So now if we checkout a specific version from before git we would not be able to reproduce if we followed git.commit.time (unless of course you actually change the pom but I'm assuming that isn't the case).

Honestly the timestamps are not important so long as they stay the same. The only reason to have them change for release is mainly generated doc and possibly the zip aka jar time stamps.

Thus I use a some static default of the last release build timestamp for my snapshots and then change it for release (well I don't change the pom but use a properties file and a custom script to change the pom on release but I assume you do something different or use the maven versions plugin or release plugin).

That being said I am hypocrite in that I think I put in the git hash in the MANIFEST.MF.

@@ -15,6 +15,7 @@
<maven.deploy.skip>true</maven.deploy.skip>
<skipNexusStagingDeployMojo>true</skipNexusStagingDeployMojo>
<jacoco.version>0.8.5</jacoco.version>
<project.build.outputTimestamp>2023-01-01T00:00:00Z</project.build.outputTimestamp>
Copy link
Owner

Choose a reason for hiding this comment

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

Everything in test are internal integration test projects that are never released (<maven.deploy.skip>true</maven.deploy.skip>).

They do not need these changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should make everything reproducible if we now spend some time anyway.

Copy link
Owner

Choose a reason for hiding this comment

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

Seems like a lot of boilerplate for no real world impact.

Copy link
Author

Choose a reason for hiding this comment

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

The timestamp doesn’t need to change on every build. Only for release.

It doesn’t even have to be the commit time to be reproducible.

It would be ridiculous IMO to change the parent pom on every commit.

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

3 participants