-
Notifications
You must be signed in to change notification settings - Fork 2.9k
First steps towards build reproducibility #48869
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
base: main
Are you sure you want to change the base?
Conversation
Better reviewed commit per commit as they are semantic and isolated. |
This comment has been minimized.
This comment has been minimized.
🎊 PR Preview 8fab080 has been successfully built and deployed to https://quarkus-pr-main-48869-preview.surge.sh/version/main/guides/
|
This comment has been minimized.
This comment has been minimized.
This improves the reproducibility of the zip files.
This will affect the Platform generator and make sure the JSON files of the Platform are generated reproducibly.
Status for workflow
|
* <p> | ||
* The approach is similar to what is done by the maven-jar-plugin with `project.build.outputTimestamp`. | ||
*/ | ||
Optional<Instant> 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.
What happens if the user does not set the value? Is project.build.outputTimestamp
propagated? I wonder if the JavaDoc should contain this kind of info.
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.
Ah, yes, I can add a comment. You actually don't need to set it in the Maven case, I propagate project.build.outputTimestamp
.
As for Gradle, I'll see when I get a Maven build fully reproducible.
Status for workflow
|
final Iterator<Path> i = files.iterator(); | ||
final Iterator<Path> i = files | ||
// we sort the elements to be deterministic, we compare the toString() to make sure the order is the same on Linux and Windows | ||
.sorted((p1, p2) -> p1.toString().compareTo(p2.toString())) |
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 appear to be generally necessary or only in specific cases?
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.
So it's probably only necessary in specific cases... but figuring out which cases is hard and when you had new use cases, people will have to think about it (and will forget).
I think in general bringing a more deterministic behavior is nice.
As for the specific comparator, it's the only way to get the same ordering on Linux and Windows.
Now, maybe we shouldn't merge this PR for now and wait for the whole thing and then try to evaluate the cost of it?
* <p> | ||
* The approach is similar to what is done by the maven-jar-plugin with `project.build.outputTimestamp`. | ||
*/ | ||
Optional<Instant> 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.
Is this similar to what I suggested before in this enhancement request (#41070)? quarkus.build.timestamp
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.
If we decide to go this way, it could be used to do what you were asking for.
For now, the main reason for it is to make sure the Quarkus jars are generated reproducibly.
About this work:
There's a companion PR here: quarkusio/quarkus-platform-bom-generator#375
Our builds is a lot better after these PRs: I was able to get a reproducible build for most of our own artifacts except for
quarkus-cli-runner
which is a Quarkus app and not yet reproducible and all the modules after it (mostly the ITs, which are also Quarkus apps).Note that this is just a start, there are a lot of tricky issues to solve to get the build of a Quarkus app reproducible. I'll continue to iterate on this task, I already started extracting and rebasing some of @ppalaga 's old PRs - they definitely go in the right direction.