fix(ci): version derives from the shipped set; untrack an 18MB binary
test-go / test (push) Successful in 1m5s
test-go / integration (push) Successful in 3m30s
release / Build signed APK (releases and dev) (push) Successful in 5m10s
release / Build + push container image (push) Successful in 1m31s
release / Verify release artifacts (tag releases only) (push) Skipped
test-go / test (push) Successful in 1m5s
test-go / integration (push) Successful in 3m30s
release / Build signed APK (releases and dev) (push) Successful in 5m10s
release / Build + push container image (push) Successful in 1m31s
release / Verify release artifacts (tag releases only) (push) Skipped
Three build-hygiene fixes that turned up while explaining the pathspec.
**version.sh derives from what SHIPPED.** It read bare HEAD, so any commit
moved the version — including one touching only CI or a README. Rules 148
and 149 both specify the pathspec form. Now a denylist, and the direction
is the point: as an allowlist the list must 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. Inverted, new
content counts by default.
android/ is deliberately NOT excluded, and that is the subtle part. This
repo ships TWO artifacts from ONE derivation: android/ is in no server
image, but it is the APK's entire source, and excluding it would stop an
Android-only commit from moving the APK's own version — the silent
downgrade the versioning rework exists to prevent. So the list 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. roundtable/roundtable-android each
keep tighter lists because they are one-artifact repos; don't copy theirs.
**.dockerignore excluded the wrong CI directory.** It named .forgejo/ and
.github/, neither of which this repo has. Gitea Actions reads .gitea/, so
the one directory that exists was the one not excluded. The "Flutter mobile
client" block had also lost its PATTERN when flutter_client/ was deleted,
leaving a comment describing an exclusion that was not happening — android/
never took its place, so 4.1MB of Gradle project entered the context and
busted the `COPY . .` layer on every Android-only change. bin/ excluded too.
**bin/minstrel was tracked** — an 18MB binary last refreshed by a commit
about web test mocks, and re-dirtied by every `make build` since. Untracked
and ignored; the file stays on disk.
Guards are behavioural rather than textual: they build throwaway repos with
pinned commit timestamps and run version.sh against them, so they break when
the derivation changes rather than when the wording does. Falsified — drop
the .gitea exclusion and the CI-only commit moves the version; add an
android exclusion and an Android commit stops moving it; exclude everything
and a source commit refuses.
One honest note on the refusal test: the script already refused an empty
result via the downstream date check, so the new explicit check improves the
diagnostic ("no commit touches the shipped file set — shallow clone?") and
not the safety. The test pins the property, which is defended in depth.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
This commit is contained in:
+19
-5
@@ -6,9 +6,20 @@
|
|||||||
**/build
|
**/build
|
||||||
web/build
|
web/build
|
||||||
|
|
||||||
# Flutter mobile client — built separately on developer machines / Flutter CI.
|
# The Android client — built by its own job, never from this context. The APK
|
||||||
# Including it in the Go build context wastes ~70 files and invalidates the
|
# reaches the image through client/, downloaded as a CI artifact, so nothing
|
||||||
# `COPY . .` layer cache on every Flutter-only change.
|
# 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 and IDE noise
|
||||||
docs/
|
docs/
|
||||||
@@ -26,5 +37,8 @@ docs/
|
|||||||
!.env.example
|
!.env.example
|
||||||
|
|
||||||
# CI workflow files don't need to ship in the image.
|
# 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 binary, built with `go test -c`
|
||||||
*.test
|
*.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
|
# Bundled Android APK + version sidecar (#397). Populated by CI for
|
||||||
# tag releases; never committed. README in client/ explains the flow.
|
# tag releases; never committed. README in client/ explains the flow.
|
||||||
client/minstrel.apk
|
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.
|
# 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
|
# code=<int> ordering key, minutes since 2020-01-01 at BUILD time
|
||||||
# tag=v<name> what a release of this commit must be called
|
# 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
|
# Both clocks are overridable so a test can pin them. Nothing but tests should
|
||||||
# set these — the defaults are the real derivation.
|
# 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:-}"
|
commit_epoch="${MINSTREL_COMMIT_EPOCH:-}"
|
||||||
if [ -z "${commit_epoch}" ]; then
|
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
|
fi
|
||||||
now_epoch="${MINSTREL_NOW_EPOCH:-$(date -u +%s)}"
|
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")
|
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