Makefile has no cibuild or precommit shims, so two documented entrypoints have no make target #34

Closed
opened 2026-08-09 18:31:05 +02:00 by clawbot · 2 comments
Collaborator

Problem

REPO_POLICIES.md: "The Makefile is authoritative documentation for how the
repo is used… If someone checks out the repo and types make<tab>, they should
see every meaningful operation available."

The Makefile provides bootstrap setup test lint fmt fmt-check check docker hooks serve. It has no target for script/cibuild or script/precommit,
both of which exist, are documented in the README's Entrypoints section, and
are part of the standard entrypoint set.

Raised independently by two reviewers, most recently on PR #32 where the
reviewer had to invoke script/cibuild directly because no make target exists.

This matters more than it did before two recent changes:

  • Since #30, a bare docker build . fails by design (the CHECK_EPOCH guard),
    and README.md now tells people to build the image only via script/cibuild
    or script/docker. So the one build path a newcomer is directed to has no
    make target, while make docker — the other path — does.
  • The global convention is to use make targets rather than invoking underlying
    tools. That is impossible for the CI build as things stand.

Fix

Add thin shims, consistent with every existing target:

cibuild:
	@script/cibuild

precommit:
	@script/precommit

and add both to .PHONY.

Check .PHONY is complete while there — it should list every target the
Makefile defines.

