diff --git a/.dockerignore b/.dockerignore index 938a46c7..0ab4ab54 100644 --- a/.dockerignore +++ b/.dockerignore @@ -6,9 +6,20 @@ **/build web/build -# Flutter mobile client — built separately on developer machines / Flutter CI. -# Including it in the Go build context wastes ~70 files and invalidates the -# `COPY . .` layer cache on every Flutter-only change. +# The Android client — built by its own job, never from this context. The APK +# reaches the image through client/, downloaded as a CI artifact, so nothing +# here reads android/ sources. +# +# This block named `flutter_client/` until 2026-09-10 and lost its PATTERN when +# that tree was deleted, leaving a comment describing an exclusion that was no +# longer happening. android/ never took its place, so 4.1 MB of Gradle project +# has been entering the context and busting the `COPY . .` layer on every +# Android-only change. +android/ + +# Local `make build` output — an 18 MB binary the image never uses, since the +# builder stage compiles its own. +bin/ # Docs and IDE noise docs/ @@ -26,5 +37,8 @@ docs/ !.env.example # CI workflow files don't need to ship in the image. -.forgejo/ -.github/ +# +# This said `.forgejo/` and `.github/` — neither of which this repo has. Gitea +# Actions reads `.gitea/`, so the one directory that actually exists was the +# one not excluded, and every workflow edit invalidated the context. +.gitea/ diff --git a/.gitignore b/.gitignore index f1619185..11b676e2 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,11 @@ # Test binary, built with `go test -c` *.test +# `make build` output. bin/minstrel was tracked until 2026-09-10 — an 18 MB +# binary committed by accident, last refreshed by a commit about web test +# mocks, and re-dirtied by every local build since. +bin/ + # Bundled Android APK + version sidecar (#397). Populated by CI for # tag releases; never committed. README in client/ explains the flow. client/minstrel.apk diff --git a/bin/minstrel b/bin/minstrel deleted file mode 100755 index 9b34c685..00000000 Binary files a/bin/minstrel and /dev/null differ diff --git a/ci/version.sh b/ci/version.sh index 72d115df..fb8774ef 100755 --- a/ci/version.sh +++ b/ci/version.sh @@ -2,7 +2,8 @@ # # Derives the three values a build is stamped with, and the tag that names it. # -# name=YYYY.MM.DD.HHMM label for people, from the COMMIT's timestamp +# name=YYYY.MM.DD.HHMM label for people, from the timestamp of the newest +# commit that CHANGED SOMETHING SHIPPED (see SHIPPED) # code= ordering key, minutes since 2020-01-01 at BUILD time # tag=v what a release of this commit must be called # @@ -33,9 +34,61 @@ readonly REF="${1:-HEAD}" # Both clocks are overridable so a test can pin them. Nothing but tests should # set these — the defaults are the real derivation. +# The paths that do NOT ship, in either artifact. Everything else counts. +# +# A DENYLIST, and the direction is the whole point. As an allowlist, the list +# has to be updated by whoever adds a directory and nothing fails if they +# don't — so the failure mode is a changed artifact keeping its old version, +# silently, on a green run. That is a build lying about what it is. Inverted, +# new content counts by default and the only way to wrongly EXCLUDE something +# is to name it here deliberately. +# +# The two error directions are not symmetric, which is why this is not taste: +# wrongly excluded → changed artifact, unchanged version. A silent lie. +# wrongly included → version moves when nothing shipped. Cosmetic noise in +# a string nobody sorts. +# +# THIS REPO SHIPS TWO ARTIFACTS FROM ONE DERIVATION, and that is why the list +# is shorter than it looks like it should be. The server image ships cmd/, +# internal/, shared/, web/, config.example.yaml and client/; the APK ships +# android/. Neither ships the other's sources — but excluding android/ here +# would stop an Android-only commit from moving the APK's OWN version, which +# is the dangerous direction. So this is the union: exclude only what ships in +# NEITHER, and accept that an Android commit also nudges the server's reported +# version. Over-inclusion across the two, which is the harmless direction. +# +# The family's other repos (roundtable / roundtable-android) each keep a +# tighter list because they are separate repos with one artifact apiece. Do +# not copy theirs onto this one. +readonly SHIPPED=( + . + ':!.gitea' # CI workflows — including this script's own caller + ':!ci' # CI scripts — including this script + ':!docs' + ':!tools' # asset/font generators; their OUTPUT ships, they do not + ':!deploy' # test-database bootstrap SQL + ':!bin' # local `make build` output + ':!*.md' + ':!Makefile' + ':!docker-compose.yml' + ':!.env.example' + ':!.gitignore' + ':!.dockerignore' + ':!renovate.json' + ':!.golangci.yml' +) + commit_epoch="${MINSTREL_COMMIT_EPOCH:-}" if [ -z "${commit_epoch}" ]; then - commit_epoch="$(git log --format=%ct -1 "${REF}")" + commit_epoch="$(git log --format=%ct -1 "${REF}" -- "${SHIPPED[@]}")" + # Loudly, on purpose. A silent fallback here is the landmine this whole + # script exists to avoid: a plausible-looking version that is quietly wrong, + # on a green run. Realistically this means a shallow clone (no commit in + # range touches the shipped set) rather than a repo of pure CI config. + if [ -z "${commit_epoch}" ]; then + echo "version.sh: no commit under '${REF}' touches the shipped file set — shallow clone? (needs fetch-depth: 0)" >&2 + exit 1 + fi fi now_epoch="${MINSTREL_NOW_EPOCH:-$(date -u +%s)}" diff --git a/internal/server/release_version_test.go b/internal/server/release_version_test.go index e27aafa0..1846adca 100644 --- a/internal/server/release_version_test.go +++ b/internal/server/release_version_test.go @@ -468,3 +468,134 @@ func TestImageBuild_StampsADerivedVersionAndAChannel(t *testing.T) { t.Error("the image job no longer derives its version from ci/version.sh — the version and the APK's version can now drift apart") } } + +// gitRepo builds a throwaway repo and returns its path. Commit timestamps are +// pinned so the derivation is deterministic. +func gitRepo(t *testing.T) string { + t.Helper() + dir := t.TempDir() + run := func(args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + run("-c", "init.defaultBranch=main", "init", "-q") + run("config", "user.email", "t@example.invalid") + run("config", "user.name", "t") + return dir +} + +// commitFile writes path and commits it with a pinned committer timestamp. +func commitFile(t *testing.T, dir, path, epoch string) { + t.Helper() + full := filepath.Join(dir, path) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte("x\n"), 0o644); err != nil { + t.Fatal(err) + } + for _, args := range [][]string{{"add", "-A"}, {"commit", "-q", "-m", path}} { + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null", + "GIT_AUTHOR_DATE=@"+epoch+" +0000", + "GIT_COMMITTER_DATE=@"+epoch+" +0000", + ) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } +} + +// versionIn runs ci/version.sh inside dir, reading real git rather than the +// pinned-clock override, so the PATHSPEC is what is under test. +func versionIn(t *testing.T, dir string) (string, error) { + t.Helper() + cmd := exec.Command(filepath.Join(repoRoot(t), "ci", "version.sh"), "HEAD") + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null", + "MINSTREL_NOW_EPOCH=1789000920", + ) + out, err := cmd.CombinedOutput() + return string(out), err +} + +// The version names what SHIPPED, so a commit that changes nothing shippable +// must not move it. +// +// The failure this prevents is not the cosmetic one. The derivation is a +// denylist precisely so that new content counts by default: the direction that +// matters is a changed artifact keeping its OLD version, silently, on a green +// run. This test pins the cheap half of that (CI-only commits are inert) and, +// in the same breath, that a source commit still moves it — because a pathspec +// typo that excluded everything would satisfy the first assertion alone. +func TestVersionName_IgnoresCommitsThatShipNothing(t *testing.T) { + const ( + shipped = "1757443736" // 2025-09-09T18:48:56Z + ciOnly = "1789000920" // 2026-09-10T00:42:00Z, later + ) + dir := gitRepo(t) + commitFile(t, dir, "internal/server/thing.go", shipped) + commitFile(t, dir, ".gitea/workflows/release.yml", ciOnly) + + out, err := versionIn(t, dir) + if err != nil { + t.Fatalf("version.sh failed: %v\n%s", err, out) + } + if !strings.Contains(out, "name=2025.09.09.1848") { + t.Errorf("a CI-only commit moved the version — the pathspec is not excluding it\n%s", out) + } + + // ...and the pathspec must not be so broad it excludes everything. + commitFile(t, dir, "internal/server/other.go", ciOnly) + out, err = versionIn(t, dir) + if err != nil { + t.Fatalf("version.sh failed: %v\n%s", err, out) + } + if !strings.Contains(out, "name=2026.09.10.0042") { + t.Errorf("a source commit did NOT move the version — the pathspec excludes too much, which is the silent-lie direction\n%s", out) + } +} + +// android/ is deliberately NOT excluded, and that is the subtle half of the +// list. It ships in no server image — but it is the APK's entire source, and +// ONE script derives the version for both artifacts. Excluding it would stop +// an Android-only commit from moving the APK's own version, which is exactly +// the silent downgrade the versioning rework exists to prevent. +func TestVersionName_AndroidSourcesCount(t *testing.T) { + dir := gitRepo(t) + commitFile(t, dir, "internal/server/thing.go", "1757443736") + commitFile(t, dir, "android/app/src/main/Thing.kt", "1789000920") + + out, err := versionIn(t, dir) + if err != nil { + t.Fatalf("version.sh failed: %v\n%s", err, out) + } + if !strings.Contains(out, "name=2026.09.10.0042") { + t.Errorf("an Android commit did not move the version; the APK would ship new code under its old version name\n%s", out) + } +} + +// No shipped commit in range means a shallow clone, and the script must refuse +// rather than emit something plausible. A wrong version builds, signs and +// publishes perfectly happily; it surfaces later as an update channel that has +// quietly stopped offering anything. +func TestVersionScript_RefusesWhenNothingShippedIsInRange(t *testing.T) { + dir := gitRepo(t) + commitFile(t, dir, "ci/version.sh", "1789000920") + + out, err := versionIn(t, dir) + if err == nil { + t.Fatalf("script succeeded with no shipped commit in range; it should refuse\n%s", out) + } + if strings.Contains(out, "name=") { + t.Errorf("script emitted a version name while refusing — that value could still be consumed\n%s", out) + } +}