From e6d0789f2ef8b89c959a8a0299318b6eb8c39847 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Tue, 28 Jul 2026 20:25:15 +0200 Subject: [PATCH] build: ask the SemVer gate about the pins, not about git history The gate diffs changed .lua files between two Git revisions of this repository. That works only while the drivers are committed here, and it was never quite the right question: what reaches a gateway is decided by where drivers/BUNDLED_SOURCE.json points, not by what this history shows. check-versions gains -old-dir/-new-dir, comparing two materialised snapshots instead of two revisions. ValidateDriverVersionChange is untouched -- it always took bytes rather than revisions, so the rule being enforced is identical. scripts/check-driver-versions.sh reads the pin at a base revision and the pin now, and stays quiet when they match: a pull request that does not move the pin cannot have moved any driver's bytes. When it has moved, both snapshots are fetched and every bundled driver compared. The Git-based mode stays for now. It still works while the files are committed, and removing it belongs with the deletion. Tested where it counts -- the failure case. Six table-driven cases in main_test.go: changed bytes under an unchanged version fail and name the driver, a bump passes, untouched files pass, a newly bundled driver has nothing to compare, a dropped one is a coverage decision rather than a version one, and every offender is reported rather than just the first. Also run against the real pin move in #704 (1f67078 -> 3f62a00), where pixii, solaredge_legacy and sungrow all changed with bumps: verified across 37 bundled drivers. Co-Authored-By: Claude Opus 5 Signed-off-by: Fredrik Ahlgren --- .../check-driver-versions-across-pins.md | 5 + .github/workflows/test.yml | 7 ++ Makefile | 11 +- README.md | 6 + go/cmd/ftw-driver-repository/main.go | 59 +++++++++ go/cmd/ftw-driver-repository/main_test.go | 118 ++++++++++++++++++ scripts/check-driver-versions.sh | 77 ++++++++++++ 7 files changed, 281 insertions(+), 2 deletions(-) create mode 100644 .changeset/check-driver-versions-across-pins.md create mode 100644 go/cmd/ftw-driver-repository/main_test.go create mode 100755 scripts/check-driver-versions.sh diff --git a/.changeset/check-driver-versions-across-pins.md b/.changeset/check-driver-versions-across-pins.md new file mode 100644 index 00000000..6de8f34d --- /dev/null +++ b/.changeset/check-driver-versions-across-pins.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +Require a SemVer bump from any bundled driver whose bytes change between the old pin and the new one, asked of the pinned snapshots rather than of this repository's Git history. diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9d3b7642..52c6715b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -248,6 +248,13 @@ jobs: go run ./cmd/ftw-driver-repository check-versions -repo-root .. -base "${{ needs.changes.outputs.base_sha }}" + + # The check above diffs this repository's history. What reaches a + # gateway is decided by where the pin points, so this asks the same + # question of the pins -- and keeps asking it once the .lua files stop + # being committed here. Quiet on a pull request that does not move it. + - name: Require version bumps across the pin + run: bash scripts/check-driver-versions.sh "${{ needs.changes.outputs.base_sha }}" -head "${{ needs.changes.outputs.head_sha }}" device-support-contract: diff --git a/Makefile b/Makefile index e1e8d87f..6407fbda 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ .PHONY: help test optimizer-install optimizer-test compose-migration-test container-boundary-test build build-arm64 build-amd64 build-windows-amd64 release \ run-sim dev fmt vet clean e2e ci ci-ui ci-hw-pi docs \ verify verify-all install-hooks driver-repository-validate driver-versions \ - drivers drivers-present + drivers drivers-present driver-versions-across-pin VERSION ?= $(shell git describe --tags --always --dirty 2>/dev/null || echo dev) LDFLAGS := -s -w -X main.Version=$(VERSION) @@ -39,6 +39,7 @@ help: @echo " install-hooks install git pre-commit + pre-push hooks (opt-in)" @echo " driver-repository-validate build and validate unsigned driver release artifacts" @echo " driver-versions require changed Lua drivers to increase SemVer" + @echo " driver-versions-across-pin same rule, asked of the pinned snapshots" @echo " ci run local CI incl. browser smoke" @echo " ci-ui browser smoke against FTW_BASE_URL" @echo " ci-hw-pi deploy candidate to Pi CI slot + browser smoke" @@ -86,7 +87,7 @@ optimizer-test: optimizer/.venv/.installed optimizer/.venv/bin/pytest -q optimizer/tests compose-migration-test: - bash -n scripts/enable-modular-stack.sh scripts/migrate-legacy-compose.sh scripts/install-macos.sh scripts/deploy-home-link-web.sh scripts/sync-bundled-drivers.sh + bash -n scripts/enable-modular-stack.sh scripts/migrate-legacy-compose.sh scripts/install-macos.sh scripts/deploy-home-link-web.sh scripts/sync-bundled-drivers.sh scripts/check-driver-versions.sh bash scripts/test-modular-compose.sh container-boundary-test: @@ -105,6 +106,12 @@ DRIVER_BASE ?= origin/master driver-versions: cd go && go run ./cmd/ftw-driver-repository check-versions -repo-root .. -base $(DRIVER_BASE) -head WORKTREE +# The same rule asked of the pins rather than of this repository's history: +# a bundled driver whose bytes moved between the old pin and the new one must +# have moved its version too. Quiet when the pin has not moved. +driver-versions-across-pin: + bash scripts/check-driver-versions.sh $(DRIVER_BASE) + ci: ./scripts/ci-local.sh diff --git a/README.md b/README.md index e812b511..7ecd35e4 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,12 @@ throwing the committed copies away and rebuilding from the pin alone. That is the groundwork for dropping them from git entirely, so this repository holds no driver source at all. +Moving the pin is a driver release. `make driver-versions-across-pin` compares +the drivers at the old pin against the drivers at the new one and fails when a +file changed without its `DRIVER` version moving — the signed channel refuses to +publish changed bytes under a version it already published, and a gateway +offered the same version twice cannot tell them apart. + ## Install on Linux The installer supports Raspberry Pi OS, Debian and Ubuntu: diff --git a/go/cmd/ftw-driver-repository/main.go b/go/cmd/ftw-driver-repository/main.go index cfa361fe..99419a36 100644 --- a/go/cmd/ftw-driver-repository/main.go +++ b/go/cmd/ftw-driver-repository/main.go @@ -154,14 +154,73 @@ func keygen(args []string) error { return nil } +// checkVersionsAcrossDirs compares two materialised driver snapshots rather +// than two Git revisions. +// +// The bundled drivers are generated from srcfl/device-drivers at a pinned +// commit, so what actually reaches a gateway is decided by where that pin +// points, not by what this repository's history happens to show. Diffing the +// pins asks the question that matters -- did a driver's bytes change without +// its version moving -- and keeps asking it once the files stop being +// committed here at all. +// +// A driver present in new but not in old is newly bundled: nothing to compare, +// and ValidateDriverVersionChange treats an empty previous as fine. One present +// in old but not in new was dropped from the pin, which is not a version +// question. +func checkVersionsAcrossDirs(oldDir, newDir string) error { + entries, err := os.ReadDir(newDir) + if err != nil { + return fmt.Errorf("read new snapshot: %w", err) + } + var failures []error + checked := 0 + for _, entry := range entries { + name := entry.Name() + if entry.IsDir() || filepath.Ext(name) != ".lua" { + continue + } + current, err := os.ReadFile(filepath.Join(newDir, name)) + if err != nil { + failures = append(failures, fmt.Errorf("%s: read new snapshot: %w", name, err)) + continue + } + previous, err := os.ReadFile(filepath.Join(oldDir, name)) + if err != nil { + if !os.IsNotExist(err) { + failures = append(failures, fmt.Errorf("%s: read old snapshot: %w", name, err)) + continue + } + previous = nil // newly bundled + } + checked++ + if err := driverrepo.ValidateDriverVersionChange(name, previous, current); err != nil { + failures = append(failures, err) + } + } + if err := errors.Join(failures...); err != nil { + return err + } + fmt.Printf("driver version changes verified across %d bundled drivers\n", checked) + return nil +} + func checkVersions(args []string) error { fs := flag.NewFlagSet("check-versions", flag.ContinueOnError) repoRoot := fs.String("repo-root", "..", "Git repository root") base := fs.String("base", "", "base Git revision") head := fs.String("head", "HEAD", "head Git revision or WORKTREE") + oldDir := fs.String("old-dir", "", "previous driver snapshot; compares snapshots instead of Git revisions") + newDir := fs.String("new-dir", "", "current driver snapshot; requires -old-dir") if err := fs.Parse(args); err != nil { return err } + if (*oldDir == "") != (*newDir == "") { + return errors.New("-old-dir and -new-dir must be given together") + } + if *oldDir != "" { + return checkVersionsAcrossDirs(*oldDir, *newDir) + } if *base == "" { return errors.New("base revision is required") } diff --git a/go/cmd/ftw-driver-repository/main_test.go b/go/cmd/ftw-driver-repository/main_test.go new file mode 100644 index 00000000..80870df5 --- /dev/null +++ b/go/cmd/ftw-driver-repository/main_test.go @@ -0,0 +1,118 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// driverFile writes a minimal driver whose DRIVER table carries a version and +// whose body can be varied to change the file's bytes. +func driverFile(t *testing.T, dir, name, version, body string) { + t.Helper() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + source := "DRIVER = {\n id = \"" + strings.TrimSuffix(name, ".lua") + "\",\n" + + " version = \"" + version + "\",\n}\n\nfunction driver_poll()\n" + body + "\nend\n" + if err := os.WriteFile(filepath.Join(dir, name), []byte(source), 0o644); err != nil { + t.Fatal(err) + } +} + +func snapshots(t *testing.T) (string, string) { + t.Helper() + root := t.TempDir() + oldDir := filepath.Join(root, "old") + newDir := filepath.Join(root, "new") + if err := os.MkdirAll(oldDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(newDir, 0o755); err != nil { + t.Fatal(err) + } + return oldDir, newDir +} + +// The failure this gate exists for: a driver's bytes move between two pins +// while its version stands still. A gateway offered that driver cannot tell +// the two apart, and the signed channel refuses to publish changed bytes +// under a version it already published. +func TestChangedBytesWithoutAVersionBumpFail(t *testing.T) { + oldDir, newDir := snapshots(t) + driverFile(t, oldDir, "pixii.lua", "2.1.0", " return 5000") + driverFile(t, newDir, "pixii.lua", "2.1.0", " return 1000") + + err := checkVersionsAcrossDirs(oldDir, newDir) + if err == nil { + t.Fatal("changed bytes under an unchanged version were accepted") + } + if !strings.Contains(err.Error(), "pixii.lua") { + t.Errorf("error does not name the driver: %v", err) + } +} + +func TestChangedBytesWithABumpPass(t *testing.T) { + oldDir, newDir := snapshots(t) + driverFile(t, oldDir, "pixii.lua", "2.1.0", " return 5000") + driverFile(t, newDir, "pixii.lua", "2.1.1", " return 1000") + + if err := checkVersionsAcrossDirs(oldDir, newDir); err != nil { + t.Fatalf("a bumped driver was rejected: %v", err) + } +} + +func TestUnchangedDriversPass(t *testing.T) { + oldDir, newDir := snapshots(t) + driverFile(t, oldDir, "pixii.lua", "2.1.0", " return 5000") + driverFile(t, newDir, "pixii.lua", "2.1.0", " return 5000") + + if err := checkVersionsAcrossDirs(oldDir, newDir); err != nil { + t.Fatalf("an untouched driver was rejected: %v", err) + } +} + +// A driver added to the pin's bundle list has no previous bytes to compare +// against. That is not a version question. +func TestNewlyBundledDriverPasses(t *testing.T) { + oldDir, newDir := snapshots(t) + driverFile(t, newDir, "atmoce.lua", "1.0.0", " return 5000") + + if err := checkVersionsAcrossDirs(oldDir, newDir); err != nil { + t.Fatalf("a newly bundled driver was rejected: %v", err) + } +} + +// Dropping a driver from the bundle list is a coverage decision, not a +// version one, so a file present only in the old snapshot is ignored. +func TestDroppedDriverIsIgnored(t *testing.T) { + oldDir, newDir := snapshots(t) + driverFile(t, oldDir, "zap.lua", "1.0.0", " return 5000") + driverFile(t, newDir, "pixii.lua", "2.1.0", " return 5000") + driverFile(t, oldDir, "pixii.lua", "2.1.0", " return 5000") + + if err := checkVersionsAcrossDirs(oldDir, newDir); err != nil { + t.Fatalf("a dropped driver was treated as a failure: %v", err) + } +} + +// Every driver is reported, not just the first bad one, so a pin move that +// missed several bumps is fixed in one pass rather than one per run. +func TestAllOffendersAreReported(t *testing.T) { + oldDir, newDir := snapshots(t) + for _, name := range []string{"pixii.lua", "sungrow.lua", "kostal.lua"} { + driverFile(t, oldDir, name, "1.0.0", " return 5000") + driverFile(t, newDir, name, "1.0.0", " return 1000") + } + + err := checkVersionsAcrossDirs(oldDir, newDir) + if err == nil { + t.Fatal("expected failures") + } + for _, name := range []string{"pixii.lua", "sungrow.lua", "kostal.lua"} { + if !strings.Contains(err.Error(), name) { + t.Errorf("%s missing from the report: %v", name, err) + } + } +} diff --git a/scripts/check-driver-versions.sh b/scripts/check-driver-versions.sh new file mode 100755 index 00000000..e93bc3fa --- /dev/null +++ b/scripts/check-driver-versions.sh @@ -0,0 +1,77 @@ +#!/usr/bin/env bash +# Require a SemVer bump from any bundled driver whose bytes changed. +# +# The drivers here are generated from srcfl/device-drivers at the commit +# pinned in drivers/BUNDLED_SOURCE.json, so what reaches a gateway is decided +# by where that pin points -- not by what this repository's history shows. +# This compares the drivers at the previous pin against the drivers at the +# current one and fails when a file changed without its DRIVER version moving. +# +# Asking the pins rather than the Git log keeps working once the .lua files +# stop being committed here, and is the more honest question even now: it +# measures what ships. +# +# Usage: +# scripts/check-driver-versions.sh +# +# The base revision is only used to read the previous drivers/BUNDLED_SOURCE.json. +# A pull request that does not move the pin has nothing to check. + +set -euo pipefail + +BASE="${1:-}" +[ -n "$BASE" ] || { echo "usage: $0 " >&2; exit 2; } + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +PIN="drivers/BUNDLED_SOURCE.json" + +command -v jq >/dev/null || { echo "jq is required" >&2; exit 2; } + +NEW_PIN_JSON="$(cat "${ROOT}/${PIN}")" +# A pull request that adds the pin for the first time has no previous one. +OLD_PIN_JSON="$(git -C "$ROOT" show "${BASE}:${PIN}" 2>/dev/null || echo '')" + +if [ -z "$OLD_PIN_JSON" ]; then + echo "no ${PIN} at ${BASE}; nothing to compare" + exit 0 +fi + +REPOSITORY="$(printf '%s' "$NEW_PIN_JSON" | jq -r .repository)" +SOURCE_DIR="$(printf '%s' "$NEW_PIN_JSON" | jq -r .source_dir)" +NEW_COMMIT="$(printf '%s' "$NEW_PIN_JSON" | jq -r .commit)" +OLD_COMMIT="$(printf '%s' "$OLD_PIN_JSON" | jq -r .commit)" + +if [ "$OLD_COMMIT" = "$NEW_COMMIT" ]; then + echo "pin unchanged at ${NEW_COMMIT:0:12}; no driver bytes can have moved" + exit 0 +fi + +WORK="$(mktemp -d)" +trap 'rm -rf "${WORK}"' EXIT + +# Materialise one commit's drivers into a flat directory, limited to the +# drivers the current pin actually bundles. A driver added to the list has no +# counterpart in the old snapshot, which reads as "newly bundled". +materialise() { + local commit="$1" dest="$2" + mkdir -p "$dest" + local extract="${WORK}/extract-${commit:0:12}" + mkdir -p "$extract" + curl -fsSL "https://codeload.github.com/${REPOSITORY}/tar.gz/${commit}" \ + | tar -xz -C "$extract" + local src + src="$(find "$extract" -maxdepth 1 -mindepth 1 -type d | head -1)/${SOURCE_DIR}" + [ -d "$src" ] || { echo "no ${SOURCE_DIR} at ${commit}" >&2; exit 1; } + while IFS= read -r id; do + [ -n "$id" ] || continue + [ -f "${src}/${id}.lua" ] && cp "${src}/${id}.lua" "${dest}/${id}.lua" + done < <(printf '%s' "$NEW_PIN_JSON" | jq -r '.drivers[]') +} + +echo "comparing ${REPOSITORY} ${OLD_COMMIT:0:12} -> ${NEW_COMMIT:0:12}" +materialise "$OLD_COMMIT" "${WORK}/old" +materialise "$NEW_COMMIT" "${WORK}/new" + +cd "${ROOT}/go" +go run ./cmd/ftw-driver-repository check-versions \ + -old-dir "${WORK}/old" -new-dir "${WORK}/new"