kernel-patches: make the carried series git am-able - #25
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a pre-build CI guard to ensure the carried Linux kernel patch series can be replayed with git am (i.e., mail headers are parseable), and fixes a known malformed From: header that previously broke git am for the whole series.
Changes:
- Add
scripts/lint-kernel-patches.shwhich validates patch mail headers viagit mailinfo. - Fix
0013-hid-flydigi-vader.patchto include a validFrom: Name <email>author identity. - Run the new lint step early in
.github/workflows/build.ymlto fail fast before the long build.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/lint-kernel-patches.sh | New lint script to verify kernel patch headers are git am-able using git mailinfo. |
| board/mister/de10nano/linux-patches/0013-hid-flydigi-vader.patch | Corrects the patch From: header to include an email so git am doesn’t fail. |
| .github/workflows/build.yml | Adds a pre-build CI step to run the new lint script and fail fast. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # mailinfo writes the split message body and diff out as files; we only care about | ||
| # the identity summary it prints, so they go to a scratch dir we discard. | ||
| scratch="$(mktemp -d)" |
There was a problem hiding this comment.
Fixed in 7b16dd0, and your point about repo convention was the useful part — I checked and you're right:
scripts/check-linux-img.sh:140 mktemp -d "${TMPDIR:-/tmp}/check-linux-img.XXXXXX"
scripts/ci-tests.sh:156-157 mktemp "${TMPDIR:-/tmp}/ci-tests-*.XXXXXX"
Both scripts now use that form. Worth noting export-kernel-tree.sh had the same bug in a subtler shape — it used mktemp -d -t <template>, which is no better: GNU deprecates -t, and BSD reads its argument as a prefix rather than a template. So the fix landed in both places, not just the reported one.
Verified after the change: lint passes and leaves no scratch dir behind, and the exporter still exits 0 with a byte-identical tree.
3a2490b to
0e0efd1
Compare
0013-hid-flydigi-vader.patch carried `From: Alexey Melnikov` with no
<email>. `git am` cannot build an author identity from that and hard-fails
the whole series on it:
fatal: empty ident name (for <>) not allowed
Buildroot never noticed because it applies patches with `patch -p1`, which
reads the diff hunks and ignores the mail headers entirely. The series
therefore builds a correct kernel and ships, and the defect only surfaces
when the patches are replayed as git history — which is exactly what an
export to a Linux-Kernel_MiSTer-style tree does.
The correct identity is not a guess. The patch's own mbox line names its
provenance, commit b1b168eb6 in MiSTer-devel/Linux-Kernel_MiSTer, whose
author is `Sorgelig <pour.garbage@gmail.com>` with a matching Date. (The
two names are one person — the fork's merge commit for PR #42 is authored
`Alexey Melnikov <pour.garbage@gmail.com>`. Whoever wrote 0013 took the
real name and dropped the address.) Using the fork's own ident also makes
0013 consistent with the other 15 Sorgelig-authored patches in the series.
Add scripts/lint-kernel-patches.sh so this cannot regress. It validates
with `git mailinfo` — the parser `git am` itself uses — rather than a regex
that approximates one, so it tests am-ability for real. It needs no kernel
tree and no network.
Wire it into build.yml before the build step rather than into ci-tests.sh,
which runs post-build: the check takes about a second, so in the existing
job it costs no additional runner minutes and fails fast instead of after a
possible 300-minute build.
Verified: with this fix the full series `git am`s cleanly onto v6.18.38,
31/31 on master (0038 lands with the diag branch and already parses).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MiSTer-devel/Linux-Kernel_MiSTer keeps the kernel as a materialized git tree:
a squashed tarball commit (`v5.15.1`) with MiSTer commits replayed on top. We
keep it as {pinned version + hash} + an ordered patch series. Those are the same
model — tarball base plus ordered series — differing only in whether the base is
stored as a hash or as 283MB of blobs. (Verified: that repo's `v5.15.1` base is
byte-for-byte pristine kernel.org v5.15.1. Its only delta is 11 absent files,
all `tools/`/`Documentation/` dotfiles lost to `.gitignore` during a `git add`
after extraction — no MiSTer change hides in the base.)
So the two formats are convertible, and this renders one into the other: a
pristine hash-verified tarball as one base commit, then one commit per carried
patch with authorship preserved, then an in-tree defconfig and an EXPORT.md.
It is a build output, not a second source of truth. Edits belong in the patch
series; this regenerates from it.
Design notes, all verified rather than assumed:
- Deterministic. `git am --committer-date-is-author-date` plus a base-commit
date taken from the extracted Makefile's mtime (kernel.org tarballs come from
`git archive`, so every file carries the tag's commit time — stable across
machines, unlike download time). Two runs produce identical SHAs, so
regenerating after no change is a no-op rather than a force-push of fresh
SHAs. This is what makes a 6.18.39 bump cheap: edit the version and the hash,
rerun.
- Fails closed. The tarball is checked against linux.hash and refuses to proceed
without a matching entry — verified by bumping the version with the tarball
cached but no hash present.
- Ships arch/arm/configs/MiSTer_defconfig, the path the fork already uses, so
the tree builds without Buildroot (`make ARCH=arm MiSTer_defconfig && make
zImage`) — the thing `make linux` inside Buildroot cannot hand anyone.
Verified that `make ARCH=arm MiSTer_defconfig` yields a .config byte-identical
to Buildroot's own `cp linux.config .config && make olddefconfig`, so the
exported tree cannot build a different kernel than the image.
Written in the kernel's minimized form, unlike the fork's 4247-line expanded
.config, which bakes CONFIG_CC_VERSION_TEXT — one machine's gcc build string —
into a file whose purpose is portability.
- Emits an ORPHAN branch. Attaching it to the 5.15 history needs a merge whose
tree ignores its first parent, which makes `git log` list ~113 commits whose
changes are absent from the tree: a reader sees "xone: update driver" and
concludes xone is present, when it is a Buildroot package now. A log listing
absent changes is worse than an absent ancestor. Disposition per fork commit
lives in MISTER-KERNEL-PATCH-RECON.md, which cites the superseding vanilla
commit — something no git command can produce, since across this much context
drift `git patch-id` matches nothing.
- Never touches a remote. Publishing is an explicit fetch-into-a-fork plus push,
spelled out in the generated EXPORT.md.
Verified end to end: exports Linux 6.18.38 + 31 patches, 34 commits, clean tree.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The orphan branch was wrong, and provably so: GitHub's compare API 404s with
"No common ancestor", so an orphan branch cannot be PR'd anywhere at all. I had
described that as the compare view being "meaningless"; it is stronger than that.
The reason it was wrong is that I mis-read the fork's structure. Linux-Kernel_MiSTer
is not one chain. Its tarball commits form a spine
e12ed6c19 v5.13.12 -> 137491a75 v5.14 -> b6f2ca1c4 v5.14.5 -> aba1ef4c1 v5.15.1
and each MiSTer-vX.Y branch hangs off a spine point with the MiSTer series replayed
on top (MiSTer-v5.15 = aba1ef4c1 + 112, MiSTer-v5.14 = 137491a75 + 63). They are
siblings, not a line. Every spine commit is a pristine tarball with no MiSTer code.
So --parent-repo/--parent parents the base commit on the newest spine point, which
is simply what this project already does, four times. It gets all three properties
at once, where each earlier design could only get two:
- PR-able: merge-base with MiSTer-v5.15 is aba1ef4c1, so GitHub can compare.
- Honest log: no MiSTer-5.15 commit appears, because they are siblings rather than
ancestors. This was the entire objection the orphan existed to avoid, and
parenting on the spine dissolves it — parenting on the branch TIP would have
reintroduced it, listing ~112 commits whose changes this tree discards.
- Meaningful base: its parent is a pristine tarball too, so the diff across it is
the pure upstream 5.15.1 -> 6.18.38 delta. Verified: 80077 files, zero
MiSTer-named paths on either side.
Also fix `git add` -> `git add --force` for the base commit. The kernel ships
.gitignore files matching paths it also tracks, so a plain add after a tarball
extract silently drops them. This is not hypothetical — it is exactly why the fork's
own v5.15.1 base is NOT byte-identical to kernel.org's v5.15.1 (11 files absent:
Documentation/.yamllint, fs/*/.kunitconfig, selftests/bpf/test_progs.c,
selftests/arm64/tags/* to the `tags` ctags pattern). Without --force we reproduced
it, losing Documentation/.renames.txt.
With --force the base tree hash is d13b0d25dbbc19af5884d0b780c74309c5d3fa1e —
byte-identical to kernel.org's v6.18.38 tree, and independently checkable against
their git. Our base is strictly more faithful than the one it extends.
Verified: spine mode is deterministic (identical SHAs across runs), 38 commits
(4 spine + base + 31 patches + defconfig + EXPORT.md), merge-base aba1ef4c1.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-tree
Without this the exported tree builds a kernel with no Xbox (xone) and no 11ac
WiFi, while MiSTer-v5.15 vendors both in-tree -- a silent feature regression for
anyone who builds the export expecting what MiSTer ships.
Vendors all four enabled kernel-module packages (xone + rtl8812au/8814au/8821au,
1882 files) at the paths the 5.15 branch uses, and emits build-mister-modules.sh
to build them.
NOT wired into Kconfig, deliberately. The Realtek Makefiles do this, ABOVE their
own `ifneq ($(KERNELRELEASE),)` guard:
export TopDIR ?= $(shell pwd)
$(shell cp $(TopDIR)/autoconf_..._linux.h $(TopDIR)/include/autoconf.h)
Parse-time filesystem mutation keyed off `pwd`. In-tree, `pwd` is the kernel root
rather than the module dir, so TopDIR points at the wrong tree and the driver's
generated autoconf.h silently never appears -- $(shell ...) eats the error. These
2594-line Makefiles assume they are never in-tree, across ~1900 files. Wiring them
in would mean inventing hooks no upstream tests, then maintaining patches to
upstream Makefiles forever. So they are built through the exact out-of-tree
invocation Buildroot already uses -- upstream's own supported path, proven by our
image builds -- with each recipe read from its .mk rather than reinvented. A
driver bump is a pin change in the .mk plus a re-run; no rewiring.
Three bugs found by actually building the result, not by reading it:
- SILENT DRIVER OMISSION. The defconfig annotates package lines with trailing
comments ("BR2_PACKAGE_RTL8812AU=y # RTL8812AU 11ac -- ..."), so anchoring the
match on `=y$` hit only BR2_PACKAGE_XONE. The export vendored xone alone and
dropped all three WiFi drivers -- exactly the regression this code exists to
prevent, with the hole sitting upstream of the fail-closed MODULE_PATH check.
Fixed, plus the identical latent bug in defconfig_value(), plus a post-export
assertion that every enabled driver is really in the committed tree.
- WRONG BUILD PRECONDITION. `modules_prepare` is not enough: external modules link
against Module.symvers, which modpost writes during `make modules`, which needs
vmlinux from the zImage build. Without it every kernel symbol reads as undefined
("ERROR: modpost: \"skb_pull\" [8812au.ko] undefined!") -- which blames the
driver when nothing is wrong with it. The script now checks and says so.
- VERMAGIC MISMATCH. This tree is a git repo ~35 commits past the v6.18.38 base,
so setlocalversion correctly appends "+" -> 6.18.38+. Buildroot builds the same
source from a tarball with no git, so its equally-patched kernel says plain
6.18.38. That "+" lands in vermagic and modprobe rejects every module on it.
Passing LOCALVERSION= (set, empty) suppresses it. Verified: rebuilt 8812au.ko
now reports `vermagic=6.18.38 SMP mod_unload ARMv7 p2v8`, byte-identical to
Buildroot's -- so the export's modules and the image's are interchangeable.
Verified end to end with the real ARM toolchain: all 4 drivers build (8812au.ko,
8814au.ko, 8821au.ko, 9 xone modules), 43 commits, 1932 files, deterministic
across runs. Committed file counts match the tarballs exactly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Upstream created MiSTer-v6.18 (d9ac12a691) as its own vanilla 6.18.38 commit and asked for the changes as a PR against it. Notably it extends the spine at aba1ef4c1 (v5.15.1) -- independently the same structure --parent produces, which is a good sign the shape was right. But `--parent aba1ef4c1` builds OUR OWN base commit from the tarball, so our branch and theirs each carry a distinct "v6.18.38" commit over the same spine point. A PR would be diverged (ahead 39, behind 5) and merging would fuse two different bases. --onto replays straight onto an existing base instead: no base commit, no tarball needed for the kernel, and the result fast-forwards. Verified: 31/31 apply to d9ac12a691, 38 commits on top, their commit is an ancestor, so the PR is exactly our delta with nothing of theirs restated. Two guards, both from things that actually went wrong here: - The base's version is read back from its own Makefile and must equal the defconfig pin. Replaying a 6.18 series onto a 5.15 base otherwise fails deep in `git am` with conflicts that look like bad patches rather than a bad base. Verified: --onto aba1ef4c1 is refused with "is Linux 5.15.1, but this repo pins 6.18.38". - --onto is resolved to a SHA in the SOURCE repo before cloning. Ref names are ambiguous across a clone boundary, and this bit for real: `git clone` copies the source's LOCAL branches to origin/*, so `--onto origin/MiSTer-v6.18` resolved inside the clone to the source's own local MiSTer-v6.18 -- our tree, not theirs -- and replayed the series onto a tree that already had it. The version check could not catch it: both were 6.18.38. `git am` did, with "MiSTer_fb.c: already exists in index". Their base is missing Documentation/.renames.txt versus kernel.org's v6.18.38 (tree 4efcf6f42 vs d13b0d25d) -- the same .gitignore-eats-a-tracked-file bug that cost their v5.15.1 base 11 files, and that `git add --force` was added here to avoid. Cosmetic, no build impact, not ours to fix in a PR; raised upstream instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two real findings from the PR #26 review, both reproduced before fixing: - `series=("$patch_dir"/*.patch)` without nullglob leaves the literal pattern in the array when nothing matches, so `((${#series[@]}))` sees length 1 and the "no patches" guard never fires. `git am` then fails on a path that does not exist, blaming a patch rather than the empty directory. Verified: an empty dir gave length 1 with element "<dir>/*.patch". Now scoped `shopt -s nullglob` around the glob; an empty patch dir dies with "no patches in <dir>". - The download path used `tarball="$(mktemp -d)/..."` with no cleanup, stranding a ~148MB kernel tarball per run, and bare `mktemp -d` is a GNU extension that errors on BSD/macOS. Now an explicit template plus an EXIT trap. --output is deliberately not cleaned: it is the deliverable and must survive a failure to be diagnosable. The review's third finding -- that `git add --all` respects .gitignore and needs -f -- was already fixed in this branch (the reviewer saw an earlier revision). That is the same bug that costs the upstream v5.15.1 base 11 files and its v6.18.38 base Documentation/.renames.txt; `git add --all --force` is why our tree hash matches kernel.org's exactly. The first cut of the trap made it worse, which is why this was tested rather than assumed: as an EXIT trap, cleanup()'s own return status becomes the script's exit status, so a bare `[[ -n $download_dir ]] && rm -rf ...` returned 1 on every run that used the dl/ cache -- the export printed PASS and exited 1, which would fail CI on success. Rewritten as an `if`. Verified after the change: export still exits 0, output SHA unchanged (9b485e0988), 31/31 patches apply. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review finding on #25, valid and checked: bare `mktemp -d` is a GNU extension -- BSD/macOS mktemp requires a template and errors without one -- and this repo already established the pattern elsewhere: scripts/check-linux-img.sh:140 mktemp -d "${TMPDIR:-/tmp}/check-linux-img.XXXXXX" scripts/ci-tests.sh:156-157 mktemp "${TMPDIR:-/tmp}/ci-tests-*.XXXXXX" lint-kernel-patches.sh was the reported site. export-kernel-tree.sh was the same bug: the fix there used `mktemp -d -t <template>`, which is no better -- GNU deprecates -t, and BSD reads its argument as a prefix rather than a template. Both now use the repo's form. Verified: lint passes and leaves no scratch dir behind; export still exits 0 and produces a byte-identical tree (same SHA as the pushed branch), so this touches the scripts and not their output. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1d5050e to
7b16dd0
Compare
`readonly REPO_ROOT="$(cd ... && pwd)"` masks the subshell's exit status (shellcheck SC2155). Noticed while writing scripts/check-fork-sync.sh: shellcheck is installed here and flagged it, and the repo's existing scripts (check-linux-img.sh, ci-tests.sh) have zero SC2155 hits -- so these two were the outliers, not the convention. Not enforced in CI, and the masked failure is not reachable in practice (dirname of BASH_SOURCE does not fail). Fixed anyway: it is two lines, and a script that trips the linter everyone else passes is an invitation to stop running the linter. Verified: both shellcheck clean, lint still passes 31/31, export still exits 0 with a byte-identical tree (same SHA as the pushed branch). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
What
Two related things, in the order they were found:
Linux-Kernel_MiSTer-style git tree — which is what surfaced (1), and what upstream has now accepted: MiSTer-devel/Linux-Kernel_MiSTer#75.1.
git am-ability0013-hid-flydigi-vader.patchcarriedFrom: Alexey Melnikovwith no<email>.git amcan't build an author identity from that and hard-fails the entire series:Buildroot never noticed: it applies patches with
patch -p1, which reads the diff hunks and ignores mail headers entirely. So the series builds a correct kernel and ships. The headers are a real interface our primary build does not exercise.The fix isn't a guess. The patch's own mbox line names its provenance —
b1b168eb6inMiSTer-devel/Linux-Kernel_MiSTer, authoredSorgelig <pour.garbage@gmail.com>,Date:matching exactly. (Same person; the fork's merge commit for PR #42 is authoredAlexey Melnikov <pour.garbage@gmail.com>. Whoever generated 0013 took the real name from a profile and dropped the address.) It also makes 0013 consistent with the other 15 Sorgelig-authored patches.scripts/lint-kernel-patches.shstops it regressing. It validates withgit mailinfo— the parsergit amitself uses — rather than a regex approximating one, so it tests am-ability for real, needs no kernel tree and no network. Wired intobuild.ymlbefore the build: ~1s, so it costs no additional runner minutes in the existing job and fails fast instead of after a possible 300-minute build.2.
scripts/export-kernel-tree.shRenders the pinned-tarball-plus-series model into the materialized git tree upstream wanted. The two formats are the same model — verified: that repo's
v5.15.1base is byte-for-byte pristine kernel.org v5.15.1, its only delta being 11 absent files lost to.gitignoreduring agit add.Verified rather than asserted:
make ARCH=arm MiSTer_defconfig→CONFIG_FB_MISTER=y,CONFIG_SND_MISTER_AUDIO=y.configis byte-identical to Buildroot'scp linux.config .config && olddefconfigpatch -p1): 0 of 91,200 files differ8812au.ko,8814au.ko,8821au.ko+ 9 xone modules, real ARM toolchainKey decisions, each forced by evidence:
--parent), or replays onto an upstream-published base (--onto). An orphan branch can't be PR'd at all — GitHub's compare API 404s on unrelated histories.git add --force— the kernel ships.gitignores matching tracked paths. Without it we'd reproduce the exact bug that costs upstream's bases their missing files. With it, our base tree hash equals kernel.org'sv6.18.38^{tree}byte-for-byte.$(shell cp ...)keyed off$(shell pwd), which in-tree resolves to the kernel root. They're built out-of-tree via a generatedbuild-mister-modules.sh, using the same invocation Buildroot already uses.LOCALVERSION=on the kernel build, or the git tree reports6.18.38+, that lands in vermagic, and modprobe rejects every module.Which patches were carried into 6.18, which were left behind, and why is in
docs/patch-provenance.md(disposition + cited evidence per fork commit) anddocs/kernel-recon/.Verification
From:→ exit 1, names the patch and the line.git ams cleanly ontov6.18.38— I built the tree rather than inferring it.🤖 Generated with Claude Code