wire source_date_epoch into rootfs and initramfs image builds

[mobileoverlord]

Follow-up to my comment on #199. Independent of #203 — no overlapping hunks, either order merges clean.

The gap

rootfs/image.rs passes -T "${SOURCE_DATE_EPOCH:-0}" to mkfs.erofs, and #199's initramfs mtime pass reads the same variable. Nothing ever set it. The only export SOURCE_DATE_EPOCH in the tree is inside the extension image script's own body (ext/image.rs:986).

Net effect: a project that sets source_date_epoch: in its config gets a reproducibility stamp on its .raw extension images and a silently ignored key for every other image type.

The fix

inject_source_date_epoch() in container.rs, mirroring the inject_repo_tls_env() next to it, called from the three sites that build images:

• rootfs image — feeds mkfs.erofs -T
• initramfs image — inert on this base. main has no reader for the variable yet; the mtime-normalization pass that consumes it lands in make cpio reproducible #199. Set anyway so every image-building run carries the same env rather than gaining it per-consumer later
• runtime build — one insert, since that run executes the rootfs, initramfs and extension sections in the same container

No signature changes, no new RunConfig field. The scripts already read the variable; this just makes something set it.

Unset stays unset

Deliberately not defaulted to 0 in the injector. The build scripts carry their own :-0 fallback, and SOURCE_DATE_EPOCH is honored by plenty of tools that can run inside a post_install hook — gzip, tar, python bytecode compilation. Exporting it unconditionally would silently change what those produce for projects that never opted in. A project configuring nothing gets byte-identical behavior and the same container env as today.

Some(0) remains meaningful and distinct from unset, since 0 is a legitimate epoch to pin to. Covered by a test.

Extensions

Nothing to extend — ext image already threads config.source_date_epoch into its script and passes it to both mkfs.erofs -T and mksquashfs. This PR brings the other two image types up to that.

Test

Three tests on the injector (configured value, explicit Some(0), unset stays absent), plus one pinning the consumer side — mkfs.erofs must read $SOURCE_DATE_EPOCH. That pairing is the actual regression risk: injection and consumption have to agree on the variable name, and a mismatch in either half fails silently by falling back to 0, which is precisely the bug being fixed here.

Full suite green, clippy clean with -D warnings.

[Copilot]

Pull request overview

This PR aims to make the source_date_epoch config key take effect for non-extension image builds by injecting SOURCE_DATE_EPOCH into the container environment used by rootfs/initramfs/runtime build commands, aligning with the reproducibility expectations already present in build scripts.

Changes:

• Add inject_source_date_epoch() helper to conditionally export SOURCE_DATE_EPOCH into container env maps.
• Call the injector from runtime build, rootfs image, and initramfs image so configured epochs reach the build container.
• Add regression tests for the injector and a rootfs script test to pin the consumer-side variable name.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/utils/container.rs	Adds inject_source_date_epoch() plus unit tests and documentation for the new env injection helper.
src/commands/runtime/build.rs	Injects SOURCE_DATE_EPOCH into the runtime build container env.
src/commands/rootfs/image.rs	Injects SOURCE_DATE_EPOCH for rootfs builds and adds a test ensuring the script consumes it via mkfs.erofs -T.
src/commands/initramfs/image.rs	Injects SOURCE_DATE_EPOCH into initramfs build container env.

Suppressed comments (1)

src/utils/container.rs:3142

• This regression-test module comment also states that the initramfs scripts read ${SOURCE_DATE_EPOCH:-0}, but there is no SOURCE_DATE_EPOCH reference in the initramfs build script today. Adjust the wording so the test description matches the actual consumers.

    /// Regression: `source_date_epoch` was plumbed into extension images only.
    /// The rootfs and initramfs scripts read `${SOURCE_DATE_EPOCH:-0}` but
    /// nothing set the variable, so a project that configured it got a
    /// reproducibility stamp on its `.raw` extensions and a silently ignored
    /// key everywhere else.

---

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

[jetm]

The shipped behavior is right and I checked the plumbing rather than assuming it: the env reaches the container on both the local path (container.rs:1457, separate argv -e elements, so no escaping question) and the runs_on remote path (runs_on.rs:543, shell_escaped, same precedence), and no embedded script section writes or unsets SOURCE_DATE_EPOCH before the rootfs section, so nothing clobbers it. Both non-test callers of the rootfs and initramfs generators are covered, and the kernel/ext/sdk/provision paths generate no script that reads the var. cargo fmt, clippy -D warnings and cargo test --lib (1395 passed) are all green at 3cc5cfd.

