sneak/sfdupes
1
0
Fork 0
Code Issues 23 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

script/bootstrap never checks the golangci-lint version, so the pin is inert #24

New Issue
Closed
opened 2026-08-09 03:58:15 +02:00 by clawbot · 1 comment
clawbot commented 2026-08-09 03:58:15 +02:00
Collaborator
Copy Link
Copy Source

script/bootstrap:74 installs the pinned linter only when the command is absent:

if missing golangci-lint; then go install "$GOLANGCI_LINT_REF"; fi

missing() is ! command -v "$1". So on any machine that already has some golangci-lint on PATH, GOLANGCI_LINT_REF is never consulted and the pin does nothing. Bumping the version in script/bootstrap — which is exactly what #3 just did — has no effect on such a machine, forever.

This is not hypothetical. Reproduced on the manager host immediately after #3 merged:

$ make check
ok    sneak.berlin/go/sfdupes  (cached)  coverage: 72.8% of statements
0 issues.
$ golangci-lint --version
golangci-lint has version 2.10.1 built with go1.26.5

So make check reported green while running a linter two minor versions behind the pin, against a config written for a newer one. The Docker lint stage uses the digest-pinned image and is unaffected, which means local and CI can disagree silently — the worst possible failure mode for a gate, because the disagreement only shows up after a push.

Pre-existing since 3abeacf; surfaced by the independent review of PR #2.

Definition of done

  1. script/bootstrap compares the installed version against the pin and reinstalls when it differs, rather than testing only for presence. Parse golangci-lint --version and match the pinned version exactly.
  2. The pinned version lives in exactly one place in the script, so future bumps cannot half-apply.
  3. The same presence-only pattern is audited for the other tools script/bootstrap installs (git, make, go); document deliberately unpinned system tools rather than silently leaving them.
  4. Running script/bootstrap on a host with a wrong-version golangci-lint upgrades it, and a second run is a no-op.
  5. make check and make docker green, and a local make lint uses the same linter version as the Dockerfile lint stage.
`script/bootstrap:74` installs the pinned linter only when the command is absent: if missing golangci-lint; then go install "$GOLANGCI_LINT_REF"; fi `missing()` is `! command -v "$1"`. So on any machine that already has *some* `golangci-lint` on `PATH`, `GOLANGCI_LINT_REF` is never consulted and the pin does nothing. Bumping the version in `script/bootstrap` — which is exactly what #3 just did — has no effect on such a machine, forever. This is not hypothetical. Reproduced on the manager host immediately after #3 merged: $ make check ok sneak.berlin/go/sfdupes (cached) coverage: 72.8% of statements 0 issues. $ golangci-lint --version golangci-lint has version 2.10.1 built with go1.26.5 So `make check` reported green while running a linter two minor versions behind the pin, against a config written for a newer one. The Docker lint stage uses the digest-pinned image and is unaffected, which means local and CI can disagree silently — the worst possible failure mode for a gate, because the disagreement only shows up after a push. Pre-existing since `3abeacf`; surfaced by the independent review of PR #2. ## Definition of done 1. `script/bootstrap` compares the installed version against the pin and reinstalls when it differs, rather than testing only for presence. Parse `golangci-lint --version` and match the pinned version exactly. 2. The pinned version lives in exactly one place in the script, so future bumps cannot half-apply. 3. The same presence-only pattern is audited for the other tools `script/bootstrap` installs (`git`, `make`, `go`); document deliberately unpinned system tools rather than silently leaving them. 4. Running `script/bootstrap` on a host with a wrong-version `golangci-lint` upgrades it, and a second run is a no-op. 5. `make check` and `make docker` green, and a local `make lint` uses the same linter version as the Dockerfile lint stage.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:58:15 +02:00
clawbot commented 2026-08-09 07:49:33 +02:00
Author
Collaborator
Copy Link
Copy Source

Implementation plan (branch bootstrap-version-check, from main at
076d822):

  1. Split the pin into a single source of truth in script/bootstrap:
    GOLANGCI_LINT_VERSION="2.12.2" (no leading v, matching what the
    binary prints), with GOLANGCI_LINT_REF derived from it as
    github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v$GOLANGCI_LINT_VERSION.
    A future bump then edits exactly one string, and the install ref and
    the comparison value cannot drift apart (DoD 2).

  2. Add a golangci_lint_version() helper that returns the installed
    version or the empty string when the tool is absent. It returns
    early if command -v golangci-lint fails (so a missing binary is
    just "no version" rather than an error under set -eu), otherwise it
    parses golangci-lint --version with awk, taking the field after
    the literal word version and stripping an optional leading v.
    That handles both shapes seen in the wild: this host's
    golangci-lint has version 2.10.1 built with go1.26.5 from (unknown, modified: ?, mod sum: "...") on (unknown) and the pinned image's
    version-plus-commit line.

  3. Replace the presence test with a version test (DoD 1):

    installed="$(golangci_lint_version)"
