bugfix: Fix data.cpio reproductibility issue introduced by #1961 #1973
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR reverts the regression introduced in #1961 to fix the reproducibility of data.cpio and streamlines the initrd packaging process. Key changes include:
- Reverting the unconditional build of board.cpio to the pre-#1961 logic.
- Refactoring the staging and creation of data.cpio with explicit per-entry copy rules.
- Updating the organization of targets for heads.cpio, modules.cpio, tools.cpio, and adding a GPG key injection helper.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a reproducibility issue for data.cpio by reverting the board.cpio logic to its previous behavior and cleaning up the data file staging process. Key changes include removing legacy data installation macros, introducing a new stage_data_file macro for better data file handling, and consolidating redundant targets and comments.
|
|
||
| # Only build data.cpio if there are data files | ||
| ifneq ($(strip $(data_files)),) | ||
| # --- DATA.CPIO STAGING AND CREATION --- |
There was a problem hiding this comment.
[nitpick] Consider adding a brief comment explaining the rationale behind switching from the old initrd_data_add macro to the new stage_data_file macro to aid future maintainers in understanding the changes.
Copilot uses AI. Check for mistakes.
|
Let me know if there is anything I can do to help with this. Happy to dig up my hashes.txt file if it will help in any way. My data.cpio was provided here in case you missed it: #1966 (comment) |
Yes. Locally rebuild any board rom from this PR and come to a different hash of data.cpio again. They should be the same, meaning data.cpi hash is the same.
I never was able to reproduce your hash :/ Tldr: please rebuild any board roms final hash and report rhy are the same @thickfont. That would help. And if different, detail your environment, not sure what would induce docker image rom built differences. |
I took the time to diffoscope your data.cpio with the one I consistently create the same data.cpio with same hash.
TLDR: both data.cpio were wrong (including empty files that aren't empty). |
|
With e746aab which successfully built at https://app.circleci.com/pipelines/github/tlaurion/heads/3339/workflows/8305914f-458a-4e97-bcb4-f17cf3669bae) Each build output results in "INSTALL-DATA" traces which picks each and single one of the files and put them in initrd/data dir prior of creating data.cpio with them in a reproducible way: While diffoscope now only shows one dicrepency between @thickfont 's provided data.cpio and mine: and its in regard of their 0 byte file: |
tlaurion
force-pushed
the
fix_data_cpio_repro
branch
from
e746aab to
d8749b9
Compare
…r cpio (make sure initrd data dir fresh + wiped after use) + document cpio properly under Makefile - move inject_gpg up (qemu workaround since we have no SPI internal flashing support), move all dev related helpers down the file. - revert old behavior for board.cpio construction; only construct if boards/board/initrd exists (currently Librem 11, Librem mini, and Librem mini 2) Signed-off-by: Thierry Laurion <insurgo@riseup.net>
tlaurion
force-pushed
the
fix_data_cpio_repro
branch
from
d8749b9 to
8417d4b
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a data.cpio reproducibility issue introduced by #1961 by reverting the unconditional board.cpio build and refactoring the data.cpio construction logic.
- Removed the old initrd_data_add macro and related calls.
- Introduced a new macro ("stage_data_file") to individually stage data files for reproducible builds.
- Consolidated and restructured cpio packaging rules including modules.cpio, tools.cpio, heads.cpio, and added an inject_gpg target.
Comment on lines
+772
to
+773
| $(if $(shell [ -d "$(src)" ] && echo yes),\ | ||
| $(foreach f,$(shell find $(src) -type f),\ |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Awesome yes.
Is the expected value. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #1969