exclude package-manager state from extension images

[mobileoverlord]

Third of the set alongside #203 and #204. Independent of both — no overlapping files.

Are we doing exclude-path today?

No. --exclude-path is only emitted for user-configured var_files patterns, and get_ext_var_files returns an empty vec when the key is absent (config.rs:1233). There is no default, so a project that sets no var_files gets no excludes at all.

Confirmed in the shipped images rather than inferred from the sysroot — grepping the built .raw files:

image	size	rpmdb.sqlite	SQLite format 3	history.sqlite
config-dev-0.1.0.raw	848K	✓	✓	—
avocado-ext-tunnels-2024.1.0.raw	9.9M	✓	✓ (×9)	✓

Scale

avocado-ext-tunnels sysroot is 22M: 13.4M of package-manager state (2.8M var/lib/rpm, 4.2M var/lib/dnf, 6.4M var/cache/dnf) against an 8.2M /usr payload. The bookkeeping is larger than the extension.

Every extension carries a floor of it — ext install and ext dnf seed each installroot with cp -rf $AVOCADO_PREFIX/rootfs/var/lib/rpm so dependencies resolve against what the rootfs already provides, and nothing removes it before the sysroot becomes an image. config-dev-0.1.0.raw and avocado-bsp-qemux86-64-2024.1.0.raw are both exactly 868,352 bytes: two unrelated extensions, identical size, because both are dominated by that same seeded rpmdb instead of their own payloads.

Why it matters beyond size

Nothing on target can read it. systemd-sysext/confext merge /usr, /opt and /etc — never /var. It is shipped and never mounted.

And it is what stops the images being reproducible across a reinstall: the rpmdb stamps INSTALLTIME/INSTALLTID per package, history.sqlite records the transaction, var/cache/dnf holds generated repodata and .solv files.

Worth being precise, since the content-addressed IDs in connect do dedupe today and that is not a contradiction. INSTALLTIME is written at install time and then sits unchanged, and dnf is a no-op when packages are already present, so re-imaging an unchanged sysroot is byte-stable and dedupes exactly as observed. What fails is re-installing — clean machine, post-avocado clean, a different CI runner. That is the independent-rebuild property, and it is the one the dedup metric can't see because it never re-installs.

Excluded, not deleted

Deliberate, and the difference from #203. ext image runs mkfs directly against the live $AVOCADO_EXT_SYSROOTS/<name> — there is no work copy, unlike the rootfs and initramfs paths — and later ext dnf / ext install calls resolve against that rpmdb. Deleting it would clobber live state; excluding costs nothing and uses the mechanism already there for var_files.

Operational note

This changes every extension's content hash exactly once. Against a content-addressed store that means a one-time dedup miss and a full re-upload wave in connect — everything looks new for one cycle. Harmless, but probably worth timing deliberately rather than landing mid-release.

Tests

Excludes applied on both the erofs and mksquashfs branches; var_files patterns still excluded alongside rather than displaced.

Also filled the gaps in what was pinned for extension reproducibility, since none of it was covered: erofs -U 0000… (else every build gets a fresh UUID), --all-root (else ownership comes from the build user), -T "$SOURCE_DATE_EPOCH", and on the squashfs side -reproducible and -no-xattrs.

One existing test changed meaning: test_create_build_script_no_var_files_no_excludes asserted that no var_files meant no excludes at all, which is no longer true. Rewritten as test_no_var_files_leaves_only_the_pkg_state_excludes, asserting exactly three excludes — same spirit (nothing unexpected gets excluded), and it now also catches accidental duplicates.

Verified mksquashfs tolerates the repeated -e form the existing codegen emits, so nothing changed there.

Full suite green (1404 passing), clippy clean with -D warnings.

[Copilot]

Pull request overview

This PR updates extension image generation to always exclude package-manager state from produced sysext/confext images, reducing image size and improving reproducibility across clean reinstalls.

Changes:

• Always exclude var/lib/rpm, var/lib/dnf, and var/cache/dnf from extension images (both mkfs.erofs and mksquashfs paths), while still honoring user-configured var_files excludes.
• Add/adjust unit tests to assert the new always-excluded paths, preserve var_files behavior, and pin key reproducibility-related image-build flags.

---

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

[jetm]

Core change is correct and it closes the exact gap I flagged on #203 — extension .raw images were the one path still shipping a full rpmdb copy. Excluding rather than deleting is the right call given later ext dnf / ext install still resolve against that database, and the comment's load-bearing claim checks out: rootfs and initramfs build from a cp -a work copy (src/commands/rootfs/image.rs:142) while ext image runs against the live sysroot, which is why they can delete and this cannot. The var/lib/rpm path choice matches how this repo actually seeds the rpmdb (ext/install.rs:603, ext/dnf.rs:270), and dnf's logdir points at the SDK prefix rather than the ext sysroot, so there is no var/log/dnf leak the allowlist misses. cargo fmt, clippy -D warnings and cargo test --lib ext::image (18/18) are green at 572d6ee.

I mutation-tested the new assertions, and the four package-state tests are genuinely wired: replacing the three-path array with an empty one fails all four. Good.

The two reproducibility tests are not.

src/commands/ext/image.rs:1211 — both new reproducibility tests duplicate assertions that already exist, and the doc comment's premise is false. It says these flags were something "which nothing pinned". I stripped --all-root and -reproducible from the templates: the two pre-existing tests failed and caught it. The new pair adds no coverage — under the same mutation that emptied the exclude array, they stayed green while the four real tests went red. Worth deleting rather than keeping, since a test that cannot fail teaches the next reader that this area is covered when the coverage is elsewhere.

