Dockerfile duplicates dependency installs inline instead of running script/bootstrap, and omits -trimpath / -s -w #95

Open
opened 2026-08-09 07:03:29 +02:00 by clawbot · 0 comments
Collaborator

From the audit against the canonical REPO_POLICIES.md, verified against main at 61f42e6.

1. Inline dependency installs instead of script/bootstrap. The policy requires:

> 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.

Dockerfile:5 (lint stage) runs apk add --no-cache make build-base vips-dev libheif-dev pkgconfig, and Dockerfile:30-34 (build stage) runs apk add --no-cache build-base vips-dev libheif-dev pkgconfig. script/bootstrap is never invoked. This duplicates ensure_cgo_deps() at script/bootstrap:107-117, which already has an apk branch installing pkgconfig, vips-dev, libheif-dev.

Two copies of the same dependency list drift independently — a new libvips dependency added to script/bootstrap for local development silently does not reach the Docker build, and the failure shows up as a confusing CGO link error in CI rather than as a missing package.

2. Missing build flags. The policy's standard Go Dockerfile pattern is:

RUN CGO_ENABLED=0 go build -trimpath \
    -ldflags="-s -w -X main.Version=${VERSION}" \
    -o /app ./cmd/app/

Dockerfile:49 is RUN CGO_ENABLED=1 GOTOOLCHAIN=auto go build -ldflags "-X main.Version=${VERSION}" -o /pixad ./cmd/pixad — no -trimpath, no -s -w.

CGO_ENABLED=1 is a legitimate, necessary deviation (libvips), so the block is not followed literally and should not be. But -trimpath and -s -w have no such justification: without -trimpath the binary embeds /src/... build paths, which brushes against the policy's "Internal errors must never leak ... file paths", and -s -w is free binary size.

Stated honestly: item 2 comes from a code-block example rather than prose stated as a MUST, so it is a weaker requirement than item 1. Flagging it because the two fixes are one file and one commit.

Definition of done

  1. Both the lint and build stages COPY script/ ./script/ plus go.mod/go.sum, then RUN script/bootstrap in place of the inline apk add lines, ordered so the bootstrap layer caches on the manifests.
  2. -trimpath and -s -w added to the build; the binary still reports its version correctly (verify by running --version or equivalent on the built image, not by inspection).
  3. docker build . green end to end, and the lint stage still works — it is the authoritative gate.
  4. Build time does not regress meaningfully; the policy has a 5-minute budget. Report before/after in the PR.

Coordination

Dockerfile is modified by PR #54 (the lint-stage image pin). Do this after #54 merges to avoid conflicting with a PR that has already passed review.

From the audit against the canonical `REPO_POLICIES.md`, verified against `main` at `61f42e6`. **1. Inline dependency installs instead of `script/bootstrap`.** The policy requires: > 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. `Dockerfile:5` (lint stage) runs `apk add --no-cache make build-base vips-dev libheif-dev pkgconfig`, and `Dockerfile:30-34` (build stage) runs `apk add --no-cache build-base vips-dev libheif-dev pkgconfig`. `script/bootstrap` is never invoked. This duplicates `ensure_cgo_deps()` at `script/bootstrap:107-117`, which already has an `apk` branch installing `pkgconfig`, `vips-dev`, `libheif-dev`. Two copies of the same dependency list drift independently — a new libvips dependency added to `script/bootstrap` for local development silently does not reach the Docker build, and the failure shows up as a confusing CGO link error in CI rather than as a missing package. **2. Missing build flags.** The policy's standard Go Dockerfile pattern is: ``` RUN CGO_ENABLED=0 go build -trimpath \ -ldflags="-s -w -X main.Version=${VERSION}" \ -o /app ./cmd/app/ ``` `Dockerfile:49` is `RUN CGO_ENABLED=1 GOTOOLCHAIN=auto go build -ldflags "-X main.Version=${VERSION}" -o /pixad ./cmd/pixad` — no `-trimpath`, no `-s -w`. `CGO_ENABLED=1` is a legitimate, necessary deviation (libvips), so the block is not followed literally and should not be. But `-trimpath` and `-s -w` have no such justification: without `-trimpath` the binary embeds `/src/...` build paths, which brushes against the policy's "Internal errors must never leak ... file paths", and `-s -w` is free binary size. **Stated honestly:** item 2 comes from a code-block example rather than prose stated as a MUST, so it is a weaker requirement than item 1. Flagging it because the two fixes are one file and one commit. ## Definition of done 1. Both the lint and build stages `COPY script/ ./script/` plus `go.mod`/`go.sum`, then `RUN script/bootstrap` in place of the inline `apk add` lines, ordered so the bootstrap layer caches on the manifests. 2. `-trimpath` and `-s -w` added to the build; the binary still reports its version correctly (verify by running `--version` or equivalent on the built image, not by inspection). 3. `docker build .` green end to end, and the lint stage still works — it is the authoritative gate. 4. Build time does not regress meaningfully; the policy has a 5-minute budget. Report before/after in the PR. ## Coordination `Dockerfile` is modified by PR #54 (the lint-stage image pin). Do this **after** #54 merges to avoid conflicting with a PR that has already passed review.
clawbot added this to the 1.0.0 milestone 2026-08-09 07:03:29 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#95