Makefile hardcodes a personal remote DOCKER_HOST, breaking make docker for everyone else #44

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

From the repo-standards audit. Present on main and the pending lint branch.

Problem

Makefile:2:

export DOCKER_HOST := ssh://root@ber1app1.local

:= assigns unconditionally, overriding any inherited DOCKER_HOST. Consequences:

  • make docker silently targets one specific machine on a private LAN, over SSH as root, for every user of this repo.
  • Any contributor, and any CI runner that invokes the make target rather than script/cibuild, fails with an unresolvable host and an error that gives no hint why.
  • Nobody but the author can build the image locally at all.

Policy: "Simple projects should be configured with environment variables", and the Makefile is meant to be "authoritative documentation for how the repo is used". A hardcoded personal host is neither.

Two smaller Makefile defects while in the same file:

Missing .PHONY entries. The list covers default bootstrap setup test lint fmt fmt-check check docker hooks vet but omits build, clean, install, and docker-run. build is the live risk: the Dockerfile runs make build at line 31, and a file or directory named build in the repo root would make the target a silent no-op, producing an image with a stale or missing binary.

Targets that bypass script/. The nine policy-named targets are correct thin shims, but build runs go build inline, and vet, clean, install, docker-run invoke tooling directly. build matters most because the Dockerfile depends on it, so real build logic lives in the Makefile rather than in script/ where policy puts it. vet is also redundant — script/test already runs go vet first.

Definition of done

  • DOCKER_HOST is no longer unconditionally assigned. Either remove the line entirely, or make it opt-in and overridable with ?= and an empty default.
  • make docker works on a stock local Docker installation with no environment setup.
  • .PHONY includes build, clean, install, and docker-run.
  • script/build exists as a POSIX sh entrypoint following the conventions of the other scripts (#!/bin/sh, set -eu, the repo-root cd idiom), and build: becomes a thin shim to it.
  • The Dockerfile still builds successfully with the reworked build target.
  • The redundant vet target is removed, or kept with a one-line comment saying why it earns its place.
  • make check green. TODO.md updated in the same commit.

Implementation requirements

  • Do not relocate the ldflags version-injection logic in a way that breaks it. Whatever moves into script/build must still produce a binary with the version string populated — build it and run the version command to confirm, since nothing in the test suite covers this.
  • Coordinate with #43 (module path rename), which also edits the ldflags lines. Whichever lands second must carry the other's change forward.
  • If removing the DOCKER_HOST line would disrupt your own workflow, the ?= form plus a line in the README noting it can be set in the environment is the better option — but the default must be a working local build.
From the repo-standards audit. Present on `main` and the pending lint branch. ## Problem `Makefile:2`: ```make export DOCKER_HOST := ssh://root@ber1app1.local ``` `:=` assigns unconditionally, overriding any inherited `DOCKER_HOST`. Consequences: - `make docker` silently targets one specific machine on a private LAN, over SSH as root, for every user of this repo. - Any contributor, and any CI runner that invokes the make target rather than `script/cibuild`, fails with an unresolvable host and an error that gives no hint why. - Nobody but the author can build the image locally at all. Policy: *"Simple projects should be configured with environment variables"*, and the Makefile is meant to be *"authoritative documentation for how the repo is used"*. A hardcoded personal host is neither. Two smaller Makefile defects while in the same file: **Missing `.PHONY` entries.** The list covers `default bootstrap setup test lint fmt fmt-check check docker hooks vet` but omits `build`, `clean`, `install`, and `docker-run`. `build` is the live risk: the `Dockerfile` runs `make build` at line 31, and a file or directory named `build` in the repo root would make the target a silent no-op, producing an image with a stale or missing binary. **Targets that bypass `script/`.** The nine policy-named targets are correct thin shims, but `build` runs `go build` inline, and `vet`, `clean`, `install`, `docker-run` invoke tooling directly. `build` matters most because the Dockerfile depends on it, so real build logic lives in the Makefile rather than in `script/` where policy puts it. `vet` is also redundant — `script/test` already runs `go vet` first. ## Definition of done - `DOCKER_HOST` is no longer unconditionally assigned. Either remove the line entirely, or make it opt-in and overridable with `?=` and an empty default. - `make docker` works on a stock local Docker installation with no environment setup. - `.PHONY` includes `build`, `clean`, `install`, and `docker-run`. - `script/build` exists as a POSIX sh entrypoint following the conventions of the other scripts (`#!/bin/sh`, `set -eu`, the repo-root `cd` idiom), and `build:` becomes a thin shim to it. - The `Dockerfile` still builds successfully with the reworked `build` target. - The redundant `vet` target is removed, or kept with a one-line comment saying why it earns its place. - `make check` green. `TODO.md` updated in the same commit. ## Implementation requirements - Do not relocate the ldflags version-injection logic in a way that breaks it. Whatever moves into `script/build` must still produce a binary with the version string populated — build it and run the version command to confirm, since nothing in the test suite covers this. - Coordinate with #43 (module path rename), which also edits the ldflags lines. Whichever lands second must carry the other's change forward. - If removing the `DOCKER_HOST` line would disrupt your own workflow, the `?=` form plus a line in the README noting it can be set in the environment is the better option — but the default must be a working local build.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:42:32 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/secret#44