Skip to content

make cpio reproducible - #199

Merged
mobileoverlord merged 1 commit into
mainfrom
beni/dev
Aug 16, 2026
Merged

make cpio reproducible#199
mobileoverlord merged 1 commit into
mainfrom
beni/dev

Conversation

@bsandu-peridio

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to make initramfs cpio outputs reproducible by normalizing filesystem mtimes in the staged initramfs tree before the archive is created.

Changes:

  • Add a build-script step that normalizes mtimes under $INITRAMFS_WORK to SOURCE_DATE_EPOCH (defaulting to 0).
  • Add unit tests that assert the mtime-normalization step is emitted and ordered correctly relative to release-file injection and cpio creation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/commands/initramfs/image.rs

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/commands/initramfs/image.rs:129

  • You pin LC_ALL=C for the cpio entry sort to avoid locale-driven archive drift, but the initramfs build ID is also derived from a locale-sensitive sort of RPM NEVRAs earlier in this generated script (rpm -qa ... | sort). If the SDK/container ever gains additional locales or forwards LC_* from the host, the same package set could produce a different INITRAMFS_BUILD_ID. Consider pinning that earlier sort to LC_ALL=C as well.
    # Reproducibility notes for the pipeline below:
    #   * `LC_ALL=C sort` — entry order is archive order (and, with
    #     --renumber-inodes, decides the inode numbers), so collation must not
    #     drift. Today's SDK ships only the C/POSIX locales, which makes this a
    #     no-op, but it stops the archive from changing if the image ever gains
    #     real locales or the CLI starts forwarding the host's LC_* vars.

Signed-off-by: Beniamin Sandu <b.sandu@peridio.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/commands/initramfs/image.rs:126

  • xargs passes arbitrary filenames from the staged tree into touch. If any path begins with -, touch can interpret it as an option. Add -- after the touch options so all subsequent arguments are treated as paths.
        | xargs -0r touch -h -d "@${{SOURCE_DATE_EPOCH:-0}}"

@mobileoverlord

Copy link
Copy Markdown
Contributor

One note on SOURCE_DATE_EPOCH, non-blocking.

The normalization pass reads ${SOURCE_DATE_EPOCH:-0}, but nothing ever sets it on this path — SdkContainer doesn't forward it into the container, and the only place it gets exported is ext/image.rs:986, inside that script's own body. So the touch is effectively hardcoded to epoch 0.

That also means the source_date_epoch config key is silently ignored here. It's wired into ext image only (ext/image.rs:572, config.source_date_epoch.unwrap_or(0)) — so a user who sets it gets it honored for extension images and quietly dropped for the initramfs. Same pre-existing gap on the rootfs side (rootfs/image.rs:182 passes -T "${SOURCE_DATE_EPOCH:-0}" to mkfs.erofs), so this PR is consistent with what's already there and I wouldn't hold it up for this.

Worth a follow-up to plumb config.source_date_epoch into the rootfs/initramfs scripts the same way, so the three image types agree. The :-0 fallback is the right default to keep either way.

Unrelated FYI while in here: the pass zeroes /usr/lib/clock-epoch, which is systemd's runtime floor for the system clock on RTC-less boards. meta-avocado's systemd still compiles in -Dtime-epoch=<build ts> (systemd_258.1.bb:209), so the floor holds and there's nothing to do — just don't turn set-time-epoch off later and expect it to survive.

@mobileoverlord mobileoverlord left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving to land the LC_ALL=C pins, which are the load-bearing part — rootfs/install.rs (collation decides which kernel binary Image points at) and sdk/install.rs (preset first-match-wins decides which units end up enabled) were real latent bugs, not cosmetics.

Two follow-ups tracked separately rather than held against this PR:

  • #203 purges the ~14MB of dnf/rpm state that cp -a carries into the staged tree. The cpio is not byte-reproducible without it — the rpmdb stamps INSTALLTIME/INSTALLTID per package and var/cache/dnf holds generated repodata, so mtime normalization alone doesn't get there. That PR needs to rebase so its purge sits above the touch pass here.
  • SOURCE_DATE_EPOCH wiring, per my comment above.

Nit for whenever you're next in this file, not worth a round trip: the case block emits all four branches regardless of the initramfs_filesystem argument, so the format loops in test_cpio_entry_order_is_locale_independent, test_all_cpio_formats_get_normalized_tree and test_gzip_omits_timestamp assert on text that's present for every input. They read as per-format coverage but aren't — one call would do.

mobileoverlord added a commit that referenced this pull request Aug 14, 2026
The purge missed var/log. rootfs/install.rs omits $DNF_SDK_HOST_OPTS,
which is what redirects logdir/cachedir/persistdir at the SDK prefix --
so the same omission that puts var/cache/dnf and var/lib/dnf in the
sysroot puts dnf.log, dnf.rpm.log and hawkey.log in var/log, every line
wall-clock stamped. Both images now remove them, and both tests assert
all six paths; deleting the new `rm -f` fails them.

Dropped the `!contains("$ROOTFS_SYSROOT/var/lib/rpm")` guard from both
tests. That string has no occurrences to begin with, so the assertion
could not fail, and the build-ID query it claimed to protect is spelled
`--root "$ROOTFS_SYSROOT"` with `--dbpath`. A test that cannot fail is
worse than no test: it reads as coverage.

Scoped the reproducibility claim to what it earns. Removing this state
is necessary but not sufficient -- `cpio --reproducible` is
--device-independent on GNU cpio 2.15 and does not normalize mtime, and
the removal itself restamps the directories it empties. That half is
#199's. The comments say so rather than implying the image lands
reproducible here.

Adds the CHANGELOG entry, including the one consumer this can surprise:
anything running `rpm -qa` against a built image rather than the sysroot.
@mobileoverlord
mobileoverlord merged commit 77b27f0 into main Aug 16, 2026
10 of 11 checks passed
@mobileoverlord
mobileoverlord deleted the beni/dev branch August 16, 2026 00:29
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.

3 participants