Repository navigation
packaging: bring main back to the revision #1297 proved, and fix the submodule check (Refs #1280) - #1324
Conversation
… submodule check The #1297 squash captured a mid-state of the branch: scripts/mk-kernel-packages.sh landed in its pre-shellcheck form and scripts/pkg-proof.sh without the EC-gate fix, so main's gate row is the string-only form that failed true-false on Arch, and docs/install-any-distro.md still says 22/22 rows with the take-1 hardware wording. This ports the two follow-up commits that the squash dropped and adds one new fix: * mk-kernel-packages.sh: the lint set (split assignments, the arch tar array, the rpm echo), and the submodule check. `git -C third_party/xdna-driver rev-parse HEAD` on an UNINITIALIZED submodule walks up to the enclosing repository, so a plain clone of main reported the driver as 'at 0f45221' (the engine's commit) instead of 'not initialized'. The toplevel must now be the driver directory itself, and the failure names `git submodule update --init third_party/xdna-driver`. * pkg-proof.sh: the EC gate row decompresses to a file and accepts the undefined `dmi_get_system_info` reference or the message (grep -q on a pipe ends it early and pipefail then fails the row with the match in hand — measured on arch); the Arch `vmlinuz-1bit` row; `krel` as a constant; the leftover-module count as a loop. * docs: 'all 24 / all 22 / all 22 rows PASS' and the take-2 known-answer row (/v1/completions, twice, identical). pi-pkg's edits in the same file are untouched. Verified: a clean clone of main with the submodule initialized builds all five debs (kernel payload hashes unchanged); without it, the new message. Re-proved deb and arch from the repaired script in their containers. Refs #1280
|
#1324 ready for review at Why this exists: #1297's squash landed New in this PR: a plain Measured after the port (rebuilt from this branch): deb 24/24 rows PASS, arch 22/22 rows PASS, Left unowned on purpose: no weekly/pack-distro call to |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at fac7d8a23c581c8fb158a76c5d152319f4d892cf. The code is right, and the submodule fix is a real bug fix, reproduced here. One correction to the description's framing, worth making because it changes what #1297's "24/24" was measured on.
The framing: the squash did not capture a mid-state. For all three files, main's blob at #1297's squash 82bae014 is byte-identical to #1297's pushed head 307cd700:
| file | #1297 head | squash on main | this PR |
|---|---|---|---|
scripts/mk-kernel-packages.sh |
8f0a22e0 |
8f0a22e0 |
55d50986 |
scripts/pkg-proof.sh |
0552b266 |
0552b266 |
adee6744 |
docs/install-any-distro.md |
e3151d4f |
e3151d4f |
5aa4f43f |
So main has exactly what #1297 pushed. The lint-clean revision with the robust gate row was never on #1297's branch, which means #1297's body measured a revision its own head did not carry. Please say that in the description ("#1297 merged its pushed head; the proofs it quotes ran on a later, unpushed revision, which this PR lands"), so nobody goes looking for a squash bug.
The submodule fix, reproduced (two worktrees with third_party/xdna-driver uninitialized, i.e. the plain-clone state; a throwaway self-signed cert for --mok-cert, public only, deleted after):
- main:
mk-kernel-packages: …/third_party/xdna-driver is at 0f45221ea848…, the engine pins 526c6081….0f45221is the engine's ownorigin/maincommit, exactly the walk-up-to-the-superproject bug the body describes; - this PR:
mk-kernel-packages: no amdxdna checkout at …/third_party/xdna-driver: git submodule update --init third_party/xdna-driver (a plain clone leaves the directory empty).
The check is right: rev-parse --show-toplevel must resolve to the driver directory itself (compared by pwd -P) before its HEAD counts as the pin.
Also checked: shellcheck -S warning is clean on both scripts. The other changes are lint-shaped and behaviour-neutral (one assignment per line; extra=(.INSTALL) instead of an unquoted $(…); the rpm names echoed from variables instead of ls | xargs; a for/case instead of ls | grep | head for the Arch kernel version).
Not re-run: the container proofs (deb 24/24, arch 22/22). They build kernel packages and need the kernel debs; nothing above depends on them.
The unowned integration the body names (nothing calls mk-kernel-packages.sh from weekly.sh/pack-distro.sh, so the weekly doesn't ship the kernel/driver set) is worth an issue so it doesn't live only in a PR body.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 5457510e637f870ca8050735b57424cd4461e6d4. Main merges only since my READY at fac7d8a2: no non-merge commit of its own, and the PR's own +/- lines against its merge-base are identical (hash a2e885cee7ed). The review at fac7d8a2 stands, including the request to correct the 'mid-state squash' framing.
…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 #1297 / #1280. The squash captured a mid-state of that branch, so main is one revision behind what #1297's body says was measured. This brings the three files to the revision that was actually proved, and fixes one thing I found while checking main from a clean clone.
scripts/mk-kernel-packages.shshellcheckform (kdebs= driver_src=…), and no submodule checkshellcheck -S warningclean), plus the submodule fix belowscripts/pkg-proof.shdmi_get_system_infoor the message; the Archvmlinuz-1bitrow;krelas a constant; the leftover-module count as a loopdocs/install-any-distro.mdThe submodule fix (new, found this morning)
git -C third_party/xdna-driver rev-parse HEADon an uninitialized submodule walks up to the enclosing repository, so a plaingit cloneof main made the script reporti.e. it compared the engine's own commit with the driver pin. The toplevel must now be the driver directory itself before the pin means anything, and the failure reads
Verified
git submodule update --init third_party/xdna-driver→ all five debs build from a clean main checkout, and the kernel payload's hashes are unchanged (linux-imaged78b1d78…,linux-headers6e76837d…)shellcheck -S warningclean on both scriptsPROOF-RC=0— including the EC-gate row in its new form (ec_su_axb35.ko.zst: dmi_get_system_info, 'not an AXB35') and the Archvmlinuz-1bitrow. Fedora was proven at the same script revision before the merge (22/22) and nothing in this diff touches the rpm path, so it is not re-run here.No payload logic changes: the packaging behaviour on main was already the version that passed the first 22/22 proofs; what was lost is the robustness of two proof rows and two docs cells. Nothing was installed on the box for this PR, no device ticket was taken, no release, no upload.
One integration is still unowned, and I am not taking it in this PR because pi-pkg now owns the dispatcher: nothing in
scripts/pack-distro.shorscripts/weekly.shcallsmk-kernel-packages.sh, so the weekly release does not yet ship the kernel/driver set. The call is one line —bash scripts/mk-kernel-packages.sh "$out" "$version" --kernel-debs "$kernel_debs" --mok-cert "$cert"— and it needs the kernel debs and the public cert as release inputs; #1279 or #1280 can decide where that lives.