Nits:

• src/commands/ext/image.rs:906 and :929 — exclude_section's is_empty() branch is now unreachable in both arms, since excludes always carries at least three entries.
• src/commands/ext/image.rs:1234 — the comment says -reproducible makes squashfs output stable. It does not normalize ownership the way the erofs arm's --all-root does. I saw this directly: a live mksquashfs run with the exact emitted flag shape reported Number of uids 1: tiamarin (1000).
• No CHANGELOG.md Unreleased entry for a user-visible change in image contents. Weak signal, the convention is inconsistent in recent history.

Two notes on my own verification. That same live mksquashfs run confirmed all three directories are correctly excluded, which is worth stating because the repeated--e form works somewhat by luck — mksquashfs treats everything after the first -e as filenames, so the stray -e tokens are silently ignored as nonexistent paths. And I did not run the erofs path: mkfs.erofs is blocked by a local safety hook here, so the erofs exclude semantics are reasoned from erofs-utils' relative-to-c_src_path matching rather than observed.

[mobileoverlord]

All taken, in 1b773da.

The two duplicate tests are gone. You were right on both counts and I confirmed the duplication before deleting: test_create_build_script_erofs_contains_reproducible_flags already asserts -U 00000000-… and --all-root, test_create_build_script_squashfs_contains_reproducible_flags already asserts -reproducible and -no-xattrs, and test_create_build_script_source_date_epoch_default already asserts the -T. So the new pair added exactly nothing, and the doc comment's "which nothing pinned" was false.

Deleted rather than kept, for your reason: a test that duplicates its neighbour is worse than an absent one, because it teaches the next reader that the area is covered here when the coverage is twenty lines up. That is the second instance of this in the review — same call on #203's vacuous guard — and I would rather take the correction twice than argue it.

The four package-state tests stay; those are the ones that go red when the array is emptied, as you measured.

Unreachable is_empty() arms. Both collapsed to a single map + collect. Output is byte-identical — same leading separator, same joins — and the existing exclude assertions cover both shapes.

The squashfs ownership claim. Corrected, and I kept your measurement rather than just deleting the sentence: there is now a comment at the mksquashfs site saying -reproducible does not normalize ownership the way erofs's --all-root does, with your Number of uids 1: <builduser> observation as the evidence. That means squashfs ext images still vary by build user while erofs ones do not — a real asymmetry that was about to be papered over by a comment claiming otherwise.

The fix is -all-root on the mksquashfs invocation. Not doing it here: it changes image contents and ownership semantics for anyone building squashfs extensions, which deserves its own change rather than riding along in an excludes PR. Say the word if you would rather see it now.

The repeated -e form. Also recorded in the code, because "works by luck" is exactly the kind of thing that gets refactored into breakage by someone who assumes each -e is parsed as a flag. The comment says mksquashfs reads everything after the first -e as filenames and silently ignores the nonexistent ones.

CHANGELOG. Added under Changed, with the measured numbers and why exclusion beats deletion on this path.

Your live mksquashfs run is what made three of these actionable rather than speculative — thank you for running it. The erofs exclude semantics stay reasoned-not-observed, same gap as #203 and #204, and I have not closed it either.

[jetm]

Re-reviewed the increment since 572d6ee. Nothing blocking - all three claims in the subject hold, and no coverage was lost. Detail, since a removal commit deserves more than "looks fine":

Deleted tests were genuinely duplicates. Both called create_build_script with arguments identical to a surviving test, so the surviving asserts run against a byte-identical script. Checked by mutation rather than by reading: deleting each flag from the format string and rebuilding turns exactly one surviving test red, so all five deleted assertions still have a live owner - --all-root and -U 0000... to test_create_build_script_erofs_contains_reproducible_flags, -T "$SOURCE_DATE_EPOCH" to test_create_build_script_source_date_epoch_default, -reproducible and -no-xattrs to test_create_build_script_squashfs_contains_reproducible_flags.

The branches were dead. excludes at image.rs:888 is a fixed 3-element array chained with var_excludes, so it is unconditionally non-empty. Stronger than the commit claims, in fact: extracting the old and new formulations into a standalone binary and diffing their output at n=0,1,3,4 gives byte-identical results at every cardinality including zero, so map+collect handles the empty case natively and the arms were redundant as well as unreachable.

The corrected claim is right and does not overshoot. The deleted doc comment said the rest of the erofs contract was unpinned, which was false - image.rs:1053/1061 and :1224 pinned all three. The replacement checks out too: running mksquashfs 4.7.5 with the generated argv shape reports Number of uids 1 despite -reproducible, so ownership genuinely is not normalized, and the container path bindfs-maps to the host uid (container.rs:1109).

Also emptied the excludes array to confirm the four package-state tests guard it, exactly as the commit says. No dangling references to either deleted test name remain. Checked every deletion against the minimality exemptions - no exempt category applies.

One correction for the commit body, not the code: it attributes all three erofs flags to *_contains_reproducible_flags, but that test never asserts -T; the third test does.

Two advisory notes were withheld rather than appended here.

[jetm]

Re-reviewed the removal increment at 1b773da - deleted tests verified as genuine duplicates by mutation, dropped branches verified unreachable, corrected claim checked against a real mksquashfs run. Nothing blocking. Approving.