Definition of done

  1. make cibuild and make precommit exist and shim to the corresponding
    scripts, matching the existing target style.
  2. .PHONY lists every target defined in the Makefile.
  3. make cibuild succeeds and its check layer genuinely executes — it must not
    report CACHED on RUN ... make check (see #23/#30).
  4. make precommit succeeds on a clean tree.
  5. README Entrypoints section mentions the make targets where it currently
    names only the scripts, so the two documents agree.
  6. make check passes.
  7. TODO.md updated.

Note

script/cibuild runs docker build, so make cibuild will be slow compared
with other targets. That is expected — it is the CI build — but worth a word in
the README so nobody types it expecting make check speed.

Out of scope

  • Changing what script/cibuild or script/precommit do.
  • The prettier-flag comment work, which is tracked in the sibling issue filed
    alongside this one.
## Problem `REPO_POLICIES.md`: "The Makefile is authoritative documentation for how the repo is used… If someone checks out the repo and types `make<tab>`, they should see every meaningful operation available." The `Makefile` provides `bootstrap setup test lint fmt fmt-check check docker hooks serve`. It has **no target for `script/cibuild` or `script/precommit`**, both of which exist, are documented in the README's Entrypoints section, and are part of the standard entrypoint set. Raised independently by two reviewers, most recently on PR #32 where the reviewer had to invoke `script/cibuild` directly because no make target exists. This matters more than it did before two recent changes: - Since #30, a bare `docker build .` fails by design (the `CHECK_EPOCH` guard), and `README.md` now tells people to build the image only via `script/cibuild` or `script/docker`. So the one build path a newcomer is directed to has no make target, while `make docker` — the other path — does. - The global convention is to use make targets rather than invoking underlying tools. That is impossible for the CI build as things stand. ## Fix Add thin shims, consistent with every existing target: ```make cibuild: @script/cibuild precommit: @script/precommit ``` and add both to `.PHONY`. Check `.PHONY` is complete while there — it should list every target the `Makefile` defines. ## Definition of done 1. `make cibuild` and `make precommit` exist and shim to the corresponding scripts, matching the existing target style. 2. `.PHONY` lists every target defined in the `Makefile`. 3. `make cibuild` succeeds and its check layer genuinely executes — it must not report `CACHED` on `RUN ... make check` (see #23/#30). 4. `make precommit` succeeds on a clean tree. 5. README Entrypoints section mentions the make targets where it currently names only the scripts, so the two documents agree. 6. `make check` passes. 7. `TODO.md` updated. ## Note `script/cibuild` runs `docker build`, so `make cibuild` will be slow compared with other targets. That is expected — it is the CI build — but worth a word in the README so nobody types it expecting `make check` speed. ## Out of scope - Changing what `script/cibuild` or `script/precommit` do. - The prettier-flag comment work, which is tracked in the sibling issue filed alongside this one.
Author
Collaborator

Two premises in the body are stale after 25b6c0a; DoD item 3 needs restating.

  • The Dockerfile no longer runs make check — it runs script/test then script/fmt-check, and lint runs in Dockerfile.lint. So "must not report CACHED on RUN ... make check" no longer names a real line. The requirement it stands for survives: make cibuild must be shown to execute its checks rather than return a warm-cache green, and script/cibuild now runs script/lint first, so there are two builds to account for, not one.
  • make lint now requires buildx. Do not paper over that here; it is #40 .

Everything else in the DoD stands. Scope stays exactly what the title says — Makefile shims plus the README wording that makes the two agree. Do not touch script/cibuild or script/precommit behaviour, and leave the README's script/lint entry alone; it is owned by #40 .

Two premises in the body are stale after `25b6c0a`; DoD item 3 needs restating. - The `Dockerfile` no longer runs `make check` — it runs `script/test` then `script/fmt-check`, and lint runs in `Dockerfile.lint`. So "must not report `CACHED` on `RUN ... make check`" no longer names a real line. The requirement it stands for survives: `make cibuild` must be shown to execute its checks rather than return a warm-cache green, and `script/cibuild` now runs `script/lint` first, so there are two builds to account for, not one. - `make lint` now requires buildx. Do not paper over that here; it is https://git.eeqj.de/sneak/lora.vegas/issues/40 . Everything else in the DoD stands. Scope stays exactly what the title says — Makefile shims plus the README wording that makes the two agree. Do not touch `script/cibuild` or `script/precommit` behaviour, and leave the README's `script/lint` entry alone; it is owned by https://git.eeqj.de/sneak/lora.vegas/issues/40 .
Author
Collaborator

Done in 341c8af on next, carried by #41 .

make cibuild and make precommit added as thin shims; .PHONY lists both (it was already complete otherwise). README's Entrypoints section now carries the script-to-target mapping, the two names that differ (script/install-precommit is make hooks, script/precommit is make precommit), and the note that make cibuild costs minutes. The script/lint bullet was left alone per #40 .

Verification: make check green, make precommit green on a clean tree. make cibuild was shown to execute rather than replay cache — one invocation, 9.8s wall, ran both builds with distinct CHECK_EPOCH values, RUN script/bootstrap CACHED above and no check layer cached below: the Dockerfile.lint build echoed lint epoch: 17863691323256328833837425 and printed hugo's build table, then the main image build echoed check epoch: 17863691366690793943837425, ran script/test, and ran script/fmt-check for 3.7s printing All matched files use Prettier code style!.

Buildx was present on the machine used, so the make lint buildx dependency did not bite here.

Done in `341c8af` on `next`, carried by https://git.eeqj.de/sneak/lora.vegas/pulls/41 . `make cibuild` and `make precommit` added as thin shims; `.PHONY` lists both (it was already complete otherwise). README's Entrypoints section now carries the script-to-target mapping, the two names that differ (`script/install-precommit` is `make hooks`, `script/precommit` is `make precommit`), and the note that `make cibuild` costs minutes. The `script/lint` bullet was left alone per https://git.eeqj.de/sneak/lora.vegas/issues/40 . Verification: `make check` green, `make precommit` green on a clean tree. `make cibuild` was shown to execute rather than replay cache — one invocation, 9.8s wall, ran both builds with distinct `CHECK_EPOCH` values, `RUN script/bootstrap` `CACHED` above and no check layer cached below: the `Dockerfile.lint` build echoed `lint epoch: 17863691323256328833837425` and printed hugo's build table, then the main image build echoed `check epoch: 17863691366690793943837425`, ran `script/test`, and ran `script/fmt-check` for 3.7s printing `All matched files use Prettier code style!`. Buildx was present on the machine used, so the `make lint` buildx dependency did not bite here.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/lora.vegas#34