Recommendation relevance, the rollback unit, and a version that names what shipped #131
+19
-5
@@ -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/
|
||||
|
||||
@@ -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
|
||||
|
||||
Binary file not shown.
+55
-2
@@ -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=<int> ordering key, minutes since 2020-01-01 at BUILD time
|
||||
# tag=v<name> 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)}"
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user