Compare commits

..
Author SHA1 Message Date
yonlu f714fe513d fix(scripts): report only what generation changed, not the worktree
CI / check (push) Skipped
CI / e2e (push) Skipped
The codegen-check hook was `go generate` followed by a bare
`git diff --name-only`, which is the whole unstaged worktree rather than
the generators' output. So a commit whose staged changes were fine failed
whenever anything unrelated sat unstaged — notes, a plan document, the
next commit's files — reporting "Generated code is out of date" and then
a diffstat of files no generator has ever written. `make generate` fixed
nothing, because nothing was stale, so the message sent you looking for a
codegen problem that did not exist. Splitting one piece of work into
several commits is exactly the shape that triggers it.

The tree is snapshotted either side of `go generate` and only what moved
across it is reported. That is deliberately a snapshot rather than the
list of generated paths the issue offers as the other option: a fourth
generator is one //go:generate line away, and a path list is a second
place to remember it.

Two things it has to get right. The comparison is a *symmetric*
difference, because generation can push a file into the unstaged set or
pull it out of one — a hand-edited generated file that the generator puts
back is stale generated code just as much as a source change that
outdates it, and comparing one direction reports it as current. And the
snapshot is content, not names, or a generated file that was already
dirty and is then rewritten further keeps its name on both sides and
slips through.