Three things worth a round trip.

Nothing pins the wiring. src/commands/rootfs/image.rs:401. I deleted the injection call and all 1395 tests stayed green. Both halves are tested — the helper's three-way Option behavior, and the script's reproducibility flags — but their connection is not, so the exact regression this PR fixes could come back silently. For contrast I also mutated -U 00000000-… to -U 11111111-… and test_rootfs_image_reproducibility_flags_are_pinned failed correctly, so the new -U assertion is load-bearing; it is only the call site that is unguarded.

The comment at src/commands/runtime/build.rs:537 is wrong in a way that could bite later. It says the container "executes the rootfs, initramfs and extension image sections", but it only copies pre-built ext artifacts (build.rs:1334) — ext images come from a separate run that exports the epoch itself. That matters because ext/image.rs:891 has no :-0 fallback, so anyone who trusts the comment and removes the export there gets an unset var rather than a zero.

post_build still silently ignores the key. src/commands/runtime/build.rs:2880 runs in its own container with runtime_env_vars(), which carries only AVOCADO_RUNTIME. So the epoch reaches post_install hooks and not post_build ones — the same silent-ignore class this PR exists to close.

Nits:

• src/utils/container.rs:531 — three repeated call sites, where the sibling inject_repo_tls_env is invoked from inside container.rs at all four run paths so no command can forget it. Real tradeoff though: moving it widens the blast radius to every container run.
• src/utils/container.rs:528 — the doc argues the var is deliberately left unset when config is absent, but ext/image.rs:573 already exports unwrap_or(0) unconditionally. The key behaves two ways depending on image type.
• src/utils/container.rs:516 — the same rationale is restated in a 16-line doc, a 5-line test-mod doc and three call-site comments for a 5-line function, and one copy has already drifted (that is the build.rs:537 finding above).
• src/commands/rootfs/image.rs:530 — test passes the zero UUID as namespace_uuid; NAMESPACE_UUID is 6ba7b810-…, and the next test 20 lines down passes it correctly.
• No CHANGELOG.md entry for a user-visible fix, and ### Fixed already exists in Unreleased. 4 of the last 5 main commits touched it.

One gap I could not close: erofs-utils is not installed here, so I could not check whether mkfs.erofs -T <n> normalizes per-file mtimes or only the superblock build time. Not raising it as a finding for that reason, but it is the question that decides whether this fully lands reproducibility or only part of it.

[mobileoverlord]

All three findings and three of the four nits, in 1a294be.

The unpinned wiring — and a story worth telling. You were right that deleting the injection call left everything green. I added a guard for it, ran your mutation to confirm, and it still passed. The guard was a mod tests inside rootfs/image.rs asserting include_str!("image.rs").contains("inject_source_date_epoch(...)") — and the needle matched the assertion's own string literal. A test that could not fail, in the same review where we deleted two others for exactly that.

It lives in tests/source_date_epoch_wiring.rs now, where the needle cannot match itself, and I re-ran your deletion to confirm it fails:

thread 'rootfs_image_run_injects_source_date_epoch' panicked:
the rootfs image run must inject SOURCE_DATE_EPOCH into its container env

Both call sites are covered. The initramfs one matters more than it looks, since it is inert on this base — a deletion there is invisible by construction until #199 lands.

Flagging the ceiling honestly: it pins the call's spelling, not its effect. Testing the effect means lifting env-map construction out of the async run path in both commands. There is a ponytail: note in the file saying so and when to upgrade it.

post_build. Fixed — it takes the epoch and injects into its own env. Your framing is what makes this a finding rather than a nit: it is the same silent-ignore class the PR exists to close, one container over, and it is the container most likely to build something.

The drifted comment. Corrected. Your point about why it matters is the sharp end — ext/image.rs:891 has no :-0 fallback, so someone trusting that comment and removing the export there gets an unset var rather than a zero. The comment now says the ext artifacts are copied in pre-built.

Nits. Doc collapsed from 16 lines to 5, and the ext image unwrap_or(0) contradiction is now stated in that doc rather than left implicit between two of them — you were right that it had already drifted once, which is the argument against restating a rationale in four places. namespace_uuid fixed to NAMESPACE_UUID.

