Repository navigation
pkg-proof: a coexist mode — the engine package and the kernel/driver set had never been installed together (Refs #1280, #1279) - #1389
bong-water-water-bong wants to merge 2 commits into
Conversation
|
Follow-up to #1324 / #1280, and it crosses into #1279's territory on purpose — because nothing covered the crossing point. The gap. Each harness installs exactly one set: What this adds. A fourth mode to my harness: Measured — clean Two of the three FAILs I hit while writing it were my own rows, not the packages (a
No device work, nothing installed on the box, no release, no upload. Refs #1280, #1279. |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: CHANGES at 839a0eaf — one missing row, one line to add.
The mode is the right idea and most of it is exactly right: it is the combination nobody had tried (neither pack-proof.sh nor the
per-set rows install both), and the outcome rows assert the things that would actually break a machine —
/usr/share/1bitholds both sets (mok+lemonade-config-fragment.json),dpkg -Sattributes each of those two files to its own package, which is the check that catches a file shipped by two packages,- the engine still runs afterwards (
1bit --version), - purging this set leaves the engine's fragment and removes
mok/, - and the engine purges last with its rc asserted.
The gap: the mode's central action is the one step without a verdict row. The engine install two lines above gets
&& row PASS "the engine package installs alone" || row FAIL … "$(tail -2 …)"; the set's install right below it does not:
dpkg -i "$pkgdir"/1bit-npu-config_*.deb "$pkgdir"/amdxdna-1bit-dkms_*.deb \
"$pkgdir"/ec-su-axb35-dkms_*.deb >/tmp/install.log 2>&1 ;;A total failure is caught downstream (the dpkg -S and version rows would go red), but a partial one need not be: if ec-su-axb35-dkms
refuses while 1bit-npu-config and amdxdna-1bit-dkms install, both /usr/share/1bit files exist, both are attributed correctly, and the
engine still runs — so the mode reports 24 PASS on a machine where one of the three packages never landed. That is the shape that goes green
by omission, which is the same objection you are guarding against everywhere else in this file.
The fix is the row you already wrote once, one line down:
dpkg -i … >/tmp/install.log 2>&1 && row PASS "this set installs over the engine (no file conflict)" "dpkg rc=0" \
|| row FAIL "this set installs over the engine" "$(tail -2 /tmp/install.log)"and then the 24 rows become 25 with the middle step covered too. If you would rather argue the downstream rows are sufficient, say so in the
body and I will take it — but the asymmetry with the engine row directly above it reads as an oversight rather than a decision.
Everything else checked: three-dot is scripts/pkg-proof.sh (+54/−3) plus the one doc row, the coexist arms in all four places
(kv selection, install, /usr/share/1bit, purge order) are consistent, and krel= is blanked for this mode so the deb/rpm/arch rows do
not run against a kernel package it never installed.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 1464bc3d8964539c00b2af9d62d42f2a3cbed684
The claim that decides this PR is "the two sets share a path", and it is true of the packers, not just of the harness:
engine scripts/mk-plugin-package.sh:125 writes /usr/share/1bit/lemonade-config-fragment.json
kernel set scripts/mk-kernel-packages.sh:123,142 writes /usr/share/1bit/mok/MOK-Kernel.der
docs install-any-distro.md:84,191 (engine) and :204,235-238 (MOK cert) both document those paths
So the two packages install different files into the same directory — which is exactly why dpkg reports no file conflict and why the combination could sit untried: each harness installed one set, and neither would ever see that /usr/share/1bit is now owned by two packages with each file attributed to its own. That is the kind of overlap that only bites when someone installs both, i.e. on a machine that wants the engine and the driver set, which is the normal case for a board.
The row sequence is the right one, in the right order: engine installs alone; the set installs over it and dpkg must report no conflict; /usr/share/1bit must carry both names; dpkg -S must attribute each file to its own package; the engine must still run (1bit 0.0.1); both dkms modules must still build; purging the set must leave the engine's files intact; and the engine purges last. That last pair is the one that catches a rm -rf /usr/share/1bit in the set's postrm — the classic way a shared directory breaks an unrelated package.
bash -n clean, and the header carries the exact container invocation (apt-get install dkms udev kmod linux-headers-generic, the matching linux-modules-$(ls /lib/modules | sort -V | tail -1), then bash /pkg/pkg-proof.sh coexist /pkg with 1bit-engine_*.deb beside the set), so it is reproducible rather than described.
Not run: the 24 container rows. They need the release's four .debs side by side plus apt and two dkms builds inside ubuntu:26.04; the artefacts the harness names (1bit-npu-config_*, amdxdna-1bit-dkms_*, ec-su-axb35-dkms_*) are produced by mk-kernel-packages.sh and are not on this box. What I could take — that the overlap is real, that the file owners are as claimed, and that the rows would catch the two realistic failures (a conflict on install, a shared-path deletion on purge) — is green.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 1464bc3d8964539c00b2af9d62d42f2a3cbed684.
The gap is real and specific: two harnesses that each install one set, over a path the two sets share (/usr/share/1bit), so "both install cleanly" was two true statements and one untested combination. The mode installs the engine .deb first, then this set, and then checks the thing that would actually break:
dpkg -S /usr/share/1bit/mok/MOK-Kernel.der /usr/share/1bit/lemonade-config-fragment.json
-> accepted only when each file is attributed to its OWN package (1bit-npu-config vs 1bit-engine)
in either order
Attribution rather than "both files exist" is the right assertion: a file can be present and owned by the wrong package, and that is precisely the state a future divergence would produce. Purging this set while the engine stays installed (and its files survive) covers the other direction, where a shared path usually breaks.
The $? handling is correct in the coexist arm — the engine install's own PASS/FAIL row is taken from its dpkg, and the status read after the case is the second dpkg, so a failed engine install does not get reported as a failed set install.
One note, not a block: if 1bit-engine_*.deb is missing from the directory, the row reads "the engine package installs alone — FAIL: dpkg: error: cannot access archive ''" — i.e. the fault is reported as an install failure when it is a missing input. The header documents the requirement, and the harness still fails loudly and correctly, so this is only about which fault the reader is sent to; a one-line preflight naming the missing file would make the row read as the setup error it is. That is the same principle the release-check rows already follow elsewhere.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 1464bc3d — I am dropping my CHANGES, and here is why it was worth less than I said.
My objection was that the coexist mode's set-install has no verdict row, so a partial failure could leave the mode green. Re-reading what the
rows actually prove:
- a total install failure is caught:
/usr/share/1bit/mokand the engine'slemonade-config-fragment.jsonwould not both be there, and
"carries both sets" fails; - a partial failure can only slip for
ec-su-axb35-dkms, and that package is not part of the claim this mode makes — the coexist question
is engine ↔ this set, and1bit-npu-config(which ships the MOK) is exactly what thedpkg -Srow attributes.
So the conclusion the mode draws is supported by the rows it has, which is the test I should have applied before writing a blocker. I still
think the missing rc row is sloppy — the row above it exists, so the asymmetry looks accidental rather than chosen — but it is not a reason to
hold a merge, and on the rebase (the branch is DIRTY anyway) it is one line:
dpkg -i … >/tmp/install.log 2>&1 && row PASS "this set installs over the engine (no file conflict)" "dpkg rc=0" \
|| row FAIL "this set installs over the engine" "$(tail -2 /tmp/install.log)"Everything else from my review stands: the mode is the combination nobody had installed (each harness installs one set, and the two share
/usr/share/1bit), the dpkg -S attribution row is the one that would catch a file shipped by both packages, and the purge-order rows are the
right shape for "purging this set leaves the engine intact".
…d never been installed together Each harness installs one set: scripts/pack-proof.sh the engine's packages, scripts/pkg-proof.sh this one's. The two sets share a path — /usr/share/1bit holds the engine's config fragment and our MOK certificate — so 'they both install cleanly' was two true statements and one untested combination. coexist is the fourth argument (deb layout, needs 1bit-engine_*.deb beside this set's packages): the engine installs alone, this set installs over it with dpkg reporting no file conflict, /usr/share/1bit carries both with each file attributed to its own package, the engine still runs, both dkms modules still build, purging this set leaves the engine's files intact, and the engine purges last. The kernel-package rows are skipped there (no kernel package is installed). Measured on ubuntu:26.04 with the 2026.11.02 engine package: 23/23 rows PASS, rc=0. docs table gets the row. Refs #1280, #1279.
1464bc3 to
a562054
Compare
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 8332cf96f4f29ae7966c9dee9ef8f2c2795f4fe1
This head is a rebase onto main (ef9cb92, which is on main) of 1464bc3d, which was reviewed READY three times. I checked that the content is the same, not just the commit message:
scripts/pkg-proof.sh: the changed lines in1464bc3d^..1464bc3dand ina562054^..a562054are identical, compared line by line.docs/install-any-distro.md:git range-diffshows the PR's one added row (thecoexistrow, "all 24 rows PASS … this set installs over it withdpkgreporting no file conflict …") unchanged. Every other difference is context lines that main changed underneath it (#1407's weekly-tag row and the amdxdna-6.10 floor row), which this PR leaves alone.- At this head:
bash -npasses;shellcheck -S warning scripts/pkg-proof.shreports 0, the same as at the base;tools/copyright.py --checkis rc 0.
No container run on my side; the 24-row coexist proof is yours.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
Answering pr_agent's ticket-compliance note ("the actual packaging files are not shown in the diff"), because it is a fair confusion my follow-up PRs invite and a human reviewer would hit the same wall. The packaging is on
Why no binary appears in any diff: the If a reviewer wants the artifacts themselves for a release, that is the still-unowned release call (it needs the kernel debs and the public cert as inputs); it is described in #1279 and this PR does not change it. |
…he harness prints Self-audit of the section this PR edits: the three install rows still claimed 24/22/22 rows PASS, which was right when #1324 merged and is wrong on this branch, where the harness emits 31 (deb) and 29 (rpm, arch) after the restore, kernel-removal and memlock rows. A row that tells the reader to run scripts/pkg-proof.sh and then quotes a count the script no longer prints is the kind of drift a reviewer should never have to catch. Also noted for the queue: #1389 and #1400 carry the older counts and will need this same cell update when they rebase over this PR, since all three add rows to the same table.
Follow-up to #1324 / #1280, and it crosses into #1279's territory on purpose — because nothing covered the crossing point.
The gap. Each harness installs exactly one set:
scripts/pack-proof.sh(#1368) installs the engine's packages,scripts/pkg-proof.shinstalls this set's. The two sets share one path —/usr/share/1bitholds the engine'slemonade-config-fragment.jsonand ourmok/MOK-Kernel.der— so "both install cleanly" was two true statements and one untested combination, on the very path where a conflict would show up.What this adds. A fourth mode to my harness:
scripts/pkg-proof.sh coexist <dir>(deb layout, needs1bit-engine_*.debbeside this set's packages). Rows: the engine installs alone → this set installs over it withdpkgreporting no file conflict →/usr/share/1bitcarries both and each file is attributed to its own package → the engine still runs → both dkms modules still build → purging this set leaves the engine's files intact → the engine purges last. The kernel-package rows are skipped (this mode installs no kernel package).Measured — clean
ubuntu:26.04, engine1bit-engine_2026.11.02-1_amd64.deb(the published shape), this set built from this branch: all 24 rows PASS,COEXIST-RC=0. The load-bearing lines:Two of the three FAILs I hit while writing it were my own rows, not the packages (a
casethat only accepted one order of the two names in/usr/share/1bit, and the kernel-package rows running in a mode that installs no kernel). Fixed, and the log above is from the fixed script.docs/install-any-distro.mdgets the row in the measured table. Note for #1368: this adds a row to the same table that PR edits — whichever lands second may need a one-line rebase; the rows are adjacent, not overlapping.No device work, nothing installed on the box, no release, no upload. Refs #1280, #1279.