From f2040d05153038954e36cb7110f8e2b8fe9a311a Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 1 Oct 2026 14:30:39 -0300 Subject: [PATCH 1/2] release: notes say what changed; the report is KVM-only at 20 boots a row The notes said which checksums moved and which checkpoints stop resuming, not why. hack/releasenotes reads git between the previous release and this one and writes what reached the machine: the commits by the part they changed (QEMU and firmware, kernel, base image, the definition and CLI), the pins that moved, the kernel options turned on, off or changed, and the patches added, changed or removed. Commits that did not reach the machine are listed apart, collapsed. It leads the notes, above the fingerprint verdict. hack/release ships every qboot patch, not a list of them: mtrr.patch was added to the build and not to the list, and v20261001.01 shipped a qboot.bin without the change it was built with. The report drops accel=tcg (the TCG build is for CI runners without KVM; timing it says nothing about a host) and boots each row 20 times: at 3 the v20261001.01 comparison flagged every KVM row 5-11% slower, and 20 boots of each release, alternated on construct, put them within 5 ms. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/release.yml | 15 ++ Taskfile.yml | 2 +- boot/bench_test.go | 3 +- boot/report_test.go | 17 +- docs/releasing.md | 17 +- hack/release | 13 +- hack/releasenotes/main.go | 360 +++++++++++++++++++++++++++++++++ hack/releasenotes/main_test.go | 149 ++++++++++++++ 8 files changed, 550 insertions(+), 26 deletions(-) create mode 100644 hack/releasenotes/main.go create mode 100644 hack/releasenotes/main_test.go diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index eda935d..16f4de1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -167,8 +167,23 @@ jobs: > /tmp/verdict.md cat /tmp/verdict.md + + # What reached the machine since the release before, read from git: the commits by the + # part they changed, the pins that moved, the kernel options and the patches. Against + # the newest published release that is not this one, as the verdict is. + previous=$(gh release list -R "$GITHUB_REPOSITORY" --limit 30 --json tagName -q '.[].tagName' | + grep -vx "$VERSION" | head -1 || true) + if [ -n "$previous" ]; then + go run ./hack/releasenotes -from "$previous" -to HEAD > /tmp/changes.md + else + echo "The first release: nothing before it to say what changed against." > /tmp/changes.md + fi + cat /tmp/changes.md + { echo 'notes< run at all, and where the JSON goes // SPIN_REPORT_FLAGS="..." spin-machine boot flags added to every boot, recorded // SPIN_REPORT_ONLY= only the rows whose id matches, for iterating on one question -// REPS= boots per row (default 3) +// REPS= boots per row (default 20) // // Needs /dev/kvm and a built release tree (SPIN_MACHINE_OUTPUT, or _output). The vsock rows -// need /dev/vhost-vsock, the TCG rows the release's TCG build; without them those rows are +// need /dev/vhost-vsock; without it those rows are // listed as skipped rather than reported as failing. With sudo and /dev/nbd0 the getty is // replaced by an echo, as in TestBootCost, and every variant of the image runs; without them // the usable column carries agetty's second and the variants that edit the image are skipped. @@ -85,7 +84,7 @@ func TestReport(t *testing.T) { t.Skip("set SPIN_REPORT=: this boots every combination of the machine's features") } out := releaseDir(t) - reps := envInt(t, "REPS", 3) + reps := envInt(t, "REPS", 20) flags := strings.Fields(os.Getenv("SPIN_REPORT_FLAGS")) only, err := regexp.Compile(os.Getenv("SPIN_REPORT_ONLY")) if err != nil { @@ -119,9 +118,6 @@ func TestReport(t *testing.T) { } else { _ = f.Close() } - if _, err := os.Stat(filepath.Join(out, "bin/qemu-system-x86_64-tcg")); err != nil { - missing["accel=tcg"] = "no TCG build in the release" - } // One boot nobody measures. The first after the release was written or the host started // reads the base image from disk and not from the page cache, and would be the slowest @@ -161,9 +157,6 @@ func TestReport(t *testing.T) { v := variant{label: row.ID, cpus: "2", memory: "2048", files: login, flags: rowFlags} - if row.Features["accel"] == "tcg" { - v.timeout = 5 * time.Minute - } row.Boot = measure(t, out, v, reps) r.Specs = append(r.Specs, row) } diff --git a/docs/releasing.md b/docs/releasing.md index 90561f1..56c62bd 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -30,14 +30,17 @@ combination of the machine's features, booted from the published tarball to a lo no API here to promise compatibility about, and the one thing a version could promise — that checkpoints still resume — is decided by the fingerprint of the artefacts, not by a number anybody chose. Pushing a `v*` tag releases that version; running the workflow by -hand with no input generates the next sequence for today, tags the commit, and puts the -three checksums in the release notes. +hand with no input generates the next sequence for today, tags the commit, and writes the +notes: what changed since the release before (`hack/releasenotes`: the commits that reached +the machine by the part they changed, the pins that moved, the kernel options turned on, off +or changed, and the patches added, changed or removed), whether checkpoints carry over +(`hack/fingerprint-diff`), and the checksums. ## The feature matrix A release also says what it costs. After it publishes, the `report` job boots the tarball it just published, on a self-hosted runner labelled `kvm`, through every combination of the -machine's features (accelerator; memory fixed or with a virtio-mem ceiling; vsock; a hotplug +machine's features under KVM (memory fixed or with a virtio-mem ceiling; vsock; a hotplug controller) and every boot variant of the image. It writes `report.json` beside the tarball: per row, the command line, the shape and fingerprint, whether QEMU ran it, and p50/p95 of each boot phase. It then appends `spin-machine compare` against the newest earlier release that has a report to the @@ -53,9 +56,11 @@ task report OUT=exp.json FLAGS='--append mitigations=off' # or --kernel /path/ _output/bin/spin-machine compare --old base.json --new exp.json ``` -`ONLY=` runs the rows whose id matches, and `REPS=` sets the boots per row (default 3, -after one unmeasured boot that warms the page cache). Times compare only between reports taken -on the same kind of host; `compare` says so when they were not. +`ONLY=` runs the rows whose id matches, and `REPS=` sets the boots per row (default 20, +after one unmeasured boot that warms the page cache). Twenty, because at three the comparison +of v20261001.01 with v20260930.02 flagged every KVM row 5-11% slower, and twenty boots of each, +alternated on one host, put them within 5 ms of each other (2026-10-01). Times compare only +between reports taken on the same kind of host; `compare` says so when they were not. `report.yml` is the job, and it runs by hand too: `gh workflow run report.yml -f version=` measures any published release and keeps `report.json` and the comparison as diff --git a/hack/release b/hack/release index c3194e3..37c8426 100755 --- a/hack/release +++ b/hack/release @@ -80,8 +80,11 @@ echo "Firmware:" for f in bios.bin bios-256k.bin pvh.bin kvmvapic.bin efi-virtio.rom qboot.bin; do require "${OUTPUT_DIR}/qemu/${f}" "${SHARE}/qemu/${f}" "firmware ${f}" done -require qemu/qboot/write-pointer.patch "${SHARE}/qemu/qboot-write-pointer.patch" "qboot's patch" -require qemu/qboot/pam.patch "${SHARE}/qemu/qboot-pam.patch" "qboot's patch" +# Every patch in the directory, not a list of them: mtrr.patch was added to the build and not to +# a list here, and v20261001.01 shipped a qboot.bin without the change it was built with. +for p in qemu/qboot/*.patch; do + require "$p" "${SHARE}/qemu/qboot-$(basename "$p")" "qboot's patch $(basename "$p")" +done require qemu/qboot/COPYING "${SHARE}/qemu/qboot-COPYING" "qboot's licence" # The patches QEMU is built with, for the same reason: a changed GPL source is distributed with # its changes. Read from the tree, as qboot's are: the binaries were built from this commit. @@ -156,7 +159,7 @@ QEMU ${QEMU_VERSION} binaries: usr/share/spin-stack/bin/qemu-system-x86_64 usr/share/spin-stack/bin/qemu-system-x86_64-tcg usr/share/spin-stack/bin/qemu-img - firmware: usr/share/spin-stack/qemu/*, except patches/, qboot.bin and the three qboot files below + firmware: usr/share/spin-stack/qemu/*, except patches/, qboot.bin and the qboot files below source: https://download.qemu.org/qemu-${QEMU_VERSION}.tar.xz sha256: ${QEMU_SHA256} built by: qemu/Dockerfile, with usr/share/spin-stack/qemu/patches applied to that source @@ -167,8 +170,8 @@ qboot ${QBOOT_COMMIT} source: https://github.com/bonzini/qboot, commit ${QBOOT_COMMIT}; the sha256 of what \`git archive\` writes for it is ${QBOOT_ARCHIVE_SHA256} licence: GPL-2.0, usr/share/spin-stack/qemu/qboot-COPYING - built by: qemu/Dockerfile's qboot stage, with usr/share/spin-stack/qemu/qboot-write-pointer.patch - and then qboot-pam.patch applied to that source, and the compiler flags in that file + built by: qemu/Dockerfile's qboot stage, with usr/share/spin-stack/qemu/qboot-*.patch applied + to that source in the order that file applies them, and its compiler flags e2fsprogs ${E2FSPROGS_VERSION} binaries: usr/share/spin-stack/bin/mkfs.ext4, debugfs, e2fsck, dumpe2fs diff --git a/hack/releasenotes/main.go b/hack/releasenotes/main.go new file mode 100644 index 0000000..e9b1fd5 --- /dev/null +++ b/hack/releasenotes/main.go @@ -0,0 +1,360 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Command releasenotes says what a release changed against the one before it, in Markdown for +// the release's notes: the commits that reached the machine, grouped by the part they changed; +// the pins that moved; the kernel options that were turned on, off or changed; and the patches +// added, removed or changed. Everything is read from git at the two refs, so it says what is +// in the tree, not what somebody remembered to write down. +// +// The fingerprint verdict (hack/fingerprint-diff) and the boot report's comparison are the +// notes' other two parts; this is the one that says why they moved. +// +// go run ./hack/releasenotes -from v20260930.02 [-to HEAD] +package main + +import ( + "bufio" + "cmp" + "flag" + "fmt" + "io" + "os" + "os/exec" + "path" + "slices" + "strings" +) + +func main() { + from := flag.String("from", "", "the previous release's tag") + to := flag.String("to", "HEAD", "the commit this release is built from") + flag.Parse() + if *from == "" { + fmt.Fprintln(os.Stderr, "releasenotes: -from is required") + os.Exit(2) + } + if err := write(os.Stdout, ".", *from, *to); err != nil { + fmt.Fprintln(os.Stderr, "releasenotes:", err) + os.Exit(1) + } +} + +// parts are what a release is made of, in the order the notes list them, each with the paths +// that build it. A commit is listed under every part it touched. +var parts = []struct { + title string + paths []string +}{ + {"QEMU and firmware", []string{"qemu/"}}, + {"Kernel", []string{"kernel/"}}, + {"Base image", []string{"image/", "e2fsprogs/"}}, + {"The machine's definition and the spin-machine CLI", []string{"machine/", "cmd/"}}, +} + +// patchDirs are where the patches applied to upstream source live; each ships in the release. +var patchDirs = []string{"qemu/patches", "qemu/qboot", "kernel/patches"} + +func write(w io.Writer, repo, from, to string) error { + g := gitAt(repo) + commits, err := g.commits(from, to) + if err != nil { + return err + } + fmt.Fprintf(w, "## What changed since %s\n\n", from) + + var outside []commit + for _, p := range parts { + var in []commit + for _, c := range commits { + if c.touches(p.paths...) { + in = append(in, c) + } + } + if len(in) == 0 { + continue + } + fmt.Fprintf(w, "### %s\n\n", p.title) + for _, c := range in { + fmt.Fprintf(w, "- %s\n", c.subject) + } + fmt.Fprintln(w) + } + for _, c := range commits { + inside := false + for _, p := range parts { + inside = inside || c.touches(p.paths...) + } + if !inside && !c.only("versions.yaml") { + outside = append(outside, c) + } + } + if len(commits) == 0 { + fmt.Fprintf(w, "Nothing: this is the tree %s was built from.\n\n", from) + } + + if err := writeVersions(w, g, from, to); err != nil { + return err + } + if err := writeKernelConfig(w, g, from, to); err != nil { + return err + } + if err := writePatches(w, g, from, to); err != nil { + return err + } + + if len(outside) > 0 { + fmt.Fprintf(w, "
%d more, none of them in the machine: tests, the lab, CI, tooling\n\n", len(outside)) + for _, c := range outside { + fmt.Fprintf(w, "- %s\n", c.subject) + } + fmt.Fprint(w, "\n
\n\n") + } + return nil +} + +// writeVersions lists the pins of versions.yaml whose version moved, appeared or went. +func writeVersions(w io.Writer, g git, from, to string) error { + old, err := g.versions(from) + if err != nil { + return err + } + cur, err := g.versions(to) + if err != nil { + return err + } + var lines []string + for _, name := range sortedKeys(old, cur) { + o, inOld := old[name] + n, inNew := cur[name] + switch { + case !inOld: + lines = append(lines, fmt.Sprintf("- `%s` added at `%s`", name, n)) + case !inNew: + lines = append(lines, fmt.Sprintf("- `%s` removed (was `%s`)", name, o)) + case o != n: + lines = append(lines, fmt.Sprintf("- `%s` `%s` → `%s`", name, o, n)) + } + } + section(w, "Pins (versions.yaml)", lines) + return nil +} + +// writeKernelConfig lists the options the kernel's configuration turned on, off or changed. +func writeKernelConfig(w io.Writer, g git, from, to string) error { + old, err := g.kernelConfig(from) + if err != nil { + return err + } + cur, err := g.kernelConfig(to) + if err != nil { + return err + } + var lines []string + for _, opt := range sortedKeys(old, cur) { + o, n := cmp.Or(old[opt], "n"), cmp.Or(cur[opt], "n") + if o != n { + lines = append(lines, fmt.Sprintf("- `CONFIG_%s` %s → %s", opt, o, n)) + } + } + section(w, "Kernel configuration", lines) + return nil +} + +// writePatches lists the patches to upstream source added, removed or changed, by subject. +func writePatches(w io.Writer, g git, from, to string) error { + var lines []string + for _, dir := range patchDirs { + changes, err := g.run("diff", "--name-status", from, to, "--", dir) + if err != nil { + return err + } + for line := range strings.Lines(changes) { + status, file, ok := strings.Cut(strings.TrimSpace(line), "\t") + if !ok || !strings.HasSuffix(file, ".patch") { + continue + } + // A renamed patch reads "R100\told\tnew": the new name is the one that ships. + if _, after, renamed := strings.Cut(file, "\t"); renamed { + file = after + } + ref, verb := to, "changed" + switch status[0] { + case 'A': + verb = "added" + case 'D': + ref, verb = from, "removed" + } + body, err := g.run("show", ref+":"+file) + if err != nil { + return err + } + about, ok := patchSubject(body) + if !ok { + // A bare diff, as qboot's are, says what it is in the commit that last changed it. + last, err := g.run("log", "-1", "--format=%s", ref, "--", file) + if err != nil { + return err + } + about = strings.TrimSpace(last) + } + lines = append(lines, fmt.Sprintf("- %s `%s`: %s", verb, file, about)) + } + } + section(w, "Patches to upstream source", lines) + return nil +} + +func section(w io.Writer, title string, lines []string) { + if len(lines) == 0 { + return + } + fmt.Fprintf(w, "### %s\n\n%s\n\n", title, strings.Join(lines, "\n")) +} + +// patchSubject is a patch's Subject header without its "[PATCH n/m]", continued lines joined; +// false for a patch without one. +func patchSubject(body string) (string, bool) { + var subject []string + in := false + for line := range strings.Lines(body) { + line = strings.TrimRight(line, "\n") + switch { + case strings.HasPrefix(line, "Subject: "): + subject, in = []string{strings.TrimPrefix(line, "Subject: ")}, true + case in && strings.HasPrefix(line, " "): + subject = append(subject, strings.TrimSpace(line)) + case in: + s := strings.Join(subject, " ") + if strings.HasPrefix(s, "[") { + if _, after, ok := strings.Cut(s, "] "); ok { + s = after + } + } + return s, true + } + } + return "", false +} + +type commit struct { + subject string + files []string +} + +func (c commit) touches(prefixes ...string) bool { + return slices.ContainsFunc(c.files, func(f string) bool { + return slices.ContainsFunc(prefixes, func(p string) bool { return strings.HasPrefix(f, p) }) + }) +} + +func (c commit) only(file string) bool { + return len(c.files) > 0 && !slices.ContainsFunc(c.files, func(f string) bool { return f != file }) +} + +type git struct{ dir string } + +func gitAt(dir string) git { return git{dir} } + +func (g git) run(args ...string) (string, error) { + cmd := exec.Command("git", args...) + cmd.Dir = g.dir + out, err := cmd.Output() + if err != nil { + var stderr string + if ee, ok := err.(*exec.ExitError); ok { + stderr = strings.TrimSpace(string(ee.Stderr)) + } + return "", fmt.Errorf("git %s: %w: %s", strings.Join(args, " "), err, stderr) + } + return string(out), nil +} + +// commits are the commits in to and not in from, oldest first, each with the files it changed. +func (g git) commits(from, to string) ([]commit, error) { + out, err := g.run("log", "--reverse", "--no-merges", "--format=%x00%s", "--name-only", from+".."+to) + if err != nil { + return nil, err + } + var cs []commit + for _, rec := range strings.Split(out, "\x00")[1:] { + lines := strings.Split(strings.TrimSpace(rec), "\n") + c := commit{subject: lines[0]} + for _, f := range lines[1:] { + if f = strings.TrimSpace(f); f != "" { + c.files = append(c.files, f) + } + } + cs = append(cs, c) + } + return cs, nil +} + +// versions are versions.yaml's entries at ref, by name; none where the file is not there. +func (g git) versions(ref string) (map[string]string, error) { + out := map[string]string{} + body, err := g.run("show", ref+":versions.yaml") + if err != nil { + return out, nil //nolint:nilerr // a tree from before versions.yaml pins nothing to compare + } + name := "" + sc := bufio.NewScanner(strings.NewReader(body)) + for sc.Scan() { + line := sc.Text() + if v, ok := strings.CutPrefix(line, "- name: "); ok { + name = strings.TrimSpace(v) + } else if v, ok := strings.CutPrefix(line, " version: "); ok && name != "" { + out[name] = strings.Trim(strings.TrimSpace(v), `"'`) + } + } + return out, sc.Err() +} + +// kernelConfig is the kernel configuration the tree builds at ref, option by option without its +// CONFIG_ prefix: its value, or "n" for one written as not set. +func (g git) kernelConfig(ref string) (map[string]string, error) { + names, err := g.run("ls-tree", "--name-only", ref, "kernel/") + if err != nil { + return nil, err + } + file := "" + for n := range strings.Lines(names) { + if n = strings.TrimSpace(n); strings.HasPrefix(path.Base(n), "config-") { + file = n + } + } + out := map[string]string{} + if file == "" { + return out, nil + } + body, err := g.run("show", ref+":"+file) + if err != nil { + return nil, err + } + for line := range strings.Lines(body) { + line = strings.TrimSpace(line) + if opt, ok := strings.CutPrefix(line, "# CONFIG_"); ok { + if o, ok := strings.CutSuffix(opt, " is not set"); ok { + out[o] = "n" + } + } else if opt, ok := strings.CutPrefix(line, "CONFIG_"); ok { + if k, v, ok := strings.Cut(opt, "="); ok { + out[k] = v + } + } + } + return out, nil +} + +func sortedKeys(maps ...map[string]string) []string { + var keys []string + for _, m := range maps { + for k := range m { + if !slices.Contains(keys, k) { + keys = append(keys, k) + } + } + } + slices.Sort(keys) + return keys +} + diff --git a/hack/releasenotes/main_test.go b/hack/releasenotes/main_test.go new file mode 100644 index 0000000..10b57df --- /dev/null +++ b/hack/releasenotes/main_test.go @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// repo is a git repository in a temporary directory, with commit and tag at hand. +type repo struct { + t *testing.T + dir string +} + +func newRepo(t *testing.T) *repo { + t.Helper() + r := &repo{t, t.TempDir()} + r.git("init", "-q", "-b", "main") + r.git("config", "user.email", "test@example.com") + r.git("config", "user.name", "test") + return r +} + +func (r *repo) git(args ...string) { + r.t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = r.dir + if out, err := cmd.CombinedOutput(); err != nil { + r.t.Fatalf("git %v: %v\n%s", args, err, out) + } +} + +// commit writes files (a nil body removes one) and commits them under subject. +func (r *repo) commit(subject string, files map[string]*string) { + r.t.Helper() + for name, body := range files { + p := filepath.Join(r.dir, name) + if body == nil { + r.git("rm", "-q", name) + continue + } + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + r.t.Fatal(err) + } + if err := os.WriteFile(p, []byte(*body), 0o644); err != nil { + r.t.Fatal(err) + } + r.git("add", name) + } + r.git("commit", "-q", "-m", subject) +} + +func s(v string) *string { return &v } + +const versionsAt = `# pins +- name: qemu + kind: download + version: %s +- name: alpine + kind: image + version: "3.22" +` + +// The notes say, for one release against the one before it, every way the tree that builds the +// machine changed: the commits by the part they touched, a pin that moved, a kernel option turned +// on, off or changed, and a patch added, changed or removed - by its Subject, or by its commit +// where it has none. What did not reach the machine is listed apart. +func TestTheNotesSayWhatChangedInTheMachine(t *testing.T) { + r := newRepo(t) + r.commit("the first release", map[string]*string{ + "versions.yaml": s(strings.Replace(versionsAt, "%s", "11.1.0", 1)), + "kernel/config-7.3-x86_64": s("CONFIG_DEVMEM=y\n# CONFIG_BPF_LSM is not set\nCONFIG_HZ=100\n"), + "qemu/patches/0001-old.patch": s("Subject: [PATCH] vl: the old change\n\ndiff\n"), + "kernel/patches/0001-keep.patch": s("Subject: [PATCH 1/2] keep: this one\n stays\n\ndiff\n"), + "qemu/qboot/pam.patch": s("diff --git a/x b/x\n"), + "boot/report_test.go": s("package boot\n"), + "image/mkosi.extra/etc/fstrim.conf": s("weekly\n"), + "kernel/patches/0002-changes.patch": s("Subject: [PATCH 2/2] changes: before\n\ndiff\n"), + ".github/workflows/release.yml": s("on: push\n"), + "cmd/spin-machine/main.go": s("package main\n"), + }) + r.git("tag", "v1") + + r.commit("qboot: enable the MTRRs (#75)", map[string]*string{"qemu/qboot/mtrr.patch": s("diff --git a/main.c b/main.c\n")}) + r.commit("kernel: lockdown and the BPF LSM", map[string]*string{ + "kernel/config-7.3-x86_64": s("# CONFIG_DEVMEM is not set\nCONFIG_BPF_LSM=y\nCONFIG_HZ=1000\nCONFIG_LSM=\"lockdown,bpf\"\n"), + }) + r.commit("kernel: a patch changed, another dropped", map[string]*string{ + "kernel/patches/0002-changes.patch": s("Subject: [PATCH 2/2] changes: after\n\ndiff\n"), + "qemu/patches/0001-old.patch": nil, + }) + r.commit("image: fstrim hourly (#76)", map[string]*string{"image/mkosi.extra/etc/fstrim.conf": s("hourly\n")}) + r.commit("qemu: bump to 11.1.1", map[string]*string{"versions.yaml": s(strings.Replace(versionsAt, "%s", "11.1.1", 1))}) + r.commit("boot: a probe (#77)", map[string]*string{"boot/report_test.go": s("package boot // more\n")}) + r.commit("spin-machine: a flag and a lab step", map[string]*string{ + "cmd/spin-machine/main.go": s("package main // flag\n"), + ".github/workflows/release.yml": s("on: workflow_dispatch\n"), + }) + + var b strings.Builder + if err := write(&b, r.dir, "v1", "HEAD"); err != nil { + t.Fatal(err) + } + notes := b.String() + t.Log(notes) + + // Each wanted line, and the heading it must be under. + for _, want := range []struct{ under, line string }{ + {"### QEMU and firmware", "- qboot: enable the MTRRs (#75)"}, + {"### Kernel\n", "- kernel: lockdown and the BPF LSM"}, + {"### Kernel\n", "- kernel: a patch changed, another dropped"}, + {"### Base image", "- image: fstrim hourly (#76)"}, + {"### The machine's definition and the spin-machine CLI", "- spin-machine: a flag and a lab step"}, + {"### Pins (versions.yaml)", "- `qemu` `11.1.0` → `11.1.1`"}, + {"### Kernel configuration", "- `CONFIG_BPF_LSM` n → y"}, + {"### Kernel configuration", "- `CONFIG_DEVMEM` y → n"}, + {"### Kernel configuration", "- `CONFIG_HZ` 100 → 1000"}, + {"### Kernel configuration", "- `CONFIG_LSM` n → \"lockdown,bpf\""}, + {"### Patches to upstream source", "- added `qemu/qboot/mtrr.patch`: qboot: enable the MTRRs (#75)"}, + {"### Patches to upstream source", "- removed `qemu/patches/0001-old.patch`: vl: the old change"}, + {"### Patches to upstream source", "- changed `kernel/patches/0002-changes.patch`: changes: after"}, + {"
", "- boot: a probe (#77)"}, + } { + head := strings.Index(notes, want.under) + if head < 0 || !strings.Contains(underHeading(notes[head:]), want.line) { + t.Errorf("%q is not under %q", want.line, want.under) + } + } + for _, absent := range []string{"alpine", "keep: this one", "qemu: bump to 11.1.1", "first release", "pam.patch"} { + if strings.Contains(notes, absent) { + t.Errorf("the notes name %q, which did not change in the machine", absent) + } + } + if !strings.Contains(notes, "1 more, none of them in the machine") { + t.Errorf("what did not reach the machine is not counted apart") + } +} + +// underHeading is the text from a heading to the next one. +func underHeading(from string) string { + if next := strings.Index(from[1:], "\n### "); next >= 0 { + return from[:next+1] + } + return from +} From 6ea1a367321f0a9fb4d2a90cddcd31a88da934a2 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 1 Oct 2026 14:34:04 -0300 Subject: [PATCH 2/2] releasenotes: every edit the mutation gate makes is refused main is run's: exit codes and an empty range are tested, a file in a patch directory that is not a patch is left out, and the commit parser no longer has an index that read the same at 1 and 2. Co-Authored-By: Claude Opus 5.5 (1M context) --- hack/releasenotes/main.go | 36 ++++++++++++++++++---------- hack/releasenotes/main_test.go | 43 ++++++++++++++++++++++++++++++---- 2 files changed, 63 insertions(+), 16 deletions(-) diff --git a/hack/releasenotes/main.go b/hack/releasenotes/main.go index e9b1fd5..efb8f03 100644 --- a/hack/releasenotes/main.go +++ b/hack/releasenotes/main.go @@ -26,17 +26,28 @@ import ( ) func main() { - from := flag.String("from", "", "the previous release's tag") - to := flag.String("to", "HEAD", "the commit this release is built from") - flag.Parse() + os.Exit(run(os.Args[1:], ".", os.Stdout, os.Stderr)) // mutate-exempt: os.Args[0] is the program; only a process run by hand sees which of the rest run is given +} + +// run is the command against the repository at dir: 0 with the notes on stdout, 2 for a +// command line it cannot use, 1 when git could not answer. +func run(args []string, dir string, stdout, stderr io.Writer) int { + fs := flag.NewFlagSet("releasenotes", flag.ContinueOnError) + fs.SetOutput(stderr) + from := fs.String("from", "", "the previous release's tag") + to := fs.String("to", "HEAD", "the commit this release is built from") + if err := fs.Parse(args); err != nil { + return 2 + } if *from == "" { - fmt.Fprintln(os.Stderr, "releasenotes: -from is required") - os.Exit(2) + fmt.Fprintln(stderr, "releasenotes: -from is required") + return 2 } - if err := write(os.Stdout, ".", *from, *to); err != nil { - fmt.Fprintln(os.Stderr, "releasenotes:", err) - os.Exit(1) + if err := write(stdout, dir, *from, *to); err != nil { + fmt.Fprintln(stderr, "releasenotes:", err) + return 1 } + return 0 } // parts are what a release is made of, in the order the notes list them, each with the paths @@ -276,10 +287,11 @@ func (g git) commits(from, to string) ([]commit, error) { return nil, err } var cs []commit + // Each record is "\x00subject\n\nfile\nfile...": what is before the first is nothing. for _, rec := range strings.Split(out, "\x00")[1:] { - lines := strings.Split(strings.TrimSpace(rec), "\n") - c := commit{subject: lines[0]} - for _, f := range lines[1:] { + subject, files, _ := strings.Cut(rec, "\n") + c := commit{subject: subject} + for f := range strings.Lines(files) { if f = strings.TrimSpace(f); f != "" { c.files = append(c.files, f) } @@ -302,7 +314,7 @@ func (g git) versions(ref string) (map[string]string, error) { line := sc.Text() if v, ok := strings.CutPrefix(line, "- name: "); ok { name = strings.TrimSpace(v) - } else if v, ok := strings.CutPrefix(line, " version: "); ok && name != "" { + } else if v, ok := strings.CutPrefix(line, " version: "); ok { out[name] = strings.Trim(strings.TrimSpace(v), `"'`) } } diff --git a/hack/releasenotes/main_test.go b/hack/releasenotes/main_test.go index 10b57df..c095e83 100644 --- a/hack/releasenotes/main_test.go +++ b/hack/releasenotes/main_test.go @@ -77,6 +77,7 @@ func TestTheNotesSayWhatChangedInTheMachine(t *testing.T) { "qemu/patches/0001-old.patch": s("Subject: [PATCH] vl: the old change\n\ndiff\n"), "kernel/patches/0001-keep.patch": s("Subject: [PATCH 1/2] keep: this one\n stays\n\ndiff\n"), "qemu/qboot/pam.patch": s("diff --git a/x b/x\n"), + "qemu/qboot/README.md": s("what the patches are\n"), "boot/report_test.go": s("package boot\n"), "image/mkosi.extra/etc/fstrim.conf": s("weekly\n"), "kernel/patches/0002-changes.patch": s("Subject: [PATCH 2/2] changes: before\n\ndiff\n"), @@ -95,15 +96,16 @@ func TestTheNotesSayWhatChangedInTheMachine(t *testing.T) { }) r.commit("image: fstrim hourly (#76)", map[string]*string{"image/mkosi.extra/etc/fstrim.conf": s("hourly\n")}) r.commit("qemu: bump to 11.1.1", map[string]*string{"versions.yaml": s(strings.Replace(versionsAt, "%s", "11.1.1", 1))}) + r.commit("qboot: say what the patches are", map[string]*string{"qemu/qboot/README.md": s("each patch, and why\n")}) r.commit("boot: a probe (#77)", map[string]*string{"boot/report_test.go": s("package boot // more\n")}) r.commit("spin-machine: a flag and a lab step", map[string]*string{ "cmd/spin-machine/main.go": s("package main // flag\n"), ".github/workflows/release.yml": s("on: workflow_dispatch\n"), }) - var b strings.Builder - if err := write(&b, r.dir, "v1", "HEAD"); err != nil { - t.Fatal(err) + var b, stderr strings.Builder + if code := run([]string{"-from", "v1"}, r.dir, &b, &stderr); code != 0 { + t.Fatalf("exit %d: %s", code, stderr.String()) } notes := b.String() t.Log(notes) @@ -130,7 +132,7 @@ func TestTheNotesSayWhatChangedInTheMachine(t *testing.T) { t.Errorf("%q is not under %q", want.line, want.under) } } - for _, absent := range []string{"alpine", "keep: this one", "qemu: bump to 11.1.1", "first release", "pam.patch"} { + for _, absent := range []string{"alpine", "keep: this one", "qemu: bump to 11.1.1", "first release", "pam.patch", "README.md"} { if strings.Contains(notes, absent) { t.Errorf("the notes name %q, which did not change in the machine", absent) } @@ -147,3 +149,36 @@ func underHeading(from string) string { } return from } + +// What the command answers, and with what exit, for a command line it cannot use, a ref git does +// not know, and a release built from the same tree as the one before it. +func TestTheCommandSaysWhyItWroteNothing(t *testing.T) { + r := newRepo(t) + r.commit("the first release", map[string]*string{"versions.yaml": s("- name: qemu\n version: 11.1.1\n")}) + r.git("tag", "v1") + for _, tc := range []struct { + name string + args []string + code int + stdout string + stderr string + }{ + {"no previous release", nil, 2, "", "-from is required"}, + {"a flag it does not take", []string{"-since", "v1"}, 2, "", "flag provided but not defined"}, + {"a ref git does not know", []string{"-from", "v0"}, 1, "", "git log"}, + {"the same tree", []string{"-from", "v1"}, 0, "Nothing: this is the tree v1 was built from.", ""}, + } { + t.Run(tc.name, func(t *testing.T) { + var stdout, stderr strings.Builder + if code := run(tc.args, r.dir, &stdout, &stderr); code != tc.code { + t.Errorf("exit %d, want %d; stderr: %s", code, tc.code, stderr.String()) + } + if !strings.Contains(stdout.String(), tc.stdout) || (tc.stdout == "" && stdout.Len() > 0) { + t.Errorf("stdout %q, want %q", stdout.String(), tc.stdout) + } + if !strings.Contains(stderr.String(), tc.stderr) || (tc.stderr == "" && stderr.Len() > 0) { + t.Errorf("stderr %q, want %q", stderr.String(), tc.stderr) + } + }) + } +}