make check must not modify tracked files (ensure_pb regenerates mf.pb.go on mtime) #71

New Issue

clawbot · 2026-08-09T03:41:10+02:00

clawbot commented

2026-08-09 03:41:10 +02:00

Context

Policy: "make check must not modify any files in the repo."

script/test:10-15, script/fmt:14-19, and script/fmt-check:9-14 each
carry an identical copy of this heuristic:

if [ ! -f mfer/mf.pb.go ] ||
    [ -n "$(find mfer/mf.proto -newer mfer/mf.pb.go 2>/dev/null)" ]; then
    (cd mfer && go generate .)
fi

Git does not preserve mtimes. On a fresh clone or a branch checkout, file
mtimes are set to checkout time in filesystem order, so mfer/mf.proto can
easily appear newer than the committed mfer/mf.pb.go. When that happens,
script/check silently regenerates a tracked, committed file — mutating the
working tree as a side effect of a read-only gate, and introducing a hard
protoc dependency for anyone running make check.

The comment in the scripts already concedes the intent: "the generated file
is committed, so this is normally a no-op". "Normally" is the problem.

There are also three divergent copies of the same logic, which is how they
will drift.

Definition of done

make check provably does not modify the working tree. Demonstrate it:
touch mfer/mf.proto so it is newer than mfer/mf.pb.go, run
make check, and confirm git status --porcelain is empty afterwards.
Regeneration happens only via an explicit target (e.g. make generate),
never as a side effect of test, lint, fmt-check, or check.
If the committed .pb.go is genuinely out of date with respect to the
.proto, the check fails loudly with an actionable message telling
the developer to run the regenerate target — it does not silently fix it.
Staleness is determined by content, not mtime. An mtime comparison cannot
work across a git checkout and must not be the basis of a gate.
The logic exists in exactly one place, not copy-pasted into three scripts.
make check passes and docker build . succeeds. TODO.md updated in
the same commit.

Implementation requirements

Content-based staleness means regenerating to a temp location and
comparing against the committed file, or comparing a recorded hash of the
.proto. Either is fine; mtime is not.
If the content check itself requires protoc, do not make make check
depend on protoc being installed — that would trade one problem for
another. Skip the staleness check gracefully when the toolchain is absent
and say so, or run the check only in the Docker build where the toolchain
is guaranteed. Decide, and write down which you chose and why.
The Dockerfile currently works around this exact hazard with
RUN touch mfer/mf.pb.go in both the lint and build stages. Once the
scripts no longer regenerate on mtime, those two touch lines are dead
workarounds and must be removed as part of this change.
Keep script/* POSIX sh with no bashisms.
Commit title must end with (closes #71).

## Context Policy: "`make check` must not modify any files in the repo." `script/test:10-15`, `script/fmt:14-19`, and `script/fmt-check:9-14` each carry an identical copy of this heuristic: ```sh if [ ! -f mfer/mf.pb.go ] || [ -n "$(find mfer/mf.proto -newer mfer/mf.pb.go 2>/dev/null)" ]; then (cd mfer && go generate .) fi ``` Git does not preserve mtimes. On a fresh clone or a branch checkout, file mtimes are set to checkout time in filesystem order, so `mfer/mf.proto` can easily appear newer than the committed `mfer/mf.pb.go`. When that happens, `script/check` silently regenerates a tracked, committed file — mutating the working tree as a side effect of a read-only gate, and introducing a hard `protoc` dependency for anyone running `make check`. The comment in the scripts already concedes the intent: "the generated file is committed, so this is normally a no-op". "Normally" is the problem. There are also three divergent copies of the same logic, which is how they will drift. ## Definition of done - `make check` provably does not modify the working tree. Demonstrate it: touch `mfer/mf.proto` so it is newer than `mfer/mf.pb.go`, run `make check`, and confirm `git status --porcelain` is empty afterwards. - Regeneration happens only via an explicit target (e.g. `make generate`), never as a side effect of `test`, `lint`, `fmt-check`, or `check`. - If the committed `.pb.go` is genuinely out of date with respect to the `.proto`, the check **fails loudly** with an actionable message telling the developer to run the regenerate target — it does not silently fix it. - Staleness is determined by content, not mtime. An mtime comparison cannot work across a git checkout and must not be the basis of a gate. - The logic exists in exactly one place, not copy-pasted into three scripts. - `make check` passes and `docker build .` succeeds. `TODO.md` updated in the same commit. ## Implementation requirements - Content-based staleness means regenerating to a temp location and comparing against the committed file, or comparing a recorded hash of the `.proto`. Either is fine; mtime is not. - If the content check itself requires `protoc`, do not make `make check` depend on `protoc` being installed — that would trade one problem for another. Skip the staleness check gracefully when the toolchain is absent and say so, or run the check only in the Docker build where the toolchain is guaranteed. Decide, and write down which you chose and why. - The `Dockerfile` currently works around this exact hazard with `RUN touch mfer/mf.pb.go` in both the lint and build stages. Once the scripts no longer regenerate on mtime, those two `touch` lines are dead workarounds and must be removed as part of this change. - Keep `script/*` POSIX sh with no bashisms. - Commit title must end with ` (closes #71)`.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:41:10 +02:00

clawbot referenced this issue

2026-08-09 07:17:21 +02:00

Configure prettier and make fmt-check cover markdown (closes #69) #88

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/mfer#71