if [ "$installed" != "$GOLANGCI_LINT_VERSION" ]; then
    go install "$GOLANGCI_LINT_REF"
fi

    which covers absent, older and newer alike, and is a no-op on the
    second run (DoD 4). It prints what it found and what it wants before
    installing, so the upgrade is visible in the bootstrap output.

  4. Audit the other installs (DoD 3): git, make and go come from
    the host package manager and are deliberately unpinned — the repo
    pins no system toolchain versions and go.mod governs the language
    version. Rather than leave that ambiguous next to a tool that now
    is version-checked, add a comment above the three missing calls
    stating that presence-only is intentional for system packages, and a
    comment on the linter block stating why it is different (a stale
    linter silently disagrees with the digest-pinned Dockerfile lint
    stage, which is exactly the failure this issue is about). No system
    package gets pinned.

POSIX sh throughout (#!/bin/sh, set -eu, no bashisms) since Alpine
has no bash; existing structure and comment style preserved.

Verification (DoD 4 and 5): this host currently has v2.10.1 against the
v2.12.2 pin, so it is a live reproduction — run script/bootstrap and
show the upgrade, run it again and show the no-op, report
golangci-lint --version before and after, then run make check and
make docker and confirm the local linter now matches the Dockerfile
lint stage. If v2.12.2 surfaces findings on main that v2.10.1 was
hiding, those get reported rather than fixed here — out of scope for
this issue. TODO.md gets a Completed Steps entry in the same commit.

Implementation plan (branch `bootstrap-version-check`, from `main` at `076d822`): 1. Split the pin into a single source of truth in `script/bootstrap`: `GOLANGCI_LINT_VERSION="2.12.2"` (no leading `v`, matching what the binary prints), with `GOLANGCI_LINT_REF` derived from it as `github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v$GOLANGCI_LINT_VERSION`. A future bump then edits exactly one string, and the install ref and the comparison value cannot drift apart (DoD 2). 2. Add a `golangci_lint_version()` helper that returns the installed version or the empty string when the tool is absent. It returns early if `command -v golangci-lint` fails (so a missing binary is just "no version" rather than an error under `set -eu`), otherwise it parses `golangci-lint --version` with `awk`, taking the field after the literal word `version` and stripping an optional leading `v`. That handles both shapes seen in the wild: this host's `golangci-lint has version 2.10.1 built with go1.26.5 from (unknown, modified: ?, mod sum: "...") on (unknown)` and the pinned image's version-plus-commit line. 3. Replace the presence test with a version test (DoD 1): installed="$(golangci_lint_version)" if [ "$installed" != "$GOLANGCI_LINT_VERSION" ]; then go install "$GOLANGCI_LINT_REF" fi which covers absent, older and newer alike, and is a no-op on the second run (DoD 4). It prints what it found and what it wants before installing, so the upgrade is visible in the bootstrap output. 4. Audit the other installs (DoD 3): `git`, `make` and `go` come from the host package manager and are deliberately unpinned — the repo pins no system toolchain versions and `go.mod` governs the language version. Rather than leave that ambiguous next to a tool that now *is* version-checked, add a comment above the three `missing` calls stating that presence-only is intentional for system packages, and a comment on the linter block stating why it is different (a stale linter silently disagrees with the digest-pinned Dockerfile lint stage, which is exactly the failure this issue is about). No system package gets pinned. POSIX `sh` throughout (`#!/bin/sh`, `set -eu`, no bashisms) since Alpine has no bash; existing structure and comment style preserved. Verification (DoD 4 and 5): this host currently has v2.10.1 against the v2.12.2 pin, so it is a live reproduction — run `script/bootstrap` and show the upgrade, run it again and show the no-op, report `golangci-lint --version` before and after, then run `make check` and `make docker` and confirm the local linter now matches the Dockerfile lint stage. If v2.12.2 surfaces findings on `main` that v2.10.1 was hiding, those get reported rather than fixed here — out of scope for this issue. `TODO.md` gets a Completed Steps entry in the same commit.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
Review passed; ready to merge
 needs-checks
CI checks failing or not yet green
 needs-rebase
Branch conflicts with main; rebase required
 needs-review
PR awaiting independent adversarial review
 needs-rework
Review found blocking problems; author must rework
No labels
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/sfdupes#24