From 279b10164dcb5a6bb1c0c5239511603b2c78693e Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 26 Oct 2026 20:39:00 +0100 Subject: [PATCH] Keep a Python test phase's lockfile install in check builds (closes #133) A Python `test` phase that installs its packages from `Pipfile.lock` now does so above `ARG CACHEBUST`, through `script/bootstrap`, in a layer of its own. Check builds keep that layer as they keep Go's modules, and the tests still run after the line on every build. In that step `script/bootstrap` installs only system packages, pipenv and its dependencies by hash, and the lockfile's packages. The policy, both checklists and the README list the steps above the line the same way; the policy has a short example. This writes down sneak's ruling at https://git.eeqj.de/sneak/rfscan/issues/80#issuecomment-153437. rfscan installs pipenv by version, not by hash, so its `script/bootstrap` does not meet this rule yet. Model: opus-5-5 --- README.md | 18 +++-- TODO.md | 16 ++++ prompts/EXISTING_REPO_CHECKLIST.md | 35 +++++++-- prompts/NEW_REPO_CHECKLIST.md | 35 +++++++-- prompts/REPO_POLICIES.md | 113 +++++++++++++++++++++++------ 5 files changed, 173 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index fce9748..7479b61 100644 --- a/README.md +++ b/README.md @@ -144,12 +144,18 @@ alpine. We provide: `make hooks` installs the pre-commit hook that runs `script/precommit`. -In every stage of a `Dockerfile`, `ARG CACHEBUST` comes right after the steps -that install system packages and download Go's modules, or right after `FROM` in -a stage with neither, and every step that lints, tests or compiles comes after -it. The `script/` builds pass a new `CACHEBUST` value on every build, which -reruns every `RUN` step after that line and leaves the layers above it and the -cache mounts in place; `--no-cache` would build those layers again and empty the +In every stage of a `Dockerfile`, every step that lints, tests or compiles comes +after `ARG CACHEBUST`. `ARG CACHEBUST` comes right after these steps: + +- the steps that install system packages +- the steps that download Go's modules +- the step that runs `script/bootstrap` to install a Python `test` phase's + packages from `Pipfile.lock` + +A stage with none of these steps declares `ARG CACHEBUST` right after `FROM`. +The `script/` builds pass a new `CACHEBUST` value on every build, which reruns +every `RUN` step after that line and leaves the layers above it and the cache +mounts in place; `--no-cache` would build those layers again and empty the mounts. This repo's own `Dockerfile` declares `ARG CACHEBUST` right after `FROM` in every stage: each stage installs system packages and node packages together in one `script/bootstrap` step, and check builds may keep system packages but diff --git a/TODO.md b/TODO.md index 8c927fd..4022afa 100644 --- a/TODO.md +++ b/TODO.md @@ -21,6 +21,22 @@ fmt-check, and commit. # Completed Steps +- 2026-10-21: Check builds may now also keep the Python packages a `test` phase + installs from `Pipfile.lock` (issue 133), on sneak's ruling in + https://git.eeqj.de/sneak/rfscan/issues/80#issuecomment-153437. The install + comes before `ARG CACHEBUST`: `COPY script/ script/`, + `COPY Pipfile Pipfile.lock ./`, then `RUN script/bootstrap`. In that step + `script/bootstrap` installs nothing beyond system packages, pipenv and its + dependencies by hash, and the packages from the lockfile. It installs no + Python, because the tests use the base image's. `--deploy` installs only what + the lockfile names, checks every package against its hashes, and fails when + the lockfile is out of date with `Pipfile`. A kept layer holds nothing a new + install would refuse. A package built from source and the system packages may + differ in a new install. As with Go's modules, the packages are kept in a + layer, not a cache mount. The tests still run after `ARG CACHEBUST`, and every + other stage still installs its Python packages after it. `REPO_POLICIES.md` + gains a short Python `test` phase example, and both checklists and the README + state the rule the same way. rfscan picks this up on its next re-vendor. - 2026-10-21: The canonical `.golangci.yml` now accepts a local `replace` of `github.com/grafana/sobek` in a repository's root `go.mod` (issue 134), for homoicon, which keeps a local copy of sobek on sneak's decision in diff --git a/prompts/EXISTING_REPO_CHECKLIST.md b/prompts/EXISTING_REPO_CHECKLIST.md index 4f1358e..79625c6 100644 --- a/prompts/EXISTING_REPO_CHECKLIST.md +++ b/prompts/EXISTING_REPO_CHECKLIST.md @@ -1,6 +1,6 @@ --- title: Existing Repo Checklist -last_modified: 2026-10-18 +last_modified: 2026-10-26 --- Use this checklist when beginning work in a repo that may not yet conform to our @@ -49,10 +49,14 @@ with your task. `--target` names it, so a lost `COPY --from=` edge leaves `docker build .` passing while the gate never runs. Confirm by planting a violation, not by reading the file. -- [ ] In every stage `ARG CACHEBUST` comes right after the steps that install - system packages and download Go's modules, or right after `FROM` in a - stage with neither, and every step that lints, tests or compiles comes - after it. A step that installs anything else as well, such as a +- [ ] In every stage, every step that lints, tests or compiles comes after + `ARG CACHEBUST`. `ARG CACHEBUST` comes right after these steps: + - the steps that install system packages + - the steps that download Go's modules + - the step that runs `script/bootstrap` to install a Python `test` phase's + packages from `Pipfile.lock` +- [ ] A stage with none of these steps declares `ARG CACHEBUST` right after + `FROM`. A step that installs anything else as well, such as a `script/bootstrap` that also installs node packages, comes after it too. The `script/` builds pass a new `CACHEBUST` value on every build, which reruns every `RUN` step after that line and leaves the layers above it and @@ -68,6 +72,18 @@ with your task. (`--mount=type=cache,target=/root/.cache/golangci-lint`), and no step mounts anything else; both `go test` runs carry `-count=1`, as in the canonical Go `Dockerfile` in `REPO_POLICIES.md` +- [ ] Python: a `test` phase that installs its packages from `Pipfile.lock` does + it above `ARG CACHEBUST`, with `COPY script/ script/`, + `COPY Pipfile Pipfile.lock ./` and `RUN script/bootstrap`. When + `script/bootstrap` runs in that step, it installs nothing beyond system + packages, pipenv and its dependencies, and the packages from + `Pipfile.lock`. It installs pipenv and its dependencies by hash, with + `pip install --require-hashes`. It installs the packages from + `Pipfile.lock` with `pipenv install --deploy`, plus `--dev` when the tests + need the development packages. It installs no Python there, because the + tests use the base image's. The tests run after `ARG CACHEBUST`, and every + other stage installs its Python packages after it, as in the Python `test` + phase example in `REPO_POLICIES.md` - [ ] The gate phases invoke their tools directly, never through `make lint` or `script/test` — those are themselves a `docker build` and would recurse inside a build step @@ -232,9 +248,12 @@ with your task. (the runner image `docker.gitea.com/runner-images:ubuntu-latest`: docker, git and node 24, with no yarn on `PATH`), and demonstrably executed the checks — a sub-second build, or `CACHED` on a step that lints or tests, - one of the `RUN` steps below `ARG CACHEBUST`, means nothing ran. The steps - that install system packages and download Go's modules above that line are - expected to show `CACHED`. + one of the `RUN` steps below `ARG CACHEBUST`, means nothing ran. Above + that line, these steps are expected to show `CACHED`: + - the steps that install system packages + - the steps that download Go's modules + - the step that runs `script/bootstrap` to install a Python `test` phase's + packages from `Pipfile.lock` - [ ] A planted lint violation fails both `make lint` and a plain `docker build .`; revert it afterwards - [ ] Commit and merge fixes before starting your actual task diff --git a/prompts/NEW_REPO_CHECKLIST.md b/prompts/NEW_REPO_CHECKLIST.md index 31f4afc..51dd89c 100644 --- a/prompts/NEW_REPO_CHECKLIST.md +++ b/prompts/NEW_REPO_CHECKLIST.md @@ -1,6 +1,6 @@ --- title: New Repo Checklist -last_modified: 2026-10-18 +last_modified: 2026-10-26 --- Use this checklist when creating a new repository from scratch. Follow the steps @@ -108,10 +108,14 @@ Template files can be fetched from: stage carries a `COPY --from=` of a harmless file from each so the image cannot be built unless both passed. Keep the final stage last: a stage nothing depends on is built only when `--target` names it. - - In every stage `ARG CACHEBUST` comes right after the steps that install - system packages and download Go's modules, or right after `FROM` in a - stage with neither, and every step that lints, tests or compiles comes - after it. A step that installs anything else as well, such as a + - In every stage, every step that lints, tests or compiles comes after + `ARG CACHEBUST`. `ARG CACHEBUST` comes right after these steps: + - the steps that install system packages + - the steps that download Go's modules + - the step that runs `script/bootstrap` to install a Python `test` + phase's packages from `Pipfile.lock` + - A stage with none of these steps declares `ARG CACHEBUST` right after + `FROM`. A step that installs anything else as well, such as a `script/bootstrap` that also installs node packages, comes after it too. The `script/` builds pass a new `CACHEBUST` value on every build, which reruns every `RUN` step after that line and leaves the layers above it and @@ -127,6 +131,18 @@ Template files can be fetched from: (`--mount=type=cache,target=/root/.cache/golangci-lint`), and no step mounts anything else; both `go test` runs carry `-count=1`, as in the canonical Go `Dockerfile` in `REPO_POLICIES.md` + - Python: a `test` phase that installs its packages from `Pipfile.lock` does + it above `ARG CACHEBUST`, with `COPY script/ script/`, + `COPY Pipfile Pipfile.lock ./` and `RUN script/bootstrap`. When + `script/bootstrap` runs in that step, it installs nothing beyond system + packages, pipenv and its dependencies, and the packages from + `Pipfile.lock`. It installs pipenv and its dependencies by hash, with + `pip install --require-hashes`. It installs the packages from + `Pipfile.lock` with `pipenv install --deploy`, plus `--dev` when the tests + need the development packages. It installs no Python there, because the + tests use the base image's. The tests run after `ARG CACHEBUST`, and every + other stage installs its Python packages after it, as in the Python `test` + phase example in `REPO_POLICIES.md` - Server: the final stage builds and runs the application - Non-server: the final stage brings up the dev environment - Image pinned by sha256 hash with version/date comment @@ -215,9 +231,12 @@ are thin shims calling them. Model scripts: (the runner image `docker.gitea.com/runner-images:ubuntu-latest`: docker, git and node 24, with no yarn on `PATH`), and demonstrably executed the checks — a sub-second build, or `CACHED` on a step that lints or tests, - one of the `RUN` steps below `ARG CACHEBUST`, means nothing ran. The steps - that install system packages and download Go's modules above that line are - expected to show `CACHED`. + one of the `RUN` steps below `ARG CACHEBUST`, means nothing ran. Above + that line, these steps are expected to show `CACHED`: + - the steps that install system packages + - the steps that download Go's modules + - the step that runs `script/bootstrap` to install a Python `test` phase's + packages from `Pipfile.lock` - [ ] Plant a lint violation and confirm both `make lint` and a plain `docker build .` fail on it; revert. A plain build that passes proves the final stage is missing its `COPY --from=` edge to the gate phases. diff --git a/prompts/REPO_POLICIES.md b/prompts/REPO_POLICIES.md index 1067698..4051b5f 100644 --- a/prompts/REPO_POLICIES.md +++ b/prompts/REPO_POLICIES.md @@ -1,6 +1,6 @@ --- title: Repository Policies -last_modified: 2026-10-18 +last_modified: 2026-10-26 --- This document covers repository structure, tooling, and workflow standards. Code @@ -174,27 +174,32 @@ style conventions are in separate documents: `script/cibuild`. `script/lint`, `script/test`, `script/cibuild` and `script/docker` are the four, and there is no fifth — `script/check` runs the two gate phases and `script/fmt-check`, and builds no image of its own. In - every stage of the `Dockerfile`, `ARG CACHEBUST` comes right after the steps - that install system packages and download Go's modules, or right after `FROM` - in a stage with neither, and every step that lints, tests or compiles comes - after it. A step that installs anything else as well, such as a - `script/bootstrap` that also installs node packages, comes after it too. A new - value reruns every `RUN` step after that line; `COPY` steps are compared by - content and need nothing. Docker keeps the layers above the line until the - base image, their own lines or the files they copy change. A stage without the - `ARG` line is served from cache on an unchanged tree, so a repository - re-vendors the scripts and adds the `ARG` lines in one commit. The scripts do - not pass `--no-cache`: it reruns every step too, but it also builds the layers - above the line again and empties every cache mount, so Go's build cache and - golangci-lint's, which the canonical Go `Dockerfile` below mounts so that they - survive between check builds, would start empty every time. A bare - `docker build .` is not evidence that anything ran: a sub-second build - reporting success is a cache hit, not a result. Never invalidate by pruning — - `docker builder prune` and friends destroy a build cache shared with every - other build on the host. When a check is added or changed, prove it works by - planting a defect it must catch and watching the run fail on it, then revert - the defect. A green run alone shows neither that the check ran nor that it - covers what it should. + every stage of the `Dockerfile`, every step that lints, tests or compiles + comes after `ARG CACHEBUST`. `ARG CACHEBUST` comes right after these steps: + - the steps that install system packages + - the steps that download Go's modules + - the step that runs `script/bootstrap` to install a Python `test` phase's + packages from `Pipfile.lock` + + A stage with none of these steps declares `ARG CACHEBUST` right after + `FROM`. A step that installs anything else as well, such as a + `script/bootstrap` that also installs node packages, comes after it too. A + new value reruns every `RUN` step after that line; `COPY` steps are compared + by content and need nothing. Docker keeps the layers above the line until + the base image, their own lines or the files they copy change. A stage + without the `ARG` line is served from cache on an unchanged tree, so a + repository re-vendors the scripts and adds the `ARG` lines in one commit. + The scripts do not pass `--no-cache`: it reruns every step too, but it also + builds the layers above the line again and empties every cache mount, so + Go's build cache and golangci-lint's, which the canonical Go `Dockerfile` + below mounts so that they survive between check builds, would start empty + every time. A bare `docker build .` is not evidence that anything ran: a + sub-second build reporting success is a cache hit, not a result. Never + invalidate by pruning — `docker builder prune` and friends destroy a build + cache shared with every other build on the host. When a check is added or + changed, prove it works by planting a defect it must catch and watching the + run fail on it, then revert the defect. A green run alone shows neither that + the check ran nor that it covers what it should. - **The gate phases are separate stages, and the build stage depends on both.** The lint phase is based on the `golangci/golangci-lint` image (pinned by @@ -309,6 +314,9 @@ style conventions are in separate documents: a package's findings only when the package's files and those of every package it imports, `go.mod`, the linters and their settings in `.golangci.yml`, and the golangci-lint version are all unchanged. + - In a Python `test` phase, the layer of the step that runs + `script/bootstrap` to install the packages from `Pipfile.lock`, as + described for that phase below. - If the project uses `//go:embed` directives that reference build artifacts (e.g. a web frontend compiled in a separate stage), the lint phase must create placeholder files so the embed directives resolve. Example: @@ -363,6 +371,67 @@ style conventions are in separate documents: `--build-arg VERSION=...`, as `script/docker` and `script/cibuild` already do. +- **A Python `test` phase that installs its packages from `Pipfile.lock` does it + above `ARG CACHEBUST`, so check builds keep them.** The steps are + `COPY script/ script/`, `COPY Pipfile Pipfile.lock ./` and + `RUN script/bootstrap`, with `ARG CACHEBUST` right after them. Docker keeps + what that `RUN` installed until the base image, `script/`, `Pipfile`, + `Pipfile.lock` or a line up to and including the `RUN` changes. The tests run + after `ARG CACHEBUST`, on every check build. In every other stage, a step that + installs Python packages comes after `ARG CACHEBUST`. + - When `script/bootstrap` runs in that step, it installs nothing beyond + system packages, pipenv and its dependencies, and the packages from + `Pipfile.lock`. It installs no Python there: the tests use the base + image's Python, which the image's hash pins. If it installs anything else + there, the step goes after `ARG CACHEBUST`. + - Every external reference must be pinned by hash, so `script/bootstrap` + installs pipenv and its dependencies by hash. It does so with + `pip install --require-hashes -r` and a requirements file in `script/` + that lists each of them with `==` and its `--hash`. pip refuses to install + a dependency the file does not list. + - It then runs `pipenv install --deploy`, with `--dev` when the tests need + the development packages. `--deploy` installs only what `Pipfile.lock` + names, checks every package against the lockfile's hashes, and fails when + the lockfile is out of date with `Pipfile`. + - A kept layer holds nothing a new install would refuse: pip checked pipenv + and its dependencies, and pipenv checked the lockfile's packages, against + their hashes. pipenv creates its virtual environment with the `virtualenv` + package. The packages that `virtualenv` adds come from wheels inside that + package, so the hash pip checked for it covers them. Two things in the + layer are not fixed by a hash. A package with no ready-built wheel for the + image is built from source. pip downloads the build tools without a hash + check, so a kept layer's build of such a package may differ from a new + one. And system packages keep the versions the package manager gave when + the layer was built; a new install may get newer ones. + - As with Go's modules, the packages are kept in a layer, not a cache mount. + A cache mount is shared by every build on the host, and nothing checks its + files again. + + In this example, `script/bootstrap` finds `make` and `git` already + installed, installs pipenv and its dependencies by hash, and runs + `pipenv install --dev --deploy`: + + ```dockerfile + # Test phase. The 90-second timeout covers both pytest runs. + # python:3.x-slim, YYYY-MM-DD + FROM python@sha256:... AS test + # make and git are base tools that script/bootstrap installs when they + # are missing. Installed here, they stay in their own layer when + # script/ changes. + RUN apt-get update \ + && apt-get install -y --no-install-recommends make git \ + && rm -rf /var/lib/apt/lists/* + WORKDIR /app + COPY script/ script/ + COPY Pipfile Pipfile.lock ./ + RUN script/bootstrap + ARG CACHEBUST + COPY . . + RUN timeout 90 sh -c 'pipenv run pytest || { \ + echo "--- Rerunning with -v for details ---"; \ + pipenv run pytest -v; exit 1; }' + ``` + - Every repo should have a Gitea Actions workflow (`.gitea/workflows/`) that runs `script/cibuild` on push, and checks out the repo as its only other step, with `persist-credentials: false`: `script/cibuild` needs no token, and