diff --git a/.dockerignore b/.dockerignore index 0228778..6b49f01 100644 --- a/.dockerignore +++ b/.dockerignore @@ -17,7 +17,17 @@ # stage that compiles runs `git describe --tags --always` on .git, which # does not need .git/config; that file can hold a credential, such as a # password in a remote URL or the token the CI checkout step stores there. -.git/config +# Each submodule keeps a config with the same exposure in its git directory +# under .git/modules/, nested again for a submodule's own submodules, or in +# its own .git directory when it keeps one. +# KNOWN GAP: a submodule whose name has a `config` segment (`config`, +# `deploy/config`, `config/lib`) loses its whole git directory, because +# `**/.git/modules/**/config` also matches that segment's directory +# under .git/modules/. Go's version stamping then fails the build; +# nothing leaks. Name such a submodule without that segment: +# `git submodule add --name`. +**/.git/config +**/.git/modules/**/config # Agent scratch: one full checkout of the repo per in-flight agent. # Anchored because it occurs once where agents run at the repo root. @@ -41,7 +51,9 @@ **/[iI][dD]_[rR][sS][aA] **/[iI][dD]_[dD][sS][aA] **/[iI][dD]_[eE][cC][dD][sS][aA] +**/[iI][dD]_[eE][cC][dD][sS][aA]_[sS][kK] **/[iI][dD]_[eE][dD]25519 +**/[iI][dD]_[eE][dD]25519_[sS][kK] # Dependencies: restored inside the image, never copied in. **/node_modules @@ -59,12 +71,9 @@ **/.vscode **/*.sublime-* -# This repo's own entries. -.gitea -*.md -LICENSE -vaultik -dist -.tool -coverage.out -coverage.html +# This repo's own host-built artifacts. +/vaultik +/dist +/.tool +/coverage.out +/coverage.html diff --git a/.editorconfig b/.editorconfig index 2fe0ce0..92ec261 100644 --- a/.editorconfig +++ b/.editorconfig @@ -10,3 +10,6 @@ insert_final_newline = true [Makefile] indent_style = tab + +[*.go] +indent_style = tab diff --git a/.gitea/workflows/check.yml b/.gitea/workflows/check.yml index b4e58dc..ee73864 100644 --- a/.gitea/workflows/check.yml +++ b/.gitea/workflows/check.yml @@ -1,14 +1,9 @@ name: check -on: - push: - branches: [main, next] - pull_request: - branches: [main, next] +on: [push] jobs: - check: - runs-on: ubuntu-latest - steps: - # actions/checkout v4, 2024-09-16 - - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 - - name: Build and check - run: script/cibuild + check: + runs-on: ubuntu-latest + steps: + # actions/checkout v4.2.2, 2026-02-22 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 + - run: script/cibuild diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index b083588..9d30f41 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -16,11 +16,12 @@ jobs: fetch-depth: 0 # goreleaser is not a compiler: it shells out to `go` for the # `before:` hook and for every one of the four cross-compiles. - # Nothing else in this repo puts a Go toolchain on the runner -- - # check.yml runs script/cibuild, which does all of its work inside - # the digest-pinned Dockerfile images -- so without this step the - # release either fails at the before-hook or, worse, ships binaries - # built by whatever Go the runner happens to carry. + # Without this step the release either fails at the before-hook or, + # worse, ships binaries built by whatever Go the runner happens to + # carry. check.yml's runner gets the same Go through + # script/bootstrap, which calls script/install-go, and uses it only + # for `go mod download` and gofmt; it compiles inside the + # digest-pinned Dockerfile images. # # actions/setup-go would pin the action by commit sha, but the Go # tarball it downloads at runtime is verified against no value in diff --git a/.gitignore b/.gitignore index df0dea6..fe7bf4a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,28 +1,62 @@ -# Binary -/vaultik - -# goreleaser output -/dist/ - -# Locally installed pinned tools (script/install-goreleaser) -/.tool/ - -# Test artifacts -*.out -*.test -coverage.html -coverage.out - -# IDE -.vscode/ -.idea/ -*.swp -*.swo - # OS .DS_Store Thumbs.db -# Local config for development +# Editors +*.swp +*.swo +*~ +*.bak +.idea/ +.vscode/ +*.sublime-* + +# Agent scratch (worktrees of this repo, created and destroyed by +# in-flight tooling). Unanchored: .gitignore patterns already match at +# every depth, so no prefix is wanted here. This is not a .dockerignore +# entry and must not be given a `**/` prefix on the way into one. +.claude/ + +# Node +node_modules/ + +# Secrets. Unanchored like every entry above, so each matches at every +# depth. Matching is case-sensitive on Linux, so names use character +# ranges rather than a lowercase form that misses `Server.Key`. + +# Environment files. `*.env` covers bare `.env` and the `prod.env` +# convention. Only the templates `example.env` and `sample.env` are +# re-included below. A repository that commits any other template adds +# its own negation after these lines, for example `!.env.example`. +*.[eE][nN][vV] +.[eE][nN][vV].* +.[eE][nN][vV][rR][cC] +!example.env +!sample.env + +# Private keys and the bundles carrying them. +*.[pP][eE][mM] +*.[kK][eE][yY] +*.[pP]12 +*.[pP][fF][xX] +[iI][dD]_[rR][sS][aA] +[iI][dD]_[dD][sS][aA] +[iI][dD]_[eE][cC][dD][sS][aA] +[iI][dD]_[eE][cC][dD][sS][aA]_[sS][kK] +[iI][dD]_[eE][dD]25519 +[iI][dD]_[eE][dD]25519_[sS][kK] + +# Go build and test output. +*.log +*.out +*.test +coverage.html + +# This repo's own host-built artifacts. +/vaultik +/dist/ +/.tool/ + +# Local configs for development; they hold storage credentials. local-config.yaml -dev-config.yaml \ No newline at end of file +dev-config.yaml diff --git a/.golangci.yml b/.golangci.yml index d8179d9..39d54d2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -17,6 +17,7 @@ linters: disable: # Genuinely incompatible with project patterns - exhaustruct # Requires all struct fields + - exhaustruct_v5 # Requires all struct fields (successor to exhaustruct) - godot # Requires comments to end with periods - wrapcheck # Too verbose for internal packages - varnamelen # Short names like db, id are idiomatic Go diff --git a/AGENTS.md b/AGENTS.md index 2f93504..2f253ae 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -108,6 +108,23 @@ Version: 2025-06-08 code that touches the affected tables) directly. After 1.0, each schema change is a new numbered file in that directory and a released file is never edited; an existing local database is then migrated when vaultik - is updated. See + is updated. Before 1.0, a local database left on an older schema is + deleted and re-created by a full backup. See [`docs/DATAMODEL.md`](docs/DATAMODEL.md#schema-migrations). +14. Never use `git add -A`. Stage only the files you intentionally + changed. + +15. Commit messages carry no attribution or advertising trailers for the + tool that helped write the code, or for its vendor: the owner is the + sole author of code written with a tool. + +16. Run the whole test suite with `make test` every time, and read its full + output. Never run `go test`, a single test or a single package, and + never grep the output. + +17. Do not stop working on a task until the definition of done given in the + initial instruction is met: all of the work, not part or most of it. + +18. For estimates: backing up over 2.5Gbit/s ethernet to an S3 server + backed by 2000MB/sec SSD takes about 4 seconds per gigabyte. diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index 7b5788e..0000000 --- a/CLAUDE.md +++ /dev/null @@ -1,44 +0,0 @@ -# Rules - -Read the rules in AGENTS.md and follow them. - -# Memory - -* Claude is an inanimate tool. The spam that Claude attempts to insert into - commit messages (which it erroneously refers to as "attribution") is not - attribution, as I am the sole author of code created using Claude. It is - corporate advertising for Anthropic and is therefore completely - unacceptable in commit messages. - -* NEVER use `git add -A`. Always add only the files you intentionally - changed. - -* Tests should always be run before committing code. No commits should be - made that do not pass tests. - -* Code should always be formatted before committing. Do not commit - unformatted code. - -* Code should always be linted before committing. Do not commit - unlinted code. - -* The test suite is fast and local. When running tests, don't run - individual parts of the test suite, always run the whole thing by running - "make test". - -* Do not stop working on a task until you have reached the definition of - done provided to you in the initial instruction. Don't do part or most of - the work, do all of the work until the criteria for done are met. - -* We do not add migrations before 1.0; schema upgrades can be handled by - deleting the local state file and doing a full backup to re-create it. - -* When testing on a 2.5Gbit/s ethernet to an s3 server backed by 2000MB/sec SSD, - estimate about 4 seconds per gigabyte of backup time. - -* When running tests, don't run individual tests, or grep the output. run - the entire test suite every time and read the full output. - -* When running tests, don't run individual tests, or try to grep the output. - never run "go test". only ever run "make test" to run the full test - suite, and examine the full output. diff --git a/Dockerfile b/Dockerfile index 6d4eb31..b769346 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,92 +1,76 @@ -# This file has no lint stage, deliberately. -# -# Linting lives in Dockerfile.lint, built by script/lint, and -# script/cibuild builds both. A lint stage here would have to either -# shell out to `make lint` -- which is now `docker build`, so -# docker-in-docker inside a BuildKit step with no daemon -- or call -# golangci-lint directly, which would mean a second, independently -# bumpable digest pin for the linter alongside the one in -# Dockerfile.lint. Two pins for one tool is the drift that -# https://git.eeqj.de/sneak/vaultik/issues/78 was filed over. See -# https://git.eeqj.de/sneak/vaultik/issues/113 for the ruling. -# -# Consequence, stated rather than left to be discovered: script/docker -# builds this file only and therefore does not lint. `make fmt-check` -# and `make test` still run here, so what a green build of this file -# means is "formatted, tested, and it compiles" -- the lint verdict -# comes from script/lint or script/cibuild. - -# Build stage -# golang:1.26.1-alpine, 2026-03-17 -FROM golang:1.26.1-alpine@sha256:2389ebfa5b7f43eeafbd6be0c3700cc46690ef842ad962f6c5bd6be49ed82039 AS builder - -# Build tooling: make, plus a C toolchain because `go test -race` needs cgo, -# and git, which derives the version below. The sqlite driver is pure Go -# (modernc.org/sqlite), so no sqlite library or CLI is required. -RUN apk add --no-cache make build-base git - +# Lint phase. The linter is invoked directly rather than through `make +# lint` or `script/lint`, which are themselves a docker build and would +# recurse into a daemon that does not exist in a build step. +# golangci/golangci-lint:v2.14.0, 2026-10-05 +FROM golangci/golangci-lint:v2.14.0@sha256:ad862ba6b3798cbe0fd9fd7408d498fd74fbd2623a92406b2fd3898faf0bf98f AS lint WORKDIR /src - -# Copy go mod files first for better layer caching COPY go.mod go.sum ./ RUN go mod download +COPY . . +# `golangci-lint run` silently ignores an unknown top-level key in +# .golangci.yml, such as a misspelt `linters:`; `config verify` fails on it. +RUN golangci-lint config verify --config .golangci.yml +RUN golangci-lint run --config .golangci.yml ./... -# Copy source code +# Test phase. -race needs cgo and so a C compiler, which the Debian Go +# image ships and the alpine one does not. +# golang:1.26.1 (Debian trixie), 2026-10-05 +FROM golang:1.26.1@sha256:cd78d88e00afadbedd272f977d375a6247455f3a4b1178f8ae8bbcb201743a8a AS test +WORKDIR /src +COPY go.mod go.sum ./ +RUN go mod download +COPY . . +RUN go test -timeout 90s -race -cover ./... || \ + { echo "--- Rerunning with -v for details ---"; \ + go test -timeout 90s -race -v ./...; exit 1; } + +# Build stage. Nothing is wanted from either phase above; the copies +# are what make BuildKit build them first, so this stage cannot run +# unless lint and test passed. +# golang:1.26.1-alpine, 2026-03-17 +FROM golang:1.26.1-alpine@sha256:2389ebfa5b7f43eeafbd6be0c3700cc46690ef842ad962f6c5bd6be49ed82039 AS builder +COPY --from=lint /src/go.sum /dev/null +COPY --from=test /src/go.sum /dev/null +RUN apk add --no-cache git +# A tar-stream context keeps the sender's file owners, which git refuses. +RUN git config --system --add safe.directory /src +WORKDIR /src +COPY go.mod go.sum ./ +RUN go mod download COPY . . -# Run the format check and the tests. -# -# CHECK_EPOCH must stay immediately above these RUNs. These layers are -# keyed on its value, so they are cache-eligible only for a value -# already built against this same tree. script/cibuild and script/docker -# each pass a fresh value on every invocation, which is what makes their -# green mean the checks really executed. -# -# The value is expanded into each check command rather than left to a -# bare declaration, so the cache miss does not depend on BuildKit's -# unreferenced-ARG handling staying as it is. It also puts the epoch in -# the build log, where a reader can see the layer was keyed fresh. -# -# A build that passes no CHECK_EPOCH, such as a plain `docker build .`, -# keys these layers on the empty string, so rebuilding an unchanged -# checkout replays them from cache and runs nothing. Only the scripts' -# builds mean the checks executed. -# -# Everything above this line (apk, go.mod, `go mod download`) is -# deliberately outside the busted range and keeps caching. -ARG CHECK_EPOCH -RUN echo "check epoch: ${CHECK_EPOCH}" && make fmt-check -RUN echo "check epoch: ${CHECK_EPOCH}" && make test - -# Version, commit and build date: the build args when given (script/docker -# and script/cibuild pass the ones they compute on the host), otherwise -# derived from the .git in the build context. The version is then `git -# describe --tags --always`: the tag on a tagged commit, tag-N-gHASH after -# one, the short commit when no tag is reachable. A context that carries -# .git and still yields no version fails the build; one without .git, as -# from a source tarball, stamps "dev" and an "unknown" commit and date. -# -# These ARGs sit here, after the checks, rather than at the top of the -# stage: every commit changes their values, and a value change -# invalidates all layers below the ARG. Declared up top they would bust -# `go mod download`; here they only rekey this build layer, which the -# COPY of the sources above already rebuilds on any change anyway. +# The VERSION build arg when one is given, otherwise +# `git describe --tags --always` on the .git in the build context. The +# commit and its date always come from that .git. With .git present, a +# version that is still empty, dev or unknown, or a commit or date that +# is unknown, fails the build: git is missing or could not read the +# checkout, as when .git is a file pointing outside the context. A +# context without .git, such as a source export, stamps "dev" and an +# "unknown" commit and date. ARG VERSION -ARG COMMIT -ARG COMMIT_DATE - -# Build (pure Go, no CGO required since we use modernc.org/sqlite) -RUN version="${VERSION:-$(git describe --tags --always || echo dev)}"; \ - if [ -e .git ] && { [ -z "$version" ] || [ "$version" = dev ] || \ - [ "$version" = unknown ]; }; then \ - echo "the build context carries .git but yields no version" >&2; \ - exit 1; \ +RUN VERSION="${VERSION:-$(git describe --tags --always || echo dev)}"; \ + commit="$(git rev-parse HEAD || echo unknown)"; \ + commit_date="$(git show -s --format=%cs HEAD || echo unknown)"; \ + if [ -e .git ]; then \ + case "$VERSION" in ""|dev|unknown) \ + echo "version is '$VERSION' although .git is present" >&2; \ + exit 1 ;; \ + esac; \ + if [ "$commit" = unknown ] || [ "$commit_date" = unknown ]; then \ + echo "commit is '$commit' and its date '$commit_date'" \ + "although .git is present" >&2; \ + exit 1; \ + fi; \ fi; \ - commit="${COMMIT:-$(git rev-parse HEAD || echo unknown)}"; \ - commit_date="${COMMIT_DATE:-$(git show -s --format=%cs HEAD || echo unknown)}"; \ - CGO_ENABLED=0 go build -ldflags "-X 'sneak.berlin/go/vaultik/internal/globals.Version=${version}' -X 'sneak.berlin/go/vaultik/internal/globals.Commit=${commit}' -X 'sneak.berlin/go/vaultik/internal/globals.CommitDate=${commit_date}'" -o /vaultik ./cmd/vaultik + globals=sneak.berlin/go/vaultik/internal/globals; \ + CGO_ENABLED=0 go build -trimpath \ + -ldflags="-s -w -X ${globals}.Version=${VERSION} \ + -X ${globals}.Commit=${commit} \ + -X ${globals}.CommitDate=${commit_date}" \ + -o /vaultik ./cmd/vaultik -# Runtime stage +# Runtime stage, and the last one: a plain `docker build .` builds this +# stage's chain and nothing else. # alpine:3.21, 2026-02-25 FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709 diff --git a/Dockerfile.lint b/Dockerfile.lint deleted file mode 100644 index 4e39700..0000000 --- a/Dockerfile.lint +++ /dev/null @@ -1,104 +0,0 @@ -# Lint image. -# -# Every lint run in this repo happens inside this image, invoked through -# script/lint, and linting is a BUILD STEP rather than a container -# command: a successful build of this file IS a clean lint. That shape -# also works where the docker daemon is remote and bind mounts are -# impossible, which `docker run` against a mounted worktree does not. -# -# This FROM line is the single source of truth for the linter version in -# this repo. Nothing else pins golangci-lint: the product Dockerfile has -# no lint stage, deliberately, so there is no second digest to bump and -# no pair of pins that can drift apart. Bump the tag AND the digest here -# and nowhere else. -# -# Note for readers coming from REPO_POLICIES.md: that document still -# describes the older pattern, a lint stage inside the product -# Dockerfile wired up with `COPY --from=lint /src/go.sum /dev/null`. -# That pattern is superseded here by the owner's ruling recorded in -# https://git.eeqj.de/sneak/vaultik/issues/113 -- lint runs in its own -# image, per run, with its own cache and its own lock, which is what -# makes concurrent runs on one host safe. The policy text is org-wide -# and is being amended separately; this file is what this repo does. -# -# golangci/golangci-lint:v2.12.2, 2026-08-10 -FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 - -WORKDIR /src - -# Copy the dependency manifests first so the module download layer stays -# cached until they change. Everything above the ARG below is cacheable -# on purpose; a cold module download on every lint would make the inner -# loop unusable and buys nothing, because it is not what the gate is -# asserting. -COPY go.mod go.sum ./ -RUN go mod download - -COPY . . - -# Force the check layers to execute on every invocation. -# -# CHECK_EPOCH must stay immediately above the RUNs below. Those layers -# are keyed on its value, so they are cache-eligible only for a value -# already built against this same tree; script/lint and script/cibuild -# each pass a fresh value on every invocation, which is what makes their -# green mean the linter really ran. Without it, `docker build -f -# Dockerfile.lint .` on an unchanged tree exits 0 in well under a second -# having linted nothing. -# -# The value is expanded into each check command itself rather than left -# to a bare declaration, so the cache miss does not depend on BuildKit's -# unreferenced-ARG handling staying as it is. It also puts the epoch in -# the build log, where a reader can see the layer was keyed fresh. -# -# The guard is what makes a build that omits --build-arg fail instead of -# lie. An unset ARG is an empty string, and an empty string is a -# perfectly stable cache key: without the guard the first such build -# lints and every one after it on an unchanged tree replays this layer, -# executes nothing, and still exits 0. Failed steps are never cached, so -# the guard fails on EVERY invocation rather than once. Do not give -# CHECK_EPOCH a default value; a default would satisfy the guard with a -# constant and restore the hole. -ARG CHECK_EPOCH -RUN [ -n "$CHECK_EPOCH" ] || exit 1 - -# Validate .golangci.yml before linting with it. -# -# This is not belt-and-braces; it closes a hole that `golangci-lint run` -# leaves wide open. `run` rejects YAML it cannot PARSE, but it silently -# IGNORES an unknown top-level KEY. Renaming `linters:` to `linterz:` -- -# one character -- discards `default: all`, the whole disable list and -# every threshold, leaves only golangci-lint's small default linter set -# running, and exits 0 reporting `0 issues.` on a tree the real config -# fails. Demonstrated on this repo at this pin, recorded on -# https://git.eeqj.de/sneak/vaultik/pulls/114: with a planted -# over-length line, `script/lint` exits 1 naming the `revive` finding -# with `linters:` and exits 0 with `linterz:`. A set-but-ineffective -# config quietly falling back to defaults is precisely the false-green -# class this gate exists to eliminate, so it must not sit in the gate's -# own configuration. -# -# `config verify` catches it, and it does so OFFLINE at this pinned -# version -- verified, not assumed. Under `docker run --network none` -# against the pinned digest it exits 0 on this repo's config and exits 3 -# on the `linterz:` variant with `additional properties 'linterz' not -# allowed`. An earlier revision of this file asserted the opposite, that -# the schema is fetched over live HTTPS from an unpinned URL, and used -# that to justify omitting this line. That claim was false at v2.12.2; -# the schema is embedded. If a future bump reintroduces a network fetch -# the failure is loud and this comment is where to record it. -# -# It is keyed on CHECK_EPOCH, like the lint run below, so it executes on -# every invocation. Content-addressing alone would arguably be enough -- -# .golangci.yml arrives through `COPY . .`, so a cache hit here implies -# a byte-identical config was validated when the layer really ran. That -# argument is exactly the one that would also excuse caching the lint -# layer, and this repo has ruled it insufficient: a cached check layer -# checks nothing, and the cost of being wrong is silent. Forcing it costs -# milliseconds and puts the epoch in the log, where a reader can see that -# this validation ran rather than being replayed. -RUN echo "check epoch: ${CHECK_EPOCH}" && \ - golangci-lint config verify --config .golangci.yml - -RUN echo "check epoch: ${CHECK_EPOCH}" && \ - golangci-lint run --config .golangci.yml ./... diff --git a/Makefile b/Makefile index d0e60c2..f694dd5 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,8 @@ .PHONY: all bootstrap setup check test lint lint-fix fmt fmt-check build clean deps test-coverage local install release release-snapshot docker hooks +# Where script/bootstrap installs Go when the host has none. +export PATH := $(PATH):$(CURDIR)/.tool/go/bin + # Version number, derived from git by script/version (`git describe # --tags --always --dirty`). This used to be a hardcoded # constant, which meant every local build claimed to be a release that @@ -40,9 +43,10 @@ setup: check: @script/check -# Run tests only. This runs the ENTIRE suite -- there is no separate -# integration target and no build-tagged subset held back. In -# particular internal/vaultik/integration_test.go, which does full +# Run tests only, by building the test phase of the Dockerfile. This +# runs the ENTIRE suite -- there is no separate integration target and +# no build-tagged subset held back. In particular +# internal/vaultik/integration_test.go, which does full # chunk -> pack -> encrypt -> upload -> restore round-trips, runs here. # A `test-integration` target used to exist and was removed: no file in # the repo carried a build tag, so `-tags=integration` selected nothing @@ -87,17 +91,17 @@ clean: go clean # Install dependencies. The linter is deliberately not installed here: -# script/lint lints by building Dockerfile.lint, whose FROM line is the -# single source of truth for the linter version. A second, separately -# pinned copy on PATH could drift from it and make a local `make lint` -# disagree with CI. +# script/lint lints by building the lint phase of the Dockerfile, whose +# FROM line is the single source of truth for the linter version. A +# second, separately pinned copy on PATH could drift from it and make a +# local `make lint` disagree with CI. deps: go mod download -# Run tests with coverage. -count=1 for the same reason script/test -# uses it: without it an unchanged package is served from Go's test -# result cache, and a coverage profile assembled from cached results -# describes a run that did not happen. +# Run tests with coverage, on the host. -count=1 because without it an +# unchanged package is served from Go's test result cache, and a +# coverage profile assembled from cached results describes a run that +# did not happen. test-coverage: go test -v -count=1 -coverprofile=coverage.out ./... go tool cover -html=coverage.out -o coverage.html diff --git a/README.md b/README.md index ecd7622..71011d1 100644 --- a/README.md +++ b/README.md @@ -728,12 +728,11 @@ regardless of color setting (emoji are not color). ## requirements * Go 1.26 or later -* Docker, with a reachable daemon, to lint, check, or commit: - `script/lint` lints by building `Dockerfile.lint`, which runs the - digest-pinned `golangci-lint` image as a build step, and `make check` - and the pre-commit hook both run it. A `golangci-lint` installed on - `PATH` is not a substitute and is never used on a host, whatever its - version. +* Docker, with a reachable daemon, to test, lint, check, or commit: + `script/test` and `script/lint` build the `test` and `lint` phases of + the `Dockerfile`, and `make check` and the pre-commit hook both run + them. A `golangci-lint` installed on `PATH` is not a substitute and is + never used on a host, whatever its version. * S3-compatible object storage (or local filesystem, or rclone remote) ## development workflow @@ -764,17 +763,20 @@ standard: normalized scripts in `script/` are the entrypoints for the development workflow, and the Makefile targets are thin shims that call them. We provide: -* `script/bootstrap` — install all development dependencies (go, Go - module download). It deliberately does not install `golangci-lint`; - see `script/lint` below. +* `script/bootstrap` — install all development dependencies (Go, Go + module download). A host without Go gets the `go.mod` version through + `script/install-go`, in `.tool/go`, which `script/bootstrap` itself, + the `Makefile`, `script/fmt`, `script/fmt-check`, `script/precommit` + and `script/release` add to their `PATH`. It + deliberately does not install `golangci-lint`; see `script/lint` + below. * `script/setup` — make a fresh clone ready for development: runs `script/bootstrap`, then `script/install-precommit` * `script/projectname` — print the project name (used for the Docker image tag) * `script/version` — print the version string to bake into the binary. - The `Makefile`'s `LDFLAGS` call this, and `script/docker` and - `script/cibuild` pass its output to the image build. See - [releasing](#releasing) for the rules. + The `Makefile`'s `LDFLAGS` call this. See [releasing](#releasing) for + the rules. * `script/install-goreleaser` — install the pinned `goreleaser` into `.tool/bin` from a sha256-verified release archive. Idempotent, and called by `script/bootstrap`; the release workflow calls it directly @@ -782,102 +784,59 @@ them. We provide: `script/bootstrap` insists on. * `script/install-go` — install the Go toolchain named by `go.mod`'s `go` directive into `.tool/go` from a sha256-verified `go.dev` - archive, and put it on `PATH`. Idempotent. Called only by the release - workflow, which needs a host Go for `goreleaser` to shell out to; - nothing else on the release runner does. `actions/setup-go` is not - used because it verifies the downloaded toolchain against no value in - this repo. Bumping Go edits `go.mod`, the checksum in this script, and - the `Dockerfile` `golang` digest together. + archive, for Linux or macOS on amd64 or arm64. Idempotent. On a CI + runner it also puts `.tool/go/bin` on `PATH` for the steps that + follow. Called by the release workflow, which needs a host Go for + `goreleaser` to shell out to, and by `script/bootstrap` on a host + without Go. `actions/setup-go` is not used because it verifies the + downloaded toolchain against no value in this repo. Bumping Go edits + `go.mod`, the checksums in this script, and the two `golang` digests + in the `Dockerfile` together. * `script/release` — cross-compile and publish the release artifacts with the pinned `goreleaser`. Refuses a `goreleaser` on `PATH` whose version is not the pinned one, on the same reasoning as `script/lint`. * `script/release-snapshot` — the same build with no publishing and no tagging, into `./dist` -* `script/test` — run the test suite (verbose rerun on failure). This - runs *everything*: there is no separate integration target and no - build-tagged subset held back, so the full round-trip tests in - `internal/vaultik/integration_test.go` run on every invocation. It - passes `-count=1`, which disables Go's test result cache. That is - deliberate and it is not free: on this repo's suite it costs about 11 - seconds on every repeat run (measured, back to back: 0.4s cached - versus 11.6s with `-count=1`). That is the price of the run meaning - anything, because without it an unchanged package prints - `ok (cached)`, which is indistinguishable from a package that - really ran, so the whole suite can report a full set of `ok` lines in - under half a second having executed nothing. The `-timeout` is a hang - backstop rather than a performance budget — it applies per test binary - to test execution only, not to compilation — and is set well above the - slowest package's measured runtime. Its 120s value deliberately - diverges from the 30s `REPO_POLICIES.md` mandates; the reasoning is in - the comment in the script, and issue #101 proposes amending the policy - text. -* `script/lint` — lint by building `Dockerfile.lint`, which runs - `golangci-lint run --config .golangci.yml ./...` as a build step - inside the digest-pinned `golangci-lint` image, so a successful build - *is* a clean lint. Nothing lints on the host, at any version, ever; - the script requires Docker and fails loudly rather than falling back - to a `golangci-lint` on `PATH`. That `FROM` line is the single source - of truth for the linter version — bump it there and nowhere else. - - It takes no arguments, because a build step has no command line to - pass flags to, and it passes a fresh `--build-arg CHECK_EPOCH` on - every invocation so the lint layer cannot be replayed from cache (see - `script/cibuild` below for what that mechanism defends against). To - watch the linter execute, run it as - `BUILDKIT_PROGRESS=plain script/lint` and check that the lint layer - says `RUN … golangci-lint` rather than `CACHED`. - - One container per run means one lint cache and one `golangci-lint` - lock per run, both private to it and discarded with it, so concurrent - runs on one host cannot contaminate or block each other. +* `script/test` — run the test suite by building the `test` phase of + the `Dockerfile` (verbose rerun on failure). This runs *everything*: + there is no separate integration target and no build-tagged subset + held back, so the full round-trip tests in + `internal/vaultik/integration_test.go` run on every invocation. The + 90s `-timeout` is a hang backstop rather than a performance budget; it + applies per test binary, to test execution only. +* `script/lint` — lint by building the `lint` phase of the + `Dockerfile`, which runs `golangci-lint config verify` and then + `golangci-lint run --config .golangci.yml ./...` as build steps in + the digest-pinned `golangci-lint` image. Nothing lints on the host. + That `FROM` line is the only pin of the linter version, and it changes + in the same commit as a re-vendored `.golangci.yml`. * `script/lint-fix` — apply the linter's autofixes (rewrites files), - using the same pinned image, parsed out of `Dockerfile.lint`. It - cannot be a build step, because fixes have to land in the worktree, so - it bind-mounts the tree into a `docker run` and therefore needs a - *local* daemon. It is a developer convenience and never a gate: no - gate reads its exit status. Run `make lint` afterwards to find out - whether the tree is clean. + using the same pinned image, parsed out of the `lint` phase's `FROM` + line. It cannot be a build step, because fixes have to land in the + worktree, so it bind-mounts the tree into a `docker run` and therefore + needs a *local* daemon. It is a developer convenience and never a + gate: no gate reads its exit status. Run `make lint` afterwards to + find out whether the tree is clean. * `script/fmt` — format all code (writes) -* `script/fmt-check` — check formatting (read-only) +* `script/fmt-check` — check formatting (read-only). It runs `gofmt` on + the host, over every Go file outside `.tool`. * `script/check` — run `script/test`, `script/lint`, and - `script/fmt-check`. This is authoritative *because* `script/lint` - builds `Dockerfile.lint`: a local `make check` and CI cannot disagree - about lint findings. + `script/fmt-check`. * `script/docker` — build the Docker image tagged via - `script/projectname`. Passes a fresh `--build-arg CHECK_EPOCH` for the - same reason `script/cibuild` does, so a local image build cannot be - green on checks it replayed from cache. It builds the *product* image - only, and the product `Dockerfile` has no lint stage, so it does not - lint: a green here means formatted, tested, and it compiles. -* `script/cibuild` — CI entrypoint, and the full gate. Two builds, in - order: `Dockerfile.lint` (the linter, as a build step) and then - `Dockerfile` (`make fmt-check` and `make test` in its builder stage, - then the product image). Either failing fails the script. It runs the - checks in the same containers CI does, from a clean copy of the tree, - so it also catches anything that depends on host state. - `.gitea/workflows/check.yml` runs it on every push to `main` and - `next` and on every pull request against either. + `script/projectname`, stamped with the version + `git describe --tags --always --dirty` gives on the host, or `unknown` + outside a git checkout. The image's build stage depends on the `lint` + and `test` phases, so this lints and tests too. +* `script/cibuild` — CI entrypoint: runs `script/bootstrap`, + `script/check`, and then the same image build as `script/docker`. + `.gitea/workflows/check.yml` runs it on every push. - It passes a fresh `--build-arg CHECK_EPOCH` to each build, unique per - invocation, which both files declare immediately above their check - `RUN`s and expand into each check command. Those layers are keyed on - that value, so a new value re-runs them even on a byte-identical tree, - and a green from this script means the checks executed. Dependency and - module layers sit above the `ARG` and still cache, so a build is not - cold. - - A `docker build -f Dockerfile.lint .` that supplies no `CHECK_EPOCH` - fails rather than lying. An unset `ARG` is an empty string and an - empty string is a stable cache key, so without a guard such a build - would serve the lint layer from cache, execute nothing, and still exit - 0. `Dockerfile.lint` therefore asserts the value is non-empty before - running anything, and because failed steps are never cached that - assertion fires on every invocation rather than once. The product - `Dockerfile` has no such guard, because a plain `docker build .` must - succeed: without `CHECK_EPOCH`, rebuilding an unchanged checkout - replays its check layers from cache. Use `script/lint`, - `script/docker` or `script/cibuild`, which pass the arg, when the - checks must run. + Every `docker build` in these scripts passes `--no-cache`, because on + an unchanged tree a cached check layer is replayed without running and + the build still exits 0. A plain `docker build .` is therefore no + evidence that the checks ran. The cost is that `script/cibuild` runs + the `lint` and `test` phases twice: once in `script/check` and again + in the image build. * `script/precommit` — pre-commit gate: `go mod tidy` + `go fmt` (must not change files), then `script/check` * `script/install-precommit` — install the git pre-commit hook that @@ -889,8 +848,8 @@ them. We provide: The version a binary reports comes from git, not from a constant in a file. It is `git describe --tags --always --dirty`, which -`script/version` runs for the `Makefile`, `script/docker` and -`script/cibuild`: +`script/version` runs for the `Makefile`, and `script/docker` and +`script/cibuild` run themselves: * `HEAD` is exactly on a tag → that tag, such as `v1.0.0`. * a commit after a tag → `--g`. @@ -901,15 +860,17 @@ file. It is `git describe --tags --always --dirty`, which A `docker build .` of a clone, with no build arguments, runs the same `git describe` (without `--dirty`) on the `.git` in its build context, so it stamps the same value for a clean commit; the build fails if the -context carries `.git` and no version comes out. A binary built without -git metadata reports `dev`. +context carries `.git` and no version, commit or commit date comes out. +A binary built without +git metadata reports `dev`, or `unknown` when `script/docker` or +`script/cibuild` built it outside a git checkout. `goreleaser` stamps a release binary with the tag minus its leading `v`, so the tag `v1.0.0` produces `vaultik 1.0.0`, matching the archive name `vaultik_1.0.0_linux_amd64.tar.gz`. `goreleaser --snapshot` stamps `dev-<12 chars of the commit sha>` rather than inventing the next patch number. `vaultik version` calls a build a development build when its -version is `dev`, `dev-`, the short commit sha or +version is `dev`, `unknown`, `dev-`, the short commit sha or `--g`, with or without `-dirty`; only a plain tag, such as `v1.0.0` or `1.0.0`, is a release. If `script/version` cannot be run at all, `make` stops with an error instead of building an diff --git a/REPO_POLICIES.md b/REPO_POLICIES.md index bc2f161..20382d1 100644 --- a/REPO_POLICIES.md +++ b/REPO_POLICIES.md @@ -1,6 +1,6 @@ --- title: Repository Policies -last_modified: 2026-07-06 +last_modified: 2026-10-04 --- This document covers repository structure, tooling, and workflow standards. Code @@ -60,17 +60,28 @@ style conventions are in separate documents: prerequisite since nvm requires bash. yarn is then pinned via `corepack prepare yarn@ --activate`. Never install "latest" or "lts"; always exact versions. `script/cibuild` runs the CI build: it changes to the - repo root and runs `docker build .`; the Gitea workflow calls it. Four further - scripts are our own extensions to the standard: `script/check` runs - `script/test`, `script/lint`, and `script/fmt-check`; `script/precommit` is - what the git pre-commit hook runs, and it calls `script/check`; - `script/install-precommit` installs the git pre-commit hook (the `make hooks` - target shims to it); and `script/projectname` (literally that filename) simply - outputs the project's name. Scripts that need the name call - `script/projectname` — e.g. `script/docker` assembles its image tag from it — - so those scripts stay byte-identical across all repos. Repo-type-specific - pre-commit extras (e.g. `go mod tidy` verification in Go repos) belong in - `script/precommit`, not in the hook itself. Model scripts are at + repo root, runs `script/bootstrap`, runs `script/check`, and builds the image + with the version; the Gitea workflow calls it. **`script/cibuild` runs + `script/bootstrap` first**, because the workflow checks out the repo and runs + nothing else, while `script/fmt-check` runs the formatter on the host: on a + pristine checkout with nothing installed the run dies there, after the + containerised gates have passed. **The bootstrap alone is not enough**: + `script/bootstrap` installs node and yarn under nvm and leaves neither on the + `PATH` of the shell that called it, so a bare `yarn` still exits 127. The host + entrypoints that need yarn — `script/fmt` and `script/fmt-check` — therefore + source nvm for the pinned node version before invoking it, exactly as + `script/bootstrap`'s own install step does. A runner carrying nothing but + docker and git then gets through `script/check`. Four further scripts are our + own extensions to the standard: `script/check` runs `script/test`, + `script/lint` and `script/fmt-check`; `script/precommit` is what the git + pre-commit hook runs, and it calls `script/check`; `script/install-precommit` + installs the git pre-commit hook (the `make hooks` target shims to it); and + `script/projectname` (literally that filename) simply outputs the project's + name. Scripts that need the name call `script/projectname` — e.g. + `script/docker` assembles its image tag from it — so those scripts stay + byte-identical across all repos. Repo-type-specific pre-commit extras (e.g. + `go mod tidy` verification in Go repos) belong in `script/precommit`, not in + the hook itself. Model scripts are at `https://git.eeqj.de/sneak/prompts/raw/branch/main/script/`. The README must document the provided scripts in an **Entrypoints** section (see the README requirements below). @@ -89,87 +100,198 @@ style conventions are in separate documents: contributor should be able to understand the entire development workflow by reading the Makefile. -- Every repo should have a `Dockerfile`. All Dockerfiles must run `make check` - as a build step so the build fails if the branch is not green. For non-server - repos, the Dockerfile should bring up a development environment and run - `make check`. For server repos, `make check` should run as an early build - stage before the final image is assembled. Dockerfiles install development - prerequisites by running `script/bootstrap` rather than duplicating installs - inline; COPY `script/` and the dependency manifests (`package.json` + - `yarn.lock`, `go.mod` + `go.sum`, etc.) before running it so the bootstrap - layer stays cached until dependencies change. +- Every repo should have a `Dockerfile`, and it carries the repo's gates: a + `lint` phase and a `test` phase, with the final stage depending on both so the + image cannot be built unless they pass. For non-server repos the final stage + brings up a development environment; for server repos it is the runtime image. + The gate phases and the build stage start from their pinned base images and + install what those images lack either inline, as the canonical Go `Dockerfile` + below does for `git`, or by running `script/bootstrap`, as the `prompts` + repo's own `Dockerfile` does for its yarn packages. The development + environment stage installs development prerequisites by running + `script/bootstrap` rather than duplicating its installs inline. A stage that + runs `script/bootstrap` COPYs `script/` and the dependency manifests + (`package.json` + `yarn.lock`, `go.mod` + `go.sum`, etc.) before running it. -- **Dockerfiles must use a separate lint stage for fail-fast feedback.** Go - repos use a multistage build where linting runs in an independent stage based - on the `golangci/golangci-lint` image (pinned by hash). This stage runs - `make fmt-check` and `make lint` before the full build begins. The build stage - then declares an explicit dependency on the lint stage via - `COPY --from=lint /src/go.sum /dev/null`, which forces BuildKit to complete - linting before proceeding to compilation and tests. This ensures lint failures - surface in seconds rather than minutes, without blocking on dependency - download or compilation in the build stage. +- **Linting and testing run in Docker, as phases of the `Dockerfile`.** There is + no separate lint file. `script/lint` and `script/test` each build one phase + and nothing else: - The standard pattern for a Go repo Dockerfile is: + ```sh + docker build --no-cache --target lint -t "$(script/projectname)-lint" . + docker build --no-cache --target test -t "$(script/projectname)-test" . + ``` + + **A stage that is not the last one in the file is built only when the final + stage's chain depends on it, or when `--target` names it.** That is why the + two gates are always invoked by name here, and why the final stage carries a + `COPY --from=` of a harmless file from each of them: without that edge a + plain `docker build .` builds the last stage alone and exits 0 having linted + and tested nothing. + + **Every `docker build` in `script/` is tagged**, here and in + `script/cibuild` and `script/docker`. An untagged build leaves a dangling + image behind on every invocation, on every developer host and every CI + runner; a tagged one replaces the previous image. + + Inside a phase the tool is invoked directly — `golangci-lint`, `go test`, + `eslint`, `prettier` — never through `make lint` or `script/test`, which are + themselves a `docker build` and would recurse into a daemon that does not + exist in a build step. Formatting is the exception and stays on the host: + `script/fmt` writes the working tree, and `script/fmt-check` is its + read-only twin. + + **No lint verdict may come from a host invocation of the linter.** On a + shared host golangci-lint reads a result cache keyed on file content rather + than location, so a second checkout of the same content is served the first + one's findings, and a host-global lock in `$TMPDIR` makes concurrent runs + exit non-zero with `parallel golangci-lint is running` — a status a caller + cannot tell from real findings. Both have produced wrong verdicts in this + org, in both directions. A container has its own cache, its own `TMPDIR` and + a digest-pinned binary, so neither is reachable. + +- **Any build that runs checks is built with `--no-cache`.** Docker invalidates + a `COPY` layer only when the copied content changes, so on an unchanged tree + the check `RUN` is served from cache, nothing executes, and the build still + exits 0. Every `docker build` in `script/` therefore passes `--no-cache`: + `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. 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 + hash), so lint failures surface in seconds rather than after a full compile, + and the test phase is based on the Debian Go image. The canonical Go repo + `Dockerfile`: ```dockerfile - # Lint stage — fast feedback on formatting and lint issues + # Lint phase # golangci/golangci-lint:v2.x.x, YYYY-MM-DD FROM golangci/golangci-lint@sha256:... AS lint WORKDIR /src COPY go.mod go.sum ./ RUN go mod download COPY . . - RUN make fmt-check - RUN make lint + RUN golangci-lint run --config .golangci.yml ./... - # Build stage - # golang:1.x-alpine, YYYY-MM-DD - FROM golang@sha256:... AS builder + # Test phase. -race needs cgo and so a C compiler, which the Debian Go + # image ships and the alpine one does not. + # golang:1.x, YYYY-MM-DD + FROM golang@sha256:... AS test WORKDIR /src - - # Force BuildKit to run the lint stage before proceeding - COPY --from=lint /src/go.sum /dev/null - COPY go.mod go.sum ./ RUN go mod download COPY . . - RUN make test + RUN go test -timeout 90s -race -cover ./... || \ + { echo "--- Rerunning with -v for details ---"; \ + go test -timeout 90s -race -v ./...; exit 1; } - ARG VERSION=dev - RUN CGO_ENABLED=0 go build -trimpath \ - -ldflags="-s -w -X main.Version=${VERSION}" \ - -o /app ./cmd/app/ + # Build stage. Nothing is wanted from either phase above; the copies + # are what make BuildKit build them first, so this stage cannot run + # unless lint and test passed. + # golang:1.x-alpine, YYYY-MM-DD + FROM golang@sha256:... AS builder + COPY --from=lint /src/go.sum /dev/null + COPY --from=test /src/go.sum /dev/null + RUN apk add --no-cache git + # A tar-stream context keeps the sender's file owners, which git refuses. + RUN git config --system --add safe.directory /src + WORKDIR /src + COPY go.mod go.sum ./ + RUN go mod download + COPY . . - # Runtime stage + # The VERSION build arg when one is given, otherwise + # `git describe --tags --always` on the .git in the build context. With + # .git present, a version that is still empty, dev or unknown fails the + # build: git is missing or could not read the checkout. + ARG VERSION + RUN VERSION="${VERSION:-$(git describe --tags --always)}"; \ + if [ -e .git ]; then \ + case "$VERSION" in ""|dev|unknown) \ + echo "version is '$VERSION' although .git is present" >&2; \ + exit 1 ;; \ + esac; \ + fi; \ + CGO_ENABLED=0 go build -trimpath \ + -ldflags="-s -w -X main.Version=${VERSION}" \ + -o /app ./cmd/app/ + + # Runtime stage, and the last one FROM alpine@sha256:... COPY --from=builder /app /usr/local/bin/app ENTRYPOINT ["app"] ``` Key points: - - The lint stage uses the `golangci/golangci-lint` image directly (it - includes both Go and the linter), so there is no need to install the - linter separately. - - `COPY --from=lint /src/go.sum /dev/null` is a no-op file copy that creates - a stage dependency. BuildKit runs stages in parallel by default; without - this line, the build stage would not wait for lint to finish and a lint - failure might not fail the overall build. + - The lint phase uses the `golangci/golangci-lint` image directly (it has + both Go and the linter), so nothing needs installing. + - `COPY --from= /src/go.sum /dev/null` is a no-op copy whose only + purpose is the ordering edge. BuildKit runs stages in parallel by default, + and a stage nothing depends on is not built at all, so without these two + lines a red gate would not fail the build. + - Keep the runtime stage last, and if you add a stage after it, give it the + same two copies. A plain `docker build .` builds the last stage's chain + and nothing else. - If the project uses `//go:embed` directives that reference build artifacts - (e.g. a web frontend compiled in a separate stage), the lint stage must + (e.g. a web frontend compiled in a separate stage), the lint phase must create placeholder files so the embed directives resolve. Example: `RUN mkdir -p web/dist && touch web/dist/index.html web/dist/style.css`. - The lint stage should not depend on the actual build output — it exists to - fail fast. - - If the project requires CGO or system libraries for linting (e.g. - `vips-dev`), install them in the lint stage with `apk add`. - - The build stage runs `make test` after compilation setup. Tests run in the - build stage, not the lint stage, because they may require compiled - artifacts or heavier dependencies. + - If the project requires CGO or system libraries for linting, install them + in the lint phase. The `golangci/golangci-lint` image is Debian-based and + has no `apk`, so install with `apt-get` under the Debian package name + (`libvips-dev`, where alpine says `vips-dev`), and delete the package + lists in the same `RUN`, so the layer does not keep them: + + ```dockerfile + RUN apt-get update \ + && apt-get install -y --no-install-recommends libvips-dev \ + && rm -rf /var/lib/apt/lists/* + ``` + + - `.dockerignore` lets `.git` into the build context. It keeps out every git + `config` at any depth (`**/.git/config`, `**/.git/modules/**/config`): the + repository's own, each submodule's under `.git/modules/`, and that of a + submodule keeping its own `.git` directory. `git describe` does not need + them, and each can hold a credential: a password in a remote URL, or the + token the CI checkout step stores there. A submodule whose name has a + `config` segment (`config`, `deploy/config`, `config/lib`) loses its whole + git directory to `**/.git/modules/**/config`, and Go's version stamping + then fails the build: give it a name without that segment + (`git submodule add --name`). The stage that compiles has `git` (the + Debian Go image has it; an alpine one needs `apk add --no-cache git`) and + takes the version from the `VERSION` build argument when one is given, + otherwise from `git describe --tags --always`. That gives the tag on a + tagged commit; on a later commit, the tag, the number of commits since it + and the short commit (`v1.2.3-4-gabc1234`); and the short commit when no + tag is reachable. The stage that compiles also marks its working directory + safe for git (`git config --system --add safe.directory /src`): a context + sent as a tar stream keeps the sender's file owners, and git refuses a + checkout owned by another user, so the version would come out empty. + `ARG VERSION` has no default, and the build fails if the context carries + `.git` and the version still comes out empty, `dev` or `unknown`. A plain + `docker build .` with no build arguments must succeed; a Dockerfile that + refuses an empty build argument drops that refusal and keeps the argument. - Every repo should have a Gitea Actions workflow (`.gitea/workflows/`) that - runs `script/cibuild` (which runs `docker build .`) on push. Since the - Dockerfile already runs `make check`, a successful build implies all checks - pass. + runs `script/cibuild` on push, and checks out the repo as its only other step. + That script bootstraps, runs the gate phases, and then builds the image, so a + successful run means every check passed; a bare `docker build .` does not + carry the same guarantee, because its gate phases may come from the cache. The + image build is uncached and so runs the gate phases a second time. That is the + price of the rule above, and it is worth paying: the image that ships is built + from a run of its own gates rather than from a cache entry. A separate + workflow limited to `main` by a `branches` list under `on: push` cannot be + checked by review: to try a change to it, add the feature branch to that list + and push, then remove the branch from the list again before merging. Keep any + job in it that publishes behind `if: github.ref_name == 'main'`, so the run + from the feature branch publishes nothing. - Use platform-standard formatters: `black` for Python, `prettier` for JS/CSS/Markdown/HTML, `go fmt` for Go. Always use default configuration with @@ -189,14 +311,21 @@ style conventions are in separate documents: module under test to verify it compiles/parses. There is no excuse for `make test` to be a no-op. -- `make test` must complete in under 20 seconds. Add a 30-second timeout in the - Makefile. +- `make test` must complete in under 60 seconds. That is the hard cap, and a + suite that exceeds it fails. Under 20 seconds is the target. A suite between + 20 and 60 seconds is still green, but the overage must be filed as an + improvement bug against that repo. Add a 90-second timeout to the test + invocation (`go test -timeout 90s`). The backstop deliberately sits above the + hard cap so that it catches a genuinely hung test rather than a merely slow + one. -- **`make test` should use the conditional verbose rerun pattern.** Run tests - without `-v` (verbose) first. If tests fail, automatically rerun with `-v` to - show full output. This keeps CI logs and `docker build` output clean on - success (just package/suite summaries) while providing full diagnostic detail - on failure (every test case, every assertion). The general shell pattern: +- **The test command should use the conditional verbose rerun pattern.** Run + tests without `-v` (verbose) first. If tests fail, automatically rerun with + `-v` to show full output. This keeps CI logs and `docker build` output clean + on success (just package/suite summaries) while providing full diagnostic + detail on failure (every test case, every assertion). The command lives in the + `test` phase of the `Dockerfile`, since `script/test` builds that phase; the + Makefile form below is the same pattern for any repo-local invocation: ```makefile test: @@ -209,11 +338,26 @@ style conventions are in separate documents: ```makefile test: - @go test -timeout 30s -race -cover ./... || \ + @go test -count=1 -timeout 90s -race -cover ./... || \ { echo "--- Rerunning with -v for details ---"; \ - go test -timeout 30s -race -v ./...; exit 1; } + go test -count=1 -timeout 90s -race -v ./...; exit 1; } ``` + `-count=1` is required on both invocations: it defeats Go's test _result_ + cache, so neither run can report a stored pass in place of running the + tests. It leaves the build cache alone, so it costs the runtime of the suite + and no recompilation. + + That cache is Go's own, separate from Docker's layer cache. Go stores a + passing result in its cache directory (`GOCACHE`), and when the same tests + run again on unchanged code it prints that result, marked `(cached)`, + without running them. That matters on a developer's machine, where this + target runs and the directory lasts from one run to the next. The `test` + phase of the `Dockerfile` needs no `-count=1`: its base image holds no + result for this repo's tests and nothing before its `go test` step runs a + test, so there is nothing to replay. `--no-cache` (above) is what makes that + step run on an unchanged tree. + Python example: ```makefile @@ -239,10 +383,84 @@ style conventions are in separate documents: must be in `.gitignore`. No exceptions. - `.gitignore` should be comprehensive from the start: OS files (`.DS_Store`), - editor files (`.swp`, `*~`), language build artifacts, and `node_modules/`. - Fetch the standard `.gitignore` from - `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitignore` when setting up - a new repo. + editor files (`.swp`, `*~`), in-repo agent scratch directories (`.claude/`), + language build artifacts, and `node_modules/`. Fetch the standard `.gitignore` + from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitignore` when + setting up a new repo. These patterns are written to `.gitignore`'s own + semantics, in which an unanchored pattern already matches at every depth; they + are not a `.dockerignore` and must not be transplanted into one unmodified. + +- **`.dockerignore` does not use `.gitignore` semantics, and copying patterns + across unmodified leaves secrets in the build context.** Docker matches with + `moby/patternmatcher`: `filepath.Match` semantics plus a `**` extension, so + `*` does not cross `/` and a pattern without a leading `**/` is anchored at + the build-context root. A `.dockerignore` listing `.env`, `*.pem` and `*.key` + therefore excludes only the copies at the repository root, while `config/.env` + and `certs/server.key` still reach the context and can land in an image layer + — which is more dangerous than a short file with no secret patterns at all, + because it reads as solved and stops anyone looking. Give every + depth-independent pattern the `**/` prefix and leave only genuinely + root-anchored entries unprefixed: `.claude`, and the repo's own host-built + binary, written `/myapp` and never `**/myapp`, which would also match + `cmd/myapp/` and delete the package directory from the context. Matching is + case-sensitive, and an ALL-CAPS twin per pattern still misses `Server.Key`, so + secret names use character ranges — `**/*.[kK][eE][yY]`, `**/*.[pP][eE][mM]`, + and likewise for `.envrc` and the extensionless SSH keys. Where such a pattern + also catches something the build needs, re-include it with a negation + (`!docs/example.env`); deleting the pattern reopens the exposure for every + other file it covers. Fetch the standard `.dockerignore` from + `https://git.eeqj.de/sneak/prompts/raw/branch/main/.dockerignore` and extend + it with the repo's own artifacts. + +- **In-repo agent scratch belongs in both files, written to each file's own + semantics.** `.claude/` holds one worktree per in-flight agent — an entire + additional checkout of the repo — so under `COPY . .` the build context + inflates by a multiple of the repo and another session's unreviewed work can + be copied into an image layer. In `.gitignore` the entry is `.claude/`, + unanchored. In `.dockerignore` it is `.claude`, anchored and with **no** `**/` + prefix, because the prefixed form would also delete any nested directory of + that name from the build. Anchoring carries a known gap that the canonical + `.dockerignore` states in its own comment, since consuming repos receive the + file and not the tracker: the directory is created in the agent's working + directory, so a repo running agents in subdirectories still ships + `services/api/.claude/` and must add its own anchored entry there. + +- **A plain `docker build .` of a clone stamps the version that + `git describe --tags --always` gives**, derived from the `.git` in the build + context as the canonical `Dockerfile` above shows. Without its failure check, + a missing `git` or an unreadable checkout would leave `-X main.Version=` empty + and the build would still exit 0. `script/docker` and `script/cibuild` pass + the version they compute on the host; it takes precedence. They do this + byte-identically across repos: + + ```sh + # Own line: a failing command substitution inside an argument does not + # trip `set -e`, so the inline form degrades to an empty constant. + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build --no-cache \ + --build-arg VERSION="$version" \ + -t "$(script/projectname)" . + ``` + + `--always` makes an untagged repo yield an abbreviated commit hash rather + than failing, and the `[ -n "$version" ]` line is the single place the + fallback is applied — a live check that fires on a build from an export with + no `.git` and on a repository with no commits yet. Do not fold it into the + substitution as `|| echo unknown`, which makes the guard unreachable. The + Dockerfile's side is `ARG VERSION` in the stage that compiles, declared + there because `ARG` is stage-scoped; passing `VERSION` to a repo whose + Dockerfile declares no such `ARG` is ignored and costs nothing, which is why + the scripts stay byte-identical. One consequence for CI: the standard + checkout action clones shallow and fetches no tags, so a repo that embeds a + tag-derived version must set `fetch-depth: 0` on its checkout step. + +- **Verify `.dockerignore` by enumerating the image, not by reading the + patterns.** Plant files at the root _and_ at least two directories deep, build + a probe image that does `COPY . .`, and list what actually landed + (`docker run --rm --entrypoint find IMAGE /app`). The `transferring context` + size is not a substitute: a nested secret is a few bytes, and BuildKit + transfers only the delta from the previous build. - **No build artifacts in version control.** Code-derived data (compiled bundles, minified output, generated assets) must never be committed to the @@ -258,9 +476,56 @@ style conventions are in separate documents: - Make all changes on a feature branch. You can do whatever you want on a feature branch. -- `.golangci.yml` is standardized and must _NEVER_ be modified by an agent, only - manually by the user. Fetch from - `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml`. +- `.golangci.yml` is standardized. The vendored copy in a consuming repo must + _NEVER_ be modified by an agent: fetch it from + `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml` and keep it + byte-identical, so that no repo can quietly loosen its own linting. Linter + configuration changes are made to the canonical copy in the `prompts` repo and + reach consuming repos by re-vendoring; an agent may open a PR against + canonical, which only the user merges. One list is exempt from byte-identity, + because it cannot be written once for every repo: the `deny` list of the + `test-support` depguard rule, where a repo names its own test-support packages + by full import path. A repo adds entries there and changes nothing else, and a + re-vendor carries its entries forward. The canonical golangci-lint version is + v2.14.0 (released 2026-09-24), pinned as the digest of the lint phase's base + image + (`golangci/golangci-lint@sha256:ad862ba6b3798cbe0fd9fd7408d498fd74fbd2623a92406b2fd3898faf0bf98f`, + which reports `2.14.0 built with go1.27.0 from 114493f9`). A module's `go` + directive must not name a newer Go minor version than the one golangci-lint + was built with, or golangci-lint refuses to lint it: this release lints + `go 1.27.1` but not `go 1.28`. That digest is the only pin, since no repo + installs golangci-lint on the host. A repo sets the lint phase digest to the + one named here and re-vendors `.golangci.yml` in the same commit, whichever of + the two prompted the change: the canonical copy can name linters that an older + golangci-lint rejects, and a newer golangci-lint can add linters that + `default: all` switches on until the canonical copy disables them. + +- **`script/bootstrap` installs a pinned tool by comparing versions, never by + testing presence.** An `if ! command -v ; then install; fi` guard tests + `PATH` only, so on an already-provisioned machine the pin is inert and a + version bump is a silent no-op — while the Dockerfile, installing into a clean + image, gets the pinned version, so a local `make check` and `make docker` can + disagree about what the tool even is. The canonical form: + - compares the installed version against the pin over the **whole** version + token; a parser that stops at the first `-` reports `2.12.2` for a host + running `2.12.2-rc1` and skips the install; + - treats absent, non-zero, empty or unrecognised `--version` output as a + mismatch, so the failure direction is a redundant install and never a + skipped one; + - after installing, re-resolves the binary the way callers do — `hash -r`, + then through `PATH`, not through the directory the installer wrote to — + and fails naming the resolved path, since an install that a shadowing + binary hides succeeds while changing nothing any caller sees; + - is actually called, and prints the version on both success paths: a + function defined and never invoked has the same exit status and the same + empty output as one that worked. + + Keep it POSIX sh: no arrays, no `[[`, no `grep -P`. + + A Go tool a repo needs on the host is installed with `go install` pinned to + a commit hash (`go install @`). It is never tracked as + a `go.mod` tool dependency or through a `tools.go` file, either of which + pulls the tool's own dependencies into the repo's `go.mod` and `go.sum`. - When pinning images or packages by hash, add a comment above the reference with the version and date (YYYY-MM-DD). @@ -374,12 +639,14 @@ style conventions are in separate documents: settings. - Avoid putting files in the repo root unless necessary. Root should contain - only project-level config files (`README.md`, `Makefile`, `Dockerfile`, - `LICENSE`, `.gitignore`, `.editorconfig`, `REPO_POLICIES.md`, and - language-specific config). Everything else goes in a subdirectory. Canonical - subdirectory names: + only project-level config files (`README.md`, `AGENTS.md`, `Makefile`, + `Dockerfile`, `LICENSE`, `.gitignore`, `.editorconfig`, `REPO_POLICIES.md`, + and language-specific config). Everything else goes in a subdirectory. + Canonical subdirectory names: - `bin/` — executable scripts and tools - - `cmd/` — Go command entrypoints + - `cmd/` — Go command entrypoints; thin only: one `main.go` per binary whose + body is a single call into `internal/` or `pkg/`, no project logic in + `cmd/` - `configs/` — configuration templates and examples - `deploy/` — deployment manifests (k8s, compose, terraform) - `docs/` — documentation and markdown (README.md stays in root) @@ -406,3 +673,7 @@ style conventions are in separate documents: - Go: `go.mod`, `go.sum`, `.golangci.yml` - JS: `package.json`, `yarn.lock`, `.prettierrc`, `.prettierignore` - Python: `pyproject.toml` + +- Guidance for coding agents lives in one `AGENTS.md` at the repository root. It + is never committed under a file or directory named after one agent tool, such + as `CLAUDE.md` or `.claude/`, and never split into separate memory files. diff --git a/TODO.md b/TODO.md index 2336169..f6eaa18 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,15 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-06: Re-vendored the canonical files from `sneak/prompts` at + `dd4027b` ([issue #213](https://git.eeqj.de/sneak/vaultik/issues/213)). + Linting and testing are now the `lint` and `test` phases of the + `Dockerfile`, and the build stage depends on both. `Dockerfile.lint`, + `CHECK_EPOCH` and the tests that checked them are gone; every + `docker build` in `script/` passes `--no-cache` instead. golangci-lint + is v2.14.0, with its new findings fixed in the code, and the rules in + `CLAUDE.md` now live in `AGENTS.md`. + - 2026-10-06: Made the local index actually run in WAL mode with a busy timeout ([issue #217](https://git.eeqj.de/sneak/vaultik/issues/217)). The connection settings were written in a form the SQLite driver diff --git a/cmd/vaultik/dockerversion_test.go b/cmd/vaultik/dockerversion_test.go index d1ee2df..6ae7b92 100644 --- a/cmd/vaultik/dockerversion_test.go +++ b/cmd/vaultik/dockerversion_test.go @@ -12,48 +12,44 @@ import ( // #75). The failure it protects against is silent: the image still // builds and runs, but `vaultik version` inside it reports "commit: // unknown" or a version of "dev", so an operator cannot tell which -// source produced a given backup. The build takes the values as build -// args, which script/docker computes on the host, and otherwise derives -// them from the .git in its context. +// source produced a given backup. The build takes the version as a +// build arg, which script/docker computes on the host, and otherwise +// derives it from the .git in its context; the commit and its date +// always come from that .git. // -// These are parses of the committed files, for the same reason the lint -// guards next door are: shelling out to docker would nest a build -// inside `make test`. That `vaultik version` in the built image really -// prints the host's version is verified by hand and recorded on the -// pull request. +// These are parses of the committed files, because shelling out to +// docker would nest a build inside `make test`. That `vaultik version` +// in the built image really prints the host's version is verified by +// hand. -// dockerScript is script/docker, relative to the repository root. -const dockerScript = "script/docker" +// The files under guard, relative to the repository root. +const ( + productDockerfile = "Dockerfile" + dockerScript = "script/docker" +) -// versionArgs are the ldflag targets the build stamps and, matching -// them, the build args the host must supply. The names line up so the -// same list checks both files. -func versionArgs() []string { - return []string{"VERSION", "COMMIT", "COMMIT_DATE"} -} - -// TestProductDockerfileTakesVersionAsBuildArgs fails unless the build -// declares each version arg, with no default, and stamps it into the -// binary whenever it is given, ahead of the value derived in the -// container. -func TestProductDockerfileTakesVersionAsBuildArgs(t *testing.T) { +// TestProductDockerfileTakesVersionAsBuildArg fails unless the build +// declares ARG VERSION, with no default, and stamps it into the binary +// whenever it is given, ahead of the value derived in the container. +func TestProductDockerfileTakesVersionAsBuildArg(t *testing.T) { t.Parallel() found := instructions(t, productDockerfile) - for _, arg := range versionArgs() { - require.Contains(t, found, "ARG "+arg, - "%s must declare `ARG %s`, with no default, so the host can"+ - " pass it in", productDockerfile, arg) + require.Contains(t, found, "ARG VERSION", + "%s must declare `ARG VERSION`, with no default, so the host can"+ + " pass it in", productDockerfile) - assertLdflagReferences(t, found, arg) - } + assertLdflagReferences(t, found, "VERSION") } // TestProductDockerfileDerivesVersionFromGit fails unless a build given // no VERSION, such as a plain `docker build .` of a clone, takes it from -// `git describe` of the .git in its context, and fails rather than -// stamp "dev" when that .git yields no version. +// `git describe` of the .git in its context, stamps "dev" when the +// context has no .git, and fails rather than stamp "dev" when that .git +// yields no version. The commit and its date come from the same .git, +// and the build fails rather than stamp them "unknown" when it is +// present. func TestProductDockerfileDerivesVersionFromGit(t *testing.T) { t.Parallel() @@ -62,31 +58,31 @@ func TestProductDockerfileDerivesVersionFromGit(t *testing.T) { buildAt := indexContaining(found, "go build") require.GreaterOrEqual(t, buildAt, 0, "%s must build", productDockerfile) - assert.Contains(t, found[buildAt], "git describe --tags --always", - "%s must derive the version from git when no VERSION is given", - productDockerfile) + assert.Contains(t, found[buildAt], "git describe --tags --always || echo dev", + "%s must derive the version from git when no VERSION is given,"+ + " and stamp dev when the context has no .git", productDockerfile) assert.Contains(t, found[buildAt], "[ -e .git ]", "%s must fail when the context carries .git but yields no version", productDockerfile) + assert.Contains(t, found[buildAt], "git rev-parse HEAD", + "%s must stamp the commit from git", productDockerfile) + assert.Contains(t, found[buildAt], "git show -s --format=%cs HEAD", + "%s must stamp the commit date from git", productDockerfile) + assert.Contains(t, found[buildAt], + `[ "$commit" = unknown ] || [ "$commit_date" = unknown ]`, + "%s must fail when the context carries .git but yields no commit"+ + " or date", productDockerfile) } // TestDockerScriptComputesVersionOnTheHost fails unless script/docker -// derives each value where .git exists and passes it as a build arg, -// with VERSION coming from script/version so a Docker build reports the -// same string a local build of the same tree would. +// passes the version it derives where .git exists as a build arg. func TestDockerScriptComputesVersionOnTheHost(t *testing.T) { t.Parallel() script := readRepoFile(t, dockerScript) - for _, arg := range versionArgs() { - assert.Contains(t, script, "--build-arg "+arg+"=", - "%s must pass --build-arg %s to the build", dockerScript, arg) - } - - assert.Contains(t, script, "/version", - "%s must take VERSION from script/version, as the Makefile does", - dockerScript) + assert.Contains(t, script, "--build-arg VERSION=", + "%s must pass --build-arg VERSION to the build", dockerScript) } // assertLdflagReferences fails unless the build instruction uses the @@ -107,3 +103,47 @@ func assertLdflagReferences(t *testing.T, found []string, arg string) { "the go build in %s must use ${%s:-...}, or the arg is passed and"+ " discarded", productDockerfile, arg) } + +// instructions returns the Dockerfile's instructions, one per element, +// with comments and blank lines dropped and continuation lines joined, +// so a multi-line RUN is one string. +func instructions(t *testing.T, name string) []string { + t.Helper() + + var ( + out []string + continued string + isContinued bool + ) + + for line := range strings.SplitSeq(readRepoFile(t, name), "\n") { + trimmed := strings.TrimSpace(line) + if !isContinued && (trimmed == "" || strings.HasPrefix(trimmed, "#")) { + continue + } + + isContinued = strings.HasSuffix(trimmed, `\`) + continued += strings.TrimSuffix(trimmed, `\`) + + if isContinued { + continue + } + + out = append(out, strings.Join(strings.Fields(continued), " ")) + continued = "" + } + + return out +} + +// indexContaining returns the position of the first instruction +// containing want; -1 if there is none. +func indexContaining(found []string, want string) int { + for i, instruction := range found { + if strings.Contains(instruction, want) { + return i + } + } + + return -1 +} diff --git a/cmd/vaultik/lintdocker_test.go b/cmd/vaultik/lintdocker_test.go deleted file mode 100644 index fa620ce..0000000 --- a/cmd/vaultik/lintdocker_test.go +++ /dev/null @@ -1,368 +0,0 @@ -package main_test - -import ( - "os" - "path/filepath" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// This file guards the shape of the lint gate. Every property asserted -// here is one whose loss is SILENT: the build still exits 0, the gate -// still looks green, and nothing was linted or tested. -// -// The gate is a build step. script/lint builds Dockerfile.lint, which -// runs golangci-lint as a RUN instruction, so a successful build is a -// clean lint. BuildKit will happily replay that RUN from cache on an -// unchanged tree in well under a second, which is why the check layers -// are keyed on a CHECK_EPOCH build arg that the calling script -// regenerates per invocation, and why an empty value is a hard error -// rather than a stable cache key. -// -// These are parses rather than invocations. Shelling out to docker from -// the test suite would nest a build inside `make test`, which itself -// runs inside a build in CI. The one property a parse cannot establish -// -- that a real finding actually fails the build -- is verified by -// hand against a deliberately broken tree, recorded on the pull -// request. -// -// One property is deliberately NOT tested here: that no script runs the -// linter on the host. script/lint is the only lint entry point, and it -// runs golangci-lint only inside the container; keeping it that way is a -// review matter, not something a test in this file establishes. - -// The files under guard, relative to the repository root. -const ( - lintDockerfile = "Dockerfile.lint" - productDockerfile = "Dockerfile" - lintScript = "script/lint" - cibuildScript = "script/cibuild" -) - -// linterBinary is the linter's command name, used to locate the -// config-verify and lint steps in Dockerfile.lint. -const linterBinary = "golangci-lint" - -// checkEpochARG is the declaration, with no default value. A default -// would satisfy the non-empty guard with a constant, and a constant is -// a stable cache key: the checks would be replayed from cache forever -// after the first build. -const checkEpochARG = "ARG CHECK_EPOCH" - -// checkEpochGuard is what turns a build that omits --build-arg into a -// loud failure instead of a quiet green. Failed steps are never cached, -// so it fires on every such invocation rather than once. -const checkEpochGuard = `RUN [ -n "$CHECK_EPOCH" ] || exit 1` - -// freshEpoch is the epoch computation the calling scripts must use, as -// a bare assignment on its own line. Inline in an argument, a failing -// `date` would not abort under `set -eu`; CHECK_EPOCH would become the -// empty string, and the guard above would be the only thing standing -// between that and a permanently cached green. `$$` is required because -// `date +%s` is second-granular and busybox silently drops `%N`, so -// without the pid two concurrent runs in one second can collide. -const freshEpoch = `epoch="$(date +%s%N)$$"` - -// TestLintDockerfilePinsTheLinterByDigest fails if the lint image stops -// being pinned. An unpinned tag makes the gate's verdict depend on -// whatever the registry currently serves under that name. -func TestLintDockerfilePinsTheLinterByDigest(t *testing.T) { - t.Parallel() - - from := "" - - for _, instruction := range instructions(t, lintDockerfile) { - if strings.HasPrefix(instruction, "FROM ") { - from = instruction - - break - } - } - - require.NotEmpty(t, from, "%s declares no FROM", lintDockerfile) - assert.Contains(t, from, "golangci/golangci-lint", - "the lint image must be the golangci-lint image") - assert.Contains(t, from, "@sha256:", - "the lint image must be pinned by digest, not by tag alone") -} - -// TestLintDockerfileCannotBeCachedGreen pins the whole cache-busting -// mechanism in the file that lints: the declaration with no default, -// the non-empty guard, and the value expanded into the lint command -// itself rather than merely declared. -func TestLintDockerfileCannotBeCachedGreen(t *testing.T) { - t.Parallel() - - found := instructions(t, lintDockerfile) - - argAt := indexOf(found, checkEpochARG) - require.GreaterOrEqual(t, argAt, 0, - "%s must declare `%s` with no default value", - lintDockerfile, checkEpochARG) - - assert.GreaterOrEqual(t, indexOf(found, checkEpochGuard), argAt, - "%s must guard against an empty CHECK_EPOCH with `%s`", - lintDockerfile, checkEpochGuard) - - assertEpochExpandedInto(t, found[argAt:], "golangci-lint run") - - // Dependency layers must stay above the ARG, or every lint run - // re-downloads the module cache and the inner loop becomes - // unusable. - download := indexOf(found, "RUN go mod download") - require.GreaterOrEqual(t, download, 0, - "%s must download modules in their own layer", lintDockerfile) - assert.Less(t, download, argAt, - "`%s` must come after `go mod download` so dependency layers"+ - " still cache", checkEpochARG) -} - -// TestLintDockerfileVerifiesTheLinterConfig guards the validation of -// .golangci.yml itself. `golangci-lint run` rejects a config it cannot -// parse but silently IGNORES an unknown top-level key, so renaming -// `linters:` to `linterz:` discards `default: all` and every threshold -// and still exits 0 reporting no issues. `config verify` is what turns -// that into a failure, and it has to run BEFORE the lint, or the lint -// spends a minute reporting a verdict from a config already known to be -// wrong. -func TestLintDockerfileVerifiesTheLinterConfig(t *testing.T) { - t.Parallel() - - found := instructions(t, lintDockerfile) - verify := linterBinary + " config verify" - - verifyAt := indexContaining(found, verify) - require.GreaterOrEqual(t, verifyAt, 0, - "%s must run `%s --config .golangci.yml`: without it a typo'd"+ - " top-level key in .golangci.yml is silently ignored and the"+ - " gate passes with only the default linter set", lintDockerfile, - verify) - - runAt := indexContaining(found, linterBinary+" run") - require.GreaterOrEqual(t, runAt, 0, "%s must lint", lintDockerfile) - assert.Less(t, verifyAt, runAt, - "%s must verify the config before linting with it", lintDockerfile) - - // Keyed on the epoch like every other check layer, so it executes - // per invocation rather than being replayed. A cached validation - // validates nothing. - assertEpochExpandedInto(t, found, verify) -} - -// TestProductDockerfileKeysChecksOnTheEpoch holds the same line for the -// checks that remain in the product image build, but without the guard: -// a plain `docker build .` with no build arguments must succeed. -func TestProductDockerfileKeysChecksOnTheEpoch(t *testing.T) { - t.Parallel() - - found := instructions(t, productDockerfile) - - argAt := indexOf(found, checkEpochARG) - require.GreaterOrEqual(t, argAt, 0, - "%s must declare `%s`", productDockerfile, checkEpochARG) - - assert.Equal(t, -1, indexOf(found, checkEpochGuard), - "%s must not refuse an empty CHECK_EPOCH: a plain `docker build .`"+ - " must succeed", productDockerfile) - - assertEpochExpandedInto(t, found[argAt:], "make fmt-check") - assertEpochExpandedInto(t, found[argAt:], "make test") -} - -// TestProductDockerfileDoesNotLint records the split deliberately: the -// linter lives in Dockerfile.lint and nowhere else, so there is exactly -// one digest pinning it. A lint stage reintroduced here would either be -// docker-in-docker (`make lint` is now `docker build`) or a second, -// independently bumpable pin. -func TestProductDockerfileDoesNotLint(t *testing.T) { - t.Parallel() - - contents := readRepoFile(t, productDockerfile) - - for _, forbidden := range []string{"golangci", "make lint"} { - assert.NotContains(t, instructionText(contents), forbidden, - "%s must not lint: the linter is pinned once, in %s", - productDockerfile, lintDockerfile) - } -} - -// TestLintScriptBuildsTheLintDockerfileWithAFreshEpoch is the other -// half of the mechanism. The Dockerfile's guard only rejects an EMPTY -// epoch; a constant non-empty one would satisfy it and still be served -// from cache forever. -func TestLintScriptBuildsTheLintDockerfileWithAFreshEpoch(t *testing.T) { - t.Parallel() - - script := readRepoFile(t, lintScript) - - assertBareEpochAssignment(t, script, lintScript) - assert.Contains(t, script, `--build-arg CHECK_EPOCH="$epoch"`, - "%s must pass the fresh epoch to the build", lintScript) - assert.Contains(t, script, lintDockerfile, - "%s must build %s", lintScript, lintDockerfile) -} - -// TestCibuildBuildsBothDockerfilesWithFreshEpochs guards the CI gate: -// dropping either build silently removes a whole class of check from -// CI while leaving it green. -func TestCibuildBuildsBothDockerfilesWithFreshEpochs(t *testing.T) { - t.Parallel() - - script := readRepoFile(t, cibuildScript) - - assertBareEpochAssignment(t, script, cibuildScript) - assert.Equal(t, 2, strings.Count(script, freshEpoch), - "%s must compute a fresh epoch for each of its two builds", - cibuildScript) - assert.Equal(t, 2, - strings.Count(script, `--build-arg CHECK_EPOCH="$epoch"`), - "%s must pass a fresh epoch to both builds", cibuildScript) - assert.Contains(t, script, "-f Dockerfile.lint", - "%s must build %s", cibuildScript, lintDockerfile) -} - -// assertEpochExpandedInto fails unless some instruction runs the named -// command with the epoch expanded into it. Expansion, not mere -// declaration: an ARG that no instruction references is not guaranteed -// to key the layer, and the expansion also puts the value in the build -// log where a reader can see the layer was keyed fresh. -func assertEpochExpandedInto(t *testing.T, found []string, command string) { - t.Helper() - - for _, instruction := range found { - if !strings.HasPrefix(instruction, "RUN ") { - continue - } - - if strings.Contains(instruction, command) && - strings.Contains(instruction, "${CHECK_EPOCH}") { - return - } - } - - assert.Fail(t, "no epoch-keyed layer runs the command", - "`%s` must run in a layer that expands ${CHECK_EPOCH}, or it"+ - " will be replayed from cache without executing", command) -} - -// assertBareEpochAssignment fails unless the script computes the epoch -// as a bare assignment on its own line. -func assertBareEpochAssignment(t *testing.T, script, name string) { - t.Helper() - - for line := range strings.SplitSeq(script, "\n") { - if strings.TrimSpace(line) == freshEpoch { - return - } - } - - assert.Fail(t, "no bare epoch assignment", - "%s must compute `%s` as a bare assignment on its own line, so"+ - " `set -e` catches a failing date instead of quietly"+ - " building with an empty epoch", name, freshEpoch) -} - -// instructions returns the Dockerfile's instructions, one per element, -// with comments and blank lines dropped and continuation lines joined, -// so a multi-line RUN is one string. -func instructions(t *testing.T, name string) []string { - t.Helper() - - return strings.Split(instructionText(readRepoFile(t, name)), "\n") -} - -// instructionText is instructions' parse, before splitting: it is also -// what a "must not contain" assertion should look at, so that a word -// appearing only in a comment is not mistaken for behaviour. -func instructionText(contents string) string { - var ( - out []string - continued string - isContinued bool - ) - - for line := range strings.SplitSeq(contents, "\n") { - trimmed := strings.TrimSpace(line) - if !isContinued && (trimmed == "" || strings.HasPrefix(trimmed, "#")) { - continue - } - - isContinued = strings.HasSuffix(trimmed, `\`) - continued += strings.TrimSuffix(trimmed, `\`) - - if isContinued { - continue - } - - out = append(out, strings.Join(strings.Fields(continued), " ")) - continued = "" - } - - return strings.Join(out, "\n") -} - -// indexOf returns the position of the first instruction equal to, or -// beginning with, want; -1 if there is none. An `ARG NAME=default` -// counts as beginning with `ARG NAME`, so a declared arg is found -// whether or not it carries a default. -func indexOf(found []string, want string) int { - for i, instruction := range found { - if instruction == want || - strings.HasPrefix(instruction, want+" ") || - strings.HasPrefix(instruction, want+"=") { - return i - } - } - - return -1 -} - -// indexContaining returns the position of the first instruction -// containing want; -1 if there is none. -func indexContaining(found []string, want string) int { - for i, instruction := range found { - if strings.Contains(instruction, want) { - return i - } - } - - return -1 -} - -// readRepoFile reads a file by its path relative to the repository -// root. -func readRepoFile(t *testing.T, name string) string { - t.Helper() - - //nolint:gosec // G304: the path is a constant relative to this repo - contents, err := os.ReadFile(filepath.Join(repoRoot(t), name)) - require.NoError(t, err) - - return string(contents) -} - -// repoRoot returns the repository root. The test binary runs with its -// package directory as the working directory, so the root is found by -// walking up until the module file appears. -func repoRoot(t *testing.T) string { - t.Helper() - - dir, err := os.Getwd() - require.NoError(t, err) - - for { - _, err = os.Stat(filepath.Join(dir, "go.mod")) - if err == nil { - return dir - } - - parent := filepath.Dir(dir) - require.NotEqual(t, dir, parent, - "walked to the filesystem root without finding a go.mod") - - dir = parent - } -} diff --git a/cmd/vaultik/makefile_test.go b/cmd/vaultik/makefile_test.go index b937f15..8b527be 100644 --- a/cmd/vaultik/makefile_test.go +++ b/cmd/vaultik/makefile_test.go @@ -1,6 +1,8 @@ package main_test import ( + "os" + "path/filepath" "regexp" "slices" "strings" @@ -84,14 +86,48 @@ func TestBuildTargetBuildsTheBinary(t *testing.T) { "`make build` must depend on the rule that builds the binary") } -// readMakefile returns the contents of the repository's Makefile. The -// root is located by the shared walk in lintdocker_test.go. +// readMakefile returns the contents of the repository's Makefile. func readMakefile(t *testing.T) string { t.Helper() return readRepoFile(t, "Makefile") } +// readRepoFile reads a file by its path relative to the repository +// root. +func readRepoFile(t *testing.T, name string) string { + t.Helper() + + //nolint:gosec // G304: the path is a constant relative to this repo + contents, err := os.ReadFile(filepath.Join(repoRoot(t), name)) + require.NoError(t, err) + + return string(contents) +} + +// repoRoot returns the repository root. The test binary runs with its +// package directory as the working directory, so the root is found by +// walking up until the module file appears. +func repoRoot(t *testing.T) string { + t.Helper() + + dir, err := os.Getwd() + require.NoError(t, err) + + for { + _, err = os.Stat(filepath.Join(dir, "go.mod")) + if err == nil { + return dir + } + + parent := filepath.Dir(dir) + require.NotEqual(t, dir, parent, + "walked to the filesystem root without finding a go.mod") + + dir = parent + } +} + // phonyTargets returns every name declared phony, across all .PHONY // lines. func phonyTargets(makefile string) []string { diff --git a/internal/config/size.go b/internal/config/size.go index 8f21a87..23b77b6 100644 --- a/internal/config/size.go +++ b/internal/config/size.go @@ -16,8 +16,6 @@ var ( // Size represents a byte size that can be specified in configuration files. // It can unmarshal from both numeric values (interpreted as bytes) and // human-readable strings like "10MB", "2.5GB", or "1TB". -// -//nolint:recvcheck // UnmarshalYAML requires a pointer; String/Int64 are value reads type Size int64 // UnmarshalYAML implements yaml.Unmarshaler for Size, allowing it to be diff --git a/internal/database/chunk_files.go b/internal/database/chunk_files.go index 308af59..0ce345a 100644 --- a/internal/database/chunk_files.go +++ b/internal/database/chunk_files.go @@ -220,7 +220,7 @@ func (r *ChunkFileRepository) CreateBatch( cf.ChunkHash.String(), cf.FileID.String(), cf.FileOffset, cf.Length) } - query += querySb183.String() //nolint:gosec // G202: appends "?" placeholders only + query += querySb183.String() query += " ON CONFLICT(chunk_hash, file_id) DO NOTHING" diff --git a/internal/database/chunks.go b/internal/database/chunks.go index 833c1aa..51e95d1 100644 --- a/internal/database/chunks.go +++ b/internal/database/chunks.go @@ -98,7 +98,7 @@ func (r *ChunkRepository) GetByHashes( args[i] = hash } - query += querySb75.String() //nolint:gosec // G202: appends "?" placeholders only + query += querySb75.String() query += ") ORDER BY chunk_hash" diff --git a/internal/database/file_chunks.go b/internal/database/file_chunks.go index 04388d7..0803de2 100644 --- a/internal/database/file_chunks.go +++ b/internal/database/file_chunks.go @@ -253,7 +253,7 @@ func (r *FileChunkRepository) CreateBatch( args = append(args, fc.FileID.String(), fc.Idx, fc.ChunkHash.String()) } - query += querySb211.String() //nolint:gosec // G202: appends "?" placeholders only + query += querySb211.String() query += " ON CONFLICT(file_id, idx) DO NOTHING" diff --git a/internal/database/files.go b/internal/database/files.go index f9d8022..c30c07e 100644 --- a/internal/database/files.go +++ b/internal/database/files.go @@ -391,7 +391,7 @@ func (r *FileRepository) CreateBatch( f.LinkTarget.String()) } - query += querySb325.String() //nolint:gosec // G202: appends "?" placeholders only + query += querySb325.String() query += ` ON CONFLICT(path) DO UPDATE SET source_path = excluded.source_path, diff --git a/internal/database/snapshots.go b/internal/database/snapshots.go index 07c8242..4479985 100644 --- a/internal/database/snapshots.go +++ b/internal/database/snapshots.go @@ -395,7 +395,7 @@ func (r *SnapshotRepository) AddFilesByIDBatch( args = append(args, snapshotID, fileID.String()) } - query += querySb312.String() //nolint:gosec // G202: appends "?" placeholders only + query += querySb312.String() var err error if tx != nil { diff --git a/internal/globals/globals.go b/internal/globals/globals.go index fd00834..984b8a0 100644 --- a/internal/globals/globals.go +++ b/internal/globals/globals.go @@ -18,14 +18,19 @@ var Appname = "vaultik" //nolint:gochecknoglobals // set via -ldflags at build t // deliberately not a number. const DevVersion = "dev" +// Unknown is what Commit and CommitDate hold when the build did not +// stamp them, and the version script/docker and script/cibuild stamp +// when the host has no git checkout. +const Unknown = "unknown" + // Version is the application version, populated from main(). var Version = DevVersion //nolint:gochecknoglobals // set via -ldflags at build time // Commit is the git commit hash, populated from main(). -var Commit = "unknown" //nolint:gochecknoglobals // set via -ldflags at build time +var Commit = Unknown //nolint:gochecknoglobals // set via -ldflags at build time // CommitDate is the ISO-8601 date of the commit, populated from main(). -var CommitDate = "unknown" //nolint:gochecknoglobals // set via -ldflags at build time +var CommitDate = Unknown //nolint:gochecknoglobals // set via -ldflags at build time // Author identifies the upstream author of vaultik. const Author = "Jeffrey Paul " @@ -72,9 +77,11 @@ func New() (*Globals, error) { // safe reading of "we could not establish that this is a release" is // that it is not one. The Makefile refuses to build at all in that // case; this is the second line of defence, for a binary linked by -// something other than the Makefile. +// something other than the Makefile. Unknown counts for the same +// reason. func IsDevVersion(v string) bool { - if v == "" || v == DevVersion || strings.HasPrefix(v, DevVersion+"-") || + if v == "" || v == Unknown || v == DevVersion || + strings.HasPrefix(v, DevVersion+"-") || strings.HasSuffix(v, "-dirty") { return true } diff --git a/internal/globals/globals_test.go b/internal/globals/globals_test.go index 7012e60..fd05e4a 100644 --- a/internal/globals/globals_test.go +++ b/internal/globals/globals_test.go @@ -74,6 +74,9 @@ func TestIsDevVersion(t *testing.T) { // as one. The Makefile refuses to build when script/version // yields nothing; this covers a binary linked some other way. {"", true}, + // What script/docker and script/cibuild stamp when the host + // has no git checkout. + {"unknown", true}, } for _, tc := range cases { diff --git a/internal/snapshot/scanner.go b/internal/snapshot/scanner.go index 850b1b8..f7eb40e 100644 --- a/internal/snapshot/scanner.go +++ b/internal/snapshot/scanner.go @@ -1377,8 +1377,7 @@ func (s *Scanner) processFileWithErrorHandling( // record a file whose chunk is in no blob and cannot be restored, so // abort the run even under --skip-errors. Only open and read errors // are skipped below. - var pErr *packerError - if errors.As(err, &pErr) { + if _, ok := errors.AsType[*packerError](err); ok { return false, fmt.Errorf("processing file %s: %w", fileToProcess.Path, err) } // Handle files that were deleted between scan and process phases diff --git a/internal/storage/url.go b/internal/storage/url.go index 24ea329..8da6eca 100644 --- a/internal/storage/url.go +++ b/internal/storage/url.go @@ -161,8 +161,7 @@ func rejectUnknownParams(query url.Values, allowed ...string) error { // *url.Error that url.Parse returns embeds the raw URL in its message, so // wrapping it directly would echo a credential-bearing URL into logs. func wrapParseError(err error) error { - var uerr *url.Error - if errors.As(err, &uerr) { + if uerr, ok := errors.AsType[*url.Error](err); ok { return fmt.Errorf("invalid URL: %w", uerr.Err) } diff --git a/script/bootstrap b/script/bootstrap index 21862fe..c1024a0 100755 --- a/script/bootstrap +++ b/script/bootstrap @@ -48,12 +48,12 @@ missing() { ! command -v "$1" >/dev/null 2>&1 } -# Docker is a hard requirement, not a nice-to-have: script/lint lints by -# building Dockerfile.lint, whose digest-pinned golangci-lint image is -# the only place the linter runs, and script/check and script/precommit -# both run script/lint. A bootstrap that prints "bootstrap complete" on a -# machine where `make check` cannot run is a false success, so this fails -# instead. +# Docker is a hard requirement, not a nice-to-have: script/lint and +# script/test build the lint and test phases of the Dockerfile, the only +# place the linter and the tests run, and script/check and +# script/precommit both run them. A bootstrap that prints "bootstrap +# complete" on a machine where `make check` cannot run is a false +# success, so this fails instead. # # Installing docker from here was considered and rejected: it needs root, # a running daemon, and on macOS a GUI cask, so an attempt would itself @@ -80,15 +80,15 @@ bootstrap: FAILED - $reason. Docker is required to develop this repo. Without it these do not work: - script/lint builds Dockerfile.lint, which runs the linter as a - build step in a digest-pinned golangci-lint image. - That FROM line is the single source of truth for the - linter version - script/check runs script/lint + script/lint builds the lint phase of the Dockerfile, which runs + the linter as a build step in a digest-pinned + golangci-lint image + script/test builds the test phase of the Dockerfile + script/check runs script/test and script/lint script/precommit runs script/check, so commits are blocked by the pre-commit hook installed by script/setup - script/cibuild builds Dockerfile.lint and Dockerfile, which is what - CI runs + script/cibuild runs script/check and builds the image, which is + what CI runs Install docker (and start the daemon, checking DOCKER_HOST and your group membership), then re-run script/bootstrap. golangci-lint on PATH @@ -104,15 +104,22 @@ main() { if missing git; then pkg_install git git git git; fi if missing make; then pkg_install gnumake make make make; fi - # Go toolchain - if missing go; then pkg_install go golang go go; fi + # Go toolchain: the host's own, or else the version go.mod names, + # hash-verified, in .tool/go. That directory is not on the caller's + # PATH; this script, the Makefile, script/fmt, script/fmt-check, + # script/precommit and script/release add it to theirs. + if missing go; then + "$ROOT/script/install-go" + PATH="$PATH:$ROOT/.tool/go/bin" + fi # golangci-lint is deliberately NOT installed: script/lint lints by - # building Dockerfile.lint, whose digest-pinned image is the only - # place the linter runs, so whatever a package manager happens to - # ship would only be a shadow of the pinned version that could drift - # from CI. Nothing on the host is ever used as a linter, at any - # version, so installing one here would buy nothing. + # building the lint phase of the Dockerfile, whose digest-pinned + # image is the only place the linter runs, so whatever a package + # manager happens to ship would only be a shadow of the pinned + # version that could drift from CI. Nothing on the host is ever used + # as a linter, at any version, so installing one here would buy + # nothing. # goreleaser, at the version pinned by script/install-goreleaser and # verified against a hardcoded sha256. Package managers are not used diff --git a/script/check b/script/check index cc046f7..92875f7 100755 --- a/script/check +++ b/script/check @@ -1,7 +1,8 @@ #!/bin/sh # script/check: run all checks (test, lint, fmt-check). Our own -# extension to scripts-to-rule-them-all. Must not modify any files. -# Generic: usually needs no adaptation. +# extension to scripts-to-rule-them-all. test and lint are Docker +# phases; fmt-check is native, because a formatter writes the working +# tree. Must not modify any files. set -eu SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" diff --git a/script/cibuild b/script/cibuild index 032fae6..d8d3200 100755 --- a/script/cibuild +++ b/script/cibuild @@ -1,78 +1,28 @@ #!/bin/sh -# script/cibuild: run the CI build. This is the full gate, and it is two -# builds, in this order: -# -# Dockerfile.lint the linter, as a build step (a clean build IS a -# clean lint) -# Dockerfile `make fmt-check` and `make test` in the builder -# stage, then the product image -# -# Either one failing fails this script. Note what follows from the -# split: script/docker builds only the product image and so no longer -# lints -- this script and script/check (which runs script/lint) are the -# things that decide whether the tree is clean. -# -# Generic apart from the two Dockerfiles: the Gitea workflow runs this -# on push. +# script/cibuild: run the CI build. It bootstraps first: a CI runner +# checks out and runs this and nothing else, and script/fmt-check runs +# the formatter on the host, which a pristine checkout cannot do. +# --no-cache for the same reason as script/docker: the gate phases the +# final stage depends on are RUN steps, and a cached one is a check that +# did not run. set -eu -ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" +ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" main() { cd "$ROOT" - # Both Dockerfiles key their check layers on CHECK_EPOCH, so a fresh - # value is what forces those layers to re-run: without it an - # unchanged tree replays them from cache, the checks never execute, - # and the build still exits 0. Each ARG sits immediately above the - # check RUNs, so dependency and module layers still cache. - # Dockerfile.lint also refuses to build at all when CHECK_EPOCH is - # empty, so a missing value fails its build loudly rather than passing - # quietly; the product Dockerfile does not, because a plain `docker - # build .` must succeed. - # - # The value must be unique per invocation, not per second. `date +%s` - # is second-granular, so two concurrent invocations in the same - # second get identical epochs and the later one can be served from - # cache -- the original defect in miniature. `%N` alone does not fix - # it: busybox silently drops %N, exits 0, and hands back second - # granularity with no warning. `$$` is what makes this correct - # regardless, since concurrent invocations have different pids. - # - # Assign the epoch on its own line rather than inline in the - # argument. Under `set -eu` a command substitution that fails - # inside an argument does NOT abort the script: CHECK_EPOCH would - # become an empty string, an empty string is a constant, and a - # constant CHECK_EPOCH is exactly the cached-check false green this - # script exists to prevent -- so the guard would disarm itself and - # still exit 0. As a bare assignment, `set -e` catches a failing - # `date` and no build starts. - # - # A separate value per build, because they are separate builds: one - # `date` shared between them would still be fresh, but reusing it - # invites the two to be collapsed into a single value that is - # computed somewhere else and passed in. - epoch="$(date +%s%N)$$" - # cacheonly for the lint build: its verdict is the exit status and - # the image is never run, so exporting it is pure cost. See - # script/lint. - docker build --output=type=cacheonly \ - --build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint . - - # Version, commit and build date are computed here on the host, the - # same way script/docker does, and passed into the product build, - # where they take precedence over what the build would derive from - # the .git in its context. VERSION comes from script/version, as in - # the Makefile. - version="$("$ROOT/script/version")" - commit="$(git rev-parse HEAD 2>/dev/null || echo unknown)" - commit_date="$(git show -s --format=%cs HEAD 2>/dev/null || echo unknown)" - - epoch="$(date +%s%N)$$" - docker build --build-arg CHECK_EPOCH="$epoch" \ + "$SCRIPT_DIR/bootstrap" + "$SCRIPT_DIR/check" + # Own line: a failing command substitution inside an argument does + # not trip `set -e`, so the inline form degrades silently to an + # empty constant. The VERSION build argument takes precedence over + # the version a build stage derives from the .git in the context. + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build --no-cache \ --build-arg VERSION="$version" \ - --build-arg COMMIT="$commit" \ - --build-arg COMMIT_DATE="$commit_date" \ - . + -t "$("$SCRIPT_DIR/projectname")" . } main "$@" diff --git a/script/docker b/script/docker index d685746..07b626c 100755 --- a/script/docker +++ b/script/docker @@ -1,15 +1,8 @@ #!/bin/sh # script/docker: build the Docker image tagged with the project name. -# The tag comes from script/projectname. Unlike the canonical copy in -# sneak/prompts, it passes a fresh CHECK_EPOCH instead of --no-cache, and -# COMMIT and COMMIT_DATE as well as VERSION. -# -# This builds the PRODUCT image only, and the product Dockerfile has no -# lint stage: linting lives in Dockerfile.lint and is run by -# script/lint. So a green here means `make fmt-check` and `make test` -# passed and the image built -- it says nothing about lint. The gates -# are script/check (which runs script/lint) and script/cibuild (which -# builds both files). +# Identical in all repos; the tag comes from script/projectname. +# --no-cache because the gate phases the final stage depends on are RUN +# steps, and a cached one is a check that did not run. set -eu SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" @@ -17,28 +10,14 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" main() { cd "$ROOT" - # Same CHECK_EPOCH contract as script/cibuild, for the same reason - # and with the same bare-assignment and `$$` requirements -- see the - # comments there. This script is not the CI gate, but a local build - # is almost always warm, so without this it would report a green the - # tree had not earned and the two entrypoints would disagree about - # whether the tree is clean. - epoch="$(date +%s%N)$$" - - # Version, commit and build date are computed here on the host and - # passed into the build, where they take precedence over what the - # build would derive from the .git in its context. VERSION comes - # from script/version, as in the Makefile, so the image reports the - # same string, -dirty included, that a local build of the same tree - # would. - version="$("$SCRIPT_DIR/version")" - commit="$(git rev-parse HEAD 2>/dev/null || echo unknown)" - commit_date="$(git show -s --format=%cs HEAD 2>/dev/null || echo unknown)" - - docker build --build-arg CHECK_EPOCH="$epoch" \ + # Own line: a failing command substitution inside an argument does + # not trip `set -e`, so the inline form degrades silently to an + # empty constant. The VERSION build argument takes precedence over + # the version a build stage derives from the .git in the context. + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build --no-cache \ --build-arg VERSION="$version" \ - --build-arg COMMIT="$commit" \ - --build-arg COMMIT_DATE="$commit_date" \ -t "$("$SCRIPT_DIR/projectname")" . } diff --git a/script/fmt b/script/fmt index e95d111..f4876e1 100755 --- a/script/fmt +++ b/script/fmt @@ -6,6 +6,8 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" + # Where script/bootstrap installs Go when the host has none. + PATH="$PATH:$ROOT/.tool/go/bin" go fmt ./... } diff --git a/script/fmt-check b/script/fmt-check index 57e8e5c..84c23df 100755 --- a/script/fmt-check +++ b/script/fmt-check @@ -7,7 +7,14 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - unformatted="$(gofmt -l .)" + # Where script/bootstrap installs Go when the host has none. + PATH="$PATH:$ROOT/.tool/go/bin" + # .tool/go holds that toolchain's own sources, which are not ours. + if ! unformatted="$(find . -path ./.tool -prune -o -name '*.go' -type f \ + -exec gofmt -l {} +)"; then + echo "fmt-check: gofmt failed" >&2 + exit 1 + fi if [ -n "$unformatted" ]; then echo "Files not formatted:" >&2 echo "$unformatted" >&2 diff --git a/script/install-go b/script/install-go index 38056dc..3442421 100755 --- a/script/install-go +++ b/script/install-go @@ -4,12 +4,13 @@ # own extension to scripts-to-rule-them-all. Idempotent: exits at once # when the pinned toolchain is already installed. # -# Only .gitea/workflows/release.yml calls this. goreleaser is not a +# .gitea/workflows/release.yml calls this, and so does script/bootstrap +# when the host has no Go, as on the check runner. goreleaser is not a # compiler: it shells out to `go` for the `before:` hook and for every # one of the four cross-compiles, so the release runner needs a Go -# toolchain on PATH. check.yml never does -- it builds inside the -# digest-pinned Dockerfile images -- so this is the release path's only -# host Go, and per REPO_POLICIES.md it must be pinned by hash. +# toolchain on PATH. The check runner compiles nothing on the host; it +# uses this Go only for bootstrap's `go mod download` and for gofmt in +# script/fmt-check. Per REPO_POLICIES.md a host Go is pinned by hash. # actions/setup-go exposes no checksum input, so Go is installed the way # script/install-goreleaser installs goreleaser: download the exact # archive from go.dev and refuse it unless its sha256 matches the value @@ -18,11 +19,11 @@ # The version is go.mod's `go` directive, the single source of truth for # the toolchain. GO_VERSION below MUST equal it, and this script fails # when they disagree -- so bumping Go is one reviewed change touching -# go.mod, the checksum here, and the Dockerfile golang digest together. +# go.mod, the checksums here, and the Dockerfile's two golang digests +# together. # -# Linux only, because that is what the release runner is. A darwin dev -# building a snapshot uses their own Go; supporting an OS means adding -# its checksums. +# Linux and macOS, each on amd64 and arm64: the four archives whose +# checksums are committed below. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" @@ -33,6 +34,8 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" GO_VERSION="1.26.1" SHA256_LINUX_AMD64="031f088e5d955bab8657ede27ad4e3bc5b7c1ba281f05f245bcc304f327c987a" SHA256_LINUX_ARM64="a290581cfe4fe28ddd737dde3095f3dbeb7f2e4065cab4eae44dfc53b760c2f7" +SHA256_DARWIN_AMD64="65773dab2f8cc4cd23d93ba6d0a805de150ca0b78378879292be0b903b8cdd08" +SHA256_DARWIN_ARM64="353df43a7811ce284c8938b5f3c7df40b7bfb6f56cb165b150bc40b5e2dd541f" GOROOT_DIR="$ROOT/.tool/go" GOCMD="$GOROOT_DIR/bin/go" @@ -101,26 +104,28 @@ main() { arch="$(uname -m)" case "$os" in Linux) os="linux" ;; + Darwin) os="darwin" ;; *) - echo "install-go: unsupported OS $os (release runner is Linux)" >&2 + echo "install-go: unsupported OS $os" >&2 exit 1 ;; esac case "$arch" in - x86_64 | amd64) - arch="amd64" - sum="$SHA256_LINUX_AMD64" - ;; - arm64 | aarch64) - arch="arm64" - sum="$SHA256_LINUX_ARM64" - ;; + x86_64 | amd64) arch="amd64" ;; + arm64 | aarch64) arch="arm64" ;; *) - echo "install-go: no pinned checksum for architecture $arch" >&2 + echo "install-go: unsupported architecture $arch" >&2 exit 1 ;; esac + case "${os}-${arch}" in + linux-amd64) sum="$SHA256_LINUX_AMD64" ;; + linux-arm64) sum="$SHA256_LINUX_ARM64" ;; + darwin-amd64) sum="$SHA256_DARWIN_AMD64" ;; + darwin-arm64) sum="$SHA256_DARWIN_ARM64" ;; + esac + archive="go${GO_VERSION}.${os}-${arch}.tar.gz" url="https://go.dev/dl/${archive}" diff --git a/script/lint b/script/lint index 5499014..2d8b075 100755 --- a/script/lint +++ b/script/lint @@ -1,108 +1,23 @@ #!/bin/sh -# script/lint: run the linter. +# script/lint: run the linter. Linting is a phase of the Dockerfile and +# this builds that phase alone; the linter is never installed or run on +# a developer host, where a shared result cache and a host-global lock +# make its answer untrustworthy. # -# The linter runs inside the image built by Dockerfile.lint, and it runs -# there as a BUILD STEP: a successful build of that file IS a clean -# lint. Nothing lints on the host, at any version, ever. That FROM line -# is the single source of truth for the linter version in this repo, so -# a local run and a CI run of the same tree cannot disagree. -# -# One container per run means one lint cache and one golangci-lint lock -# per run, both private to that run and thrown away with it. That is -# what makes concurrent runs on a shared host safe, and it is why this -# script no longer carries per-worktree cache directories, a lock-retry -# loop, or an output audit: there is no shared state left for them to -# defend (issue https://git.eeqj.de/sneak/vaultik/issues/113). -# -# To watch the linter execute, set BUILDKIT_PROGRESS=plain, which docker -# honours directly: -# -# BUILDKIT_PROGRESS=plain script/lint -# -# The check layers -- `golangci-lint config verify` and then -# `golangci-lint run` -- must appear as executing rather than CACHED on -# every run; see the CHECK_EPOCH comment in Dockerfile.lint. +# The phase is not the last stage in the file, so it is built only when +# --target names it. --no-cache because a cached lint layer is a lint +# that did not run. The tag makes each build replace the previous image +# instead of leaving a dangling one behind. set -eu -ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" -DOCKERFILE="$ROOT/Dockerfile.lint" - -require_docker() { - if ! command -v docker >/dev/null 2>&1; then - cat >&2 </dev/null 2>&1; then - cat >&2 <&2 <&2 + echo "lint-fix: no FROM ... AS lint line found in $DOCKERFILE" >&2 exit 1 fi diff --git a/script/precommit b/script/precommit index 50f02ae..e55afea 100755 --- a/script/precommit +++ b/script/precommit @@ -9,6 +9,8 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" main() { cd "$ROOT" + # Where script/bootstrap installs Go when the host has none. + PATH="$PATH:$ROOT/.tool/go/bin" go mod tidy go fmt ./... git diff --exit-code -- go.mod go.sum || { diff --git a/script/release b/script/release index a9e2056..05ec0ae 100755 --- a/script/release +++ b/script/release @@ -44,6 +44,8 @@ resolve_goreleaser() { main() { cd "$ROOT" + # Where script/bootstrap installs Go when the host has none. + PATH="$PATH:$ROOT/.tool/go/bin" if ! bin="$(resolve_goreleaser)"; then cat >&2 < (cached)`, and that line is -# indistinguishable -- to every check this repo performs -- from a -# package that actually ran. The whole suite reports its full set of -# `ok` lines in under half a second having executed nothing. That -# matters beyond the local inner loop: the Dockerfile's `RUN make test` -# is forced to re-execute by CHECK_EPOCH, but a GOCACHE baked into an -# earlier image layer survives into the re-executed step, so the step -# can re-run and still do no work. It is applied unconditionally rather -# than only in the containerised path because the pre-commit hook runs -# this same script; a gate that is honest only in CI is dishonest -# exactly where people lean on it most. -# -# -timeout is a hang backstop, not a performance budget: its job is to -# turn a deadlocked test into a stack dump instead of a wedged CI job, -# so it wants to sit far above the slowest legitimate runtime, not just -# above it. It is per test binary and covers test execution only -- the -# clock starts inside testing.M.Run, after compilation and linking, so -# build time is not charged against it. (Measured: a containerised run -# with an empty GOCACHE reports per-package durations within noise of a -# warm host run. A shell `timeout 30 go test ./...` would include -# compilation, but that is a different mechanism from this flag.) -# -# The 120s value DELIBERATELY DIVERGES from REPO_POLICIES.md:192, which -# mandates "Add a 30-second timeout", and from that file's canonical Go -# recipe at :212-214, which uses -timeout 30s. REPO_POLICIES.md is -# org-canonical and cannot be amended from this repo, so the divergence -# is recorded here instead, and issue #101 proposes amending the policy -# text upstream. Do not revert this to 30s without reading #101 first. -# -# Why it diverges: the slowest packages are internal/database and -# internal/vaultik, observed under -race at about 6.4s warm, 8.1s in a -# cold containerised run on a contended host, and 10.2s in an -# independent cold run on this same host. The worst case is not tightly -# characterised -- each fresh measurement has come in above the last -- -# which is itself an argument for generous headroom. Against the 10.2s -# observation, 30s is only 2.9x: not a safety margin but a flake -# waiting for a slow day, whose failure mode is a timeout that looks -# like a real defect. 120s leaves about 12x while still bounding a hung -# package -- including the verbose rerun below -- to a few minutes. The -# cost of that choice, also recorded on #101: because of the rerun, a -# hung package pays the timeout twice. -run_tests() { - go test -race -timeout 120s -count=1 "$@" ./... -} +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" +ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" main() { cd "$ROOT" - run_tests || { - echo "--- Rerunning with -v for details ---" - run_tests -v - exit 1 - } + docker build --no-cache \ + --target test \ + -t "$("$SCRIPT_DIR/projectname")-test" . } main "$@" diff --git a/script/version b/script/version index 7ba6ad0..4006043 100755 --- a/script/version +++ b/script/version @@ -3,9 +3,8 @@ # Our own extension to scripts-to-rule-them-all. The Makefile's LDFLAGS # call this rather than carrying a hardcoded constant, which is what used # to make every local build claim to be 1.0.0-rc.1 regardless of git -# state. script/docker and script/cibuild pass its output to the image -# build; given no version, the image build runs `git describe --tags -# --always` itself, without `--dirty`. +# state. script/docker and script/cibuild run the same `git describe` +# themselves, and fall back to "unknown" rather than "dev". # # The version is `git describe --tags --always --dirty`: the tag on a # tagged commit, tag-N-gHASH on a commit after one, the short commit