The one I did not take: moving inject_source_date_epoch inside container.rs alongside inject_repo_tls_env. You named the tradeoff yourself and I land on the other side of it for now — inject_repo_tls_env is unconditional infrastructure, while this key changes the behavior of gzip, tar and python bytecode inside user post_install hooks, so widening it to every container run is a bigger change than it looks. Three explicit call sites are auditable; the wiring test is what stops a fourth from being forgotten. Happy to move it if you still prefer that.

The erofs gap stays open. Nobody in this review has erofs-utils available, so whether mkfs.erofs -T normalizes per-file mtimes or only the superblock is still unanswered. It is the question that decides whether the rootfs half lands fully, and I would rather leave it stated than assume. Same gap noted on #203.

cargo fmt, clippy -D warnings, 1395 lib tests and the 2 new wiring tests green at 1a294be.

[jetm]

Re-reviewed the increment since 3cc5cfd, checking the subject's three claims rather than re-reading the change.

The corrected comments hold - I checked every factual claim in both rewrites and they all match the code. The other two are partial, and both inline findings are the same shape: the wiring landed on the path that was pointed at and not on its sibling.

The new guard is not the generated-text pattern the earlier PRs in this series shipped - it reads real source and goes red when the rootfs injection is deleted. Its gap is which files it scans.

Five advisory notes were withheld rather than appended here. One of them is worth a glance regardless: runtime/build.rs:2839 now carries a duplicated #[allow(clippy::too_many_arguments)]. CI will not flag it - I confirmed duplicated_attributes fires on free functions in this crate but not on impl methods, which is where this one sits.

[jetm]

The post_build claim covers only one of the two post_build hooks

"Rootfs, initramfs and post_build runs now all carry it" reads as unqualified, but there are two run_post_build implementations in this tree and only the runtime one was wired. The extension hook (src/commands/ext/build.rs:1473) still passes bare env_vars: self.runtime_env_vars() at :1520 with no injection.

The runtime side is genuinely wired end to end, to be clear - config.source_date_epoch reaches run_post_build (runtime/build.rs:743), is injected into that container's env map (:2890), and RunConfig.env_vars reaches the container as -e KEY=VALUE on both the local (container.rs:1450) and runs_on remote (:1510) paths, carrying the same value the build run at :541 uses.

Failure path: avocado.yaml sets source_date_epoch: 1700000000; an extension declares post_build: scripts/bake.sh that gzips or tars a generated file into the ext sysroot. Per ext/build.rs:705 that hook runs before the .raw is sealed, so wall-clock timestamps land in the artifact's content - gzip and tar headers, .pyc - which mkfs.erofs -T cannot normalize afterwards. The .raw then differs build to build while this entry tells the user post_build is covered.

Within one avocado build, Phase 1 runs the ext post_build hook with no epoch and ext image then seals the .raw with one, so the value is not consistent across hooks in a single build. The ext hook already holds config: &Config in scope, so this is a one-line wiring rather than a plumbing constraint.

[jetm]

The guard cannot see either runtime/build.rs call site, including the new one

Four call sites spell the injection - rootfs/image.rs:402, initramfs/image.rs:318, runtime/build.rs:541, runtime/build.rs:2890 - and this guard pins the first two. runtime/build.rs:541 matches the needle byte-for-byte but is never scanned, and :2890, the call this commit adds, is spelled (&mut env, source_date_epoch) so the needle can never match it.

Demonstrated rather than argued: I deleted both runtime/build.rs injections and ran the full suite - 1395/1404 unit tests plus every integration target passed, this guard included.

What that leaves exposed is the primary path. runtime/build.rs:541 feeds the runtime build script avocado build actually runs, the one interpolating the rootfs and initramfs sections (:2318, :2327). A refactor dropping it stops a configured epoch reaching the rootfs image, with the suite green throughout - verbatim the failure this file's own header says it exists to prevent. The commit's headline wiring at :2890 has zero coverage.

Credit where due: this is not the generated-text pattern - it reads the real source files, and deleting rootfs/image.rs:402 does turn it red with a clear message. Two narrower notes for the fix: it is a contains on source text, so it also passes on a commented-out call; and an allowlist of two paths only covers what someone remembers to add, where the sibling guard it cites sweeps all of src/ so new files are covered on landing.