refind: make the build reproducible #318499
Conversation
Avoid leaking build timestamps into the output.
raboof
added
6.topic: reproducible builds
backport release-24.05
|
LGTM Current Master PR 318499 Result of |
I haven't really checked, but since the MR isn't merged yet that commit is still at risk of being garbage-collected, so this is safer (as per #317314) |
|
Successfully created backport PR for |
| @@ -34,6 +34,9 @@ stdenv.mkDerivation rec { | |||
| patches = [ | |||
| # Removes hardcoded toolchain for aarch64, allowing successful aarch64 builds. | |||
| ./0001-toolchain.patch | |||
| # Avoid leaking the build timestamp | |||
| # https://sourceforge.net/p/refind/code/merge-requests/53/ | |||
| ./reproducible.patch | |||
There was a problem hiding this comment.
| ./reproducible.patch | |
| ./0002-preserve-dates.patch |
There was a problem hiding this comment.
Ah, sorry, I had merged already but am happy to follow up.
Is the number prefix really useful? The ordering is already specified by the list of patches, having the ordering in the filenames as well just seems to make things messy when patches are added or removed later. On the other hand, indeed it seems somewhat common: about 500 of the 3700 patches in nixpkgs seem to be numbered that way.
There was a problem hiding this comment.
I think you just need to use git format-patch IIRC.
There was a problem hiding this comment.
I mean, I can, but I'm not sure I understand the benefit.
(indeed preserve-dates might have been a better name though)
There was a problem hiding this comment.
Yeah I don't really understand the benefits either.
There was a problem hiding this comment.
Right, so it makes it easier to work with git am and rebase all patches in one go. Good to know - though indeed probably not universally necessary.
There was a problem hiding this comment.
We should prefer git format-patch patches all the time. Not for the filename, but for the additional metadata in its header, and tracking authorship information.
There was a problem hiding this comment.
Good idea. Could y'all add those recommendations to https://github.com/NixOS/nixpkgs/tree/master/pkgs#patches ?
There was a problem hiding this comment.
We should prefer
git format-patchpatches all the time. Not for the filename, but for the additional metadata in its header, and tracking authorship information.
I wouldn't want to write that as a hard requirement. It doesn't work when changes are done on top of a downloaded tarball and we already have most of the metadata like who wrote it, when it was written and there should be a short comment above the patch why we need it. We then would also track frequently changing information like git version.
Also format-patch is meant for sending patches via mail.
There was a problem hiding this comment.
My main concern with the numbering and the name is that patches are meant to be applied in sequence. (Sometimes patches can be applied unordered, but sometimes not.)
(Except maybe in Pijul, but I digress.)
Yes, lists in Nix are positional, but since sometimes the patch can be useful outside Nixpkgs - by a question of principle: we pick patches from other distros, and they can pick patches from us - it is a good idea to put an order on the patches.
P.S.: git-format-patch can be used outside git? Sometimes the patch comes from other VCSes...
As requested in NixOS#318499 (comment)
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Description of changes
Avoid leaking build timestamps into the output.
/cc @AndersonTorres @samueldr @chewblacka
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.