Closes #131
2026-08-19 14:08:05 -04:00
3 changed files with 87 additions and 74 deletions
+6 -11
View File
@@ -20,19 +20,14 @@ pre-commit:
glob: "*.go"
run: go tool golangci-lint run --timeout 5m ./...
# Snapshots the tree either side of the generators and reports only
# what moved across them. This used to be `go generate` plus a bare
# `git diff --name-only`, which is the *whole unstaged worktree* — so
# any unrelated edit sitting there was reported as stale generated
# code, and `make generate` then fixed nothing. See the script.
codegen-check:
glob: "*.{go,sql,templ}"
run: |
go generate ./...
if [ -n "$(git diff --name-only)" ]; then
echo "Generated code is out of date. Run 'make generate' and stage the changes."
# --no-pager, or this blocks forever on `less` waiting for a
# keypress that a hook run without a tty will never get: the
# commit hangs at exactly the moment it is trying to tell you
# why it failed.
git --no-pager diff --stat
exit 1
fi
run: ./scripts/codegen-check.sh
# frontend/bindings is generated by `wails3`, not `go generate`, so
# the check above does not cover it. ~3.5s warm, ~20s on a cold
+81
View File
@@ -0,0 +1,81 @@
#!/usr/bin/env bash
#
# Fails when `go generate ./...` would change something that is not staged.
#
# The obvious spelling of this is `go generate && git diff --name-only`,
# which is what the hook used to be, and it answers the wrong question:
# that diff is the *whole unstaged worktree*, so any unrelated edit — a
# note, a plan document, the next commit's files sitting there while this
# one lands — was reported as
#
# Generated code is out of date. Run 'make generate' and stage the changes.
#
# Running `make generate` then does nothing, because nothing generated is
# stale, and the message sends you looking for a codegen problem that does
# not exist. Splitting one piece of work into several commits is exactly
# the shape that triggers it, so the workaround was a constraint on commit
# order for no real reason.
#
# So the tree is snapshotted either side of the generators and only what
# *moved across them* is reported. That is deliberately not a list of
# generated paths: sqlcgen, `*_templ.go` and `frontend/src/events.ts` are
# today's answer, a fourth generator is one `//go:generate` line away, and
# a path list is a second place to remember it — the same reasoning that
# keeps staleshape.go parsing sql/schemas/ rather than restating it.
#
# Content, not names: a generated file that is *already* dirty and is then
# rewritten further keeps its name in both snapshots and would otherwise
# slip through.
set -euo pipefail
cd "$(dirname "$0")/.."
# name + worktree blob hash for every file that differs from the index.
# A file listed but absent (a deletion) hashes as "gone" rather than
# aborting the pipeline.
snapshot() {
git diff --name-only | while IFS= read -r f; do
if [ -f "$f" ]; then
printf '%s %s\n' "$f" "$(git hash-object -- "$f")"
else
printf '%s gone\n' "$f"
fi
done
}
# A brand-new generated file is not in either diff, because it is not
# tracked at all — the same blind spot bindings-check.sh names. Both
# snapshots are taken before the generators run.
before="$(snapshot)"
before_untracked="$(git ls-files --others --exclude-standard)"
go generate ./...
after="$(snapshot)"
after_untracked="$(git ls-files --others --exclude-standard)"
# Symmetric difference, and the symmetry is the whole point. Generation
# can push a file *into* the unstaged set (it was current, now it is not)
# or *out* of it (someone hand-edited generated output and the generator
# put it back) — and the second is stale generated code just as much as
# the first. Comparing one direction only reports "current" for it,
# which is the failure this script was written to stop.
moved="$(comm -3 <(printf '%s\n' "$before" | sort) <(printf '%s\n' "$after" | sort) |
cut -d' ' -f1 | tr -d '\t' | sort -u | grep -v '^$' || true)"
if [ -n "$moved" ]; then
echo "codegen-check: generated code is out of date." >&2
echo "Run 'make generate' and stage:" >&2
printf ' %s\n' $moved >&2
exit 1
fi
if [ "$after_untracked" != "$before_untracked" ]; then
echo "codegen-check: generation produced new files. Stage them:" >&2
comm -13 <(printf '%s\n' "$before_untracked" | sort) \
<(printf '%s\n' "$after_untracked" | sort) >&2
exit 1
fi
echo "codegen-check: generated code is current"
-63
View File
@@ -97,55 +97,6 @@ if [ -f "$PID_FILE" ] && kill -0 "$(cat "$PID_FILE")" 2>/dev/null; then
fi
rm -f "$PID_FILE"
# ── Refuse to inherit somebody else's port ───────────────────────────
# The PID check above only knows about *this* worktree: `make dev-stop`
# kills the pid in this .dev/app.pid and nothing else. Several worktrees
# of this repo share the default port, so an app orphaned by a deleted
# worktree goes on listening with nothing left to stop it.
#
# Without this check the new app starts, fails to bind, exits — and every
# curl and playwright-cli call afterwards goes to the *other* process, so
# the harness reports facts about an app nobody asked for. That is not a
# quiet wrongness either: it presented as
# "no such table: libraries" against a freshly created YJ_HOME, which
# reads exactly like applySchema or staleshape.go having gone wrong and
# is a frightening place to start looking.
#
# The startup wait below cannot catch it, because the health check is
# satisfied by *any* app on the port — which is precisely the failure.
# So it is refused here, before anything is launched, rather than warned
# about. --port already exists for the legitimate second-app case.
port_holder() {
command -v ss >/dev/null || return 0
ss -lptn "sport = :$PORT" 2>/dev/null | grep -oP 'pid=\K[0-9]+' | head -n 1
}
if curl -sf -o /dev/null --max-time 2 "http://localhost:$PORT/" ||
[ -n "$(port_holder)" ]; then
holder="$(port_holder)"
echo "dev-headless: :$PORT is already in use; refusing to start" >&2
if [ -n "$holder" ]; then
# /proc/<pid>/cwd names the checkout it belongs to, and says
# "(deleted)" for the orphaned-worktree case that is the whole
# reason this is worth a check.
cwd="$(readlink "/proc/$holder/cwd" 2>/dev/null || echo unknown)"
cmd="$(tr '\0' ' ' <"/proc/$holder/cmdline" 2>/dev/null || echo unknown)"
echo " pid $holder ($cmd)" >&2
echo " cwd $cwd" >&2
# The PID-file check above has already passed, so whatever this
# is, `make dev-stop` does not know about it — saying otherwise
# sends you to a command that will report success and change
# nothing. Never `pkill -f` here either: the pattern would
# match this script's own command line.
echo " 'make dev-stop' will not touch it (it is not in" >&2
echo " ${PID_FILE#"$REPO_ROOT"/}): kill $holder, or pass --port." >&2
else
echo " The holder could not be identified (no ss, or it belongs" >&2
echo " to another user). Try: ss -lptn 'sport = :$PORT'" >&2
fi
exit 1
fi
# ── Choose the YJ_HOME ───────────────────────────────────────────────
# A seed is a YJ_HOME that a previous run of the app produced, tarred
# up (see scripts/seed-sandbox.sh). Restoring it means starting *in*
@@ -249,20 +200,6 @@ until curl -sf -o /dev/null "http://localhost:$PORT/"; do
sleep 0.25
done
# The loop above exits on the first answer from the port, and "something
# answered" is not "the app we started answered". The pre-launch guard
# makes that unlikely rather than impossible — a race, or a listener
# started in between — and the check is one signal, so it is worth making
# here too. An empty log beside a dead pid is the "it exited immediately
# and nothing said so" case that the original report spent its time on.
if ! kill -0 "$APP_PID" 2>/dev/null; then
echo "dev-headless: :$PORT answered, but the app we started (pid" >&2
echo " $APP_PID) is gone — something else holds the port." >&2
tail -n 30 "$LOG_FILE" >&2
rm -f "$PID_FILE"
exit 1
fi
cat <<EOF
dev-headless: up
url http://localhost:$PORT