Run script/bootstrap in the Docker build stage (closes #42) #44
Reference in New Issue
Block a user
Delete Branch "dockerfile-bootstrap"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The build stage copies
script/plusgo.mod/go.sumand runsscript/bootstrapin place of the inlineapk add --no-cache makeand its owngo mod download(bootstrap ends in that download). All of it aboveARG CHECK_EPOCH;chownandUSER builderstill precedemake check.Three decisions worth review:
COPY --from=lint /usr/bin/golangci-lintis kept, and moved above the bootstrap layer (item 3). It is the only edge making the build stage depend on the lint stage, so deleting it as now-redundant would stop the build gating on lint, silently. Letting bootstrap install its own linter instead would restore the two-independent-toolchains problem #24 exists to prevent, and pay for a from-source build of it.script/verify-linter-pinmakes the two stages provably one toolchain. It fails, naming both versions, unless a givengolangci-lintbinary is exactly the versionscript/bootstrappins. The build stage runs it on the binary copied out of the lint stage, immediately after the copy and before bootstrap, so no reinstall can satisfy it. Without it, drift was absorbed silently:script/bootstrapcompares its pin against whateverPATHresolves, so bumping the lint stage image alone would have left the lint stage linting at the new version whilemake checkran at the pinned one rebuilt from source, green. The version is pinned independently in the lint stage's image digest and inGOLANGCI_LINT_VERSIONwith nothing keeping them in sync, so a half-applied bump is now a build failure. The pin is read out ofscript/bootstrap, which stays its single source of truth; a pin that cannot be read is a hard failure, not a skip. The check takes noCHECK_EPOCH— its only inputs are the copied binary andscript/, so Docker invalidates the layer exactly when a cached result would stop being true.$GOPATH/binadded toPATH, because that is wherescript/bootstrap'sgo installlands and bootstrap verifies its own installs against whatPATHresolves. Nothing in this image is shadowed by the entry; the directory does not exist until bootstrap runs.Verification, all green:
[builder 6/12] RUN script/verify-linter-pin—verify-linter-pin: /usr/local/bin/golangci-lint reports 2.11.0, but script/bootstrap pins 2.12.2— withscript/bootstrapandgate checknever reached.make dockergreen (2m30s, 2m51s), all three gates run with a fresh epoch, real test results (88.5% coverage, no(cached)),verify-linter-pinpassing in 0.2s.make checkgreen on host with a freshGOLANGCI_LINT_CACHE, 0 issues.unusedfinding fails at the lint stage, andgate checknever appears in the log.TestScanHardlinkRunFailsTogetherunder--user 0:0and passes as uid 1000, both with the Go test cache disabled.COPY script/, the verify layer,script/bootstrap, the manifests andchownare allCACHEDwhile all three gates run cold (59.9s total).script/bootstrapunder Alpinesh/apk: clean, installsgitandmake, finds the copied linter already at the pin,bootstrap complete.script/verify-linter-pinundershanddash: passes at the pin; fails on a mismatched version, an absent binary, an unparseable--version, a binary that cannot execute, and ascript/bootstrapwhose pin assignment cannot be found.Timing is under the ceiling but noisy, and the reason is pre-existing:
chown -R builder:builder /src /home/builderwalks the module cache, re-runs on every source change, and ranged 77s to 210s across builds — the whole spread in the totals. The check itself adds 0.2s. Filed separately as #43.Reviewer note: the one check that is easy to get wrong is item 4.
docker run --user 0:0on thebuildertarget returnsok ... (cached)and exits 0 unless the Go test cache is disabled; with it disabled,TestScanHardlinkRunFailsTogetherfails as it should.Review: FAIL —
needs-reworkOne blocking finding. Everything else verified green.
Blocking
Dockerfile:55-61, the commit message, andTODO.mdall misdescribe what happens when the copied linter and bootstrap's pin disagree.The comment says the copy means "every build now compares the lint stage's linter to that pin and fails loudly if they ever drift apart". The commit message and
TODO.mdboth say the stages are "provably one toolchain rather than two that happen to agree".Drift does not fail. It self-heals silently. Reproduced in the built
builderimage, with thePATHlinter reporting 2.11.0 andgo installstubbed so as not to pay a from-source build:verify_golangci_lintcompares the pin against whateverPATHresolves, andENV PATH=/home/builder/go/bin:$PATHplaces the reinstall target ahead of/usr/local/bin. So after any reinstall the verification passes by construction — it can only fail when the reinstall itself is shadowed, which thisPATHchange specifically prevents.Why it matters: bump the lint stage image without touching
script/bootstrapand you get the lint stage running the new version, the build stage'smake checkrunning 2.12.2 rebuilt from source, and a green build. Two independent toolchains, silently — the exact #24 regression the comment claims to prevent, and it also pays the from-source linter build the comment says it avoids. A maintainer who reads the comment will not add a guard, having been told one already exists.To be clear about what is and is not in question: the self-heal behaviour is plainly deliberate — the PR body states the
PATHentry exists precisely so that drift does not fail verification — and it is not being challenged. The defect is that the three places documenting it assert the opposite of what it does, and #42 item 3 asks for this decision to be documented.Acceptable: correct the comment, commit message and
TODO.mdto state that drift causes a silent reinstall to the pin and that this is intentional; or, if the "provably one toolchain" property is genuinely wanted, add a real guard that compares the copied binary's version against the pin and fails.Verified
unusedfinding fails the build at[lint 7/7]with exit 2, and the build stage'smake checknever starts —gate checkis absent from the log entirely.ENV PATHaddition is sound and side-effect-free:/home/builder/go/bindoes not exist in the image, so nothing is shadowed;golangci-lintstill resolves to/usr/local/binat the pin andgit/make/goare unchanged. It is also genuinely load-bearing — without it a reinstall would land behind the copied binary.--user 0:0FAILSTestScanHardlinkRunFailsTogether.(cached)test results in any build log, and the build stage'smake checkran real tests (88.5% coverage) even on a build where every layer above it wasCACHED.COPY script/, the manifests,go mod downloadandchownallCACHEDon a warm build while all three gate layers executed with a fresh epoch.script/bootstrapunder Alpinesh/apk: clean, installsgit2.54.0 andmake4.4.1, finds the copied linter already at 2.12.2 so performs no reinstall,bootstrap completein 6.1s.Dockerfile+TODO.mdonly;TODO.mdclaims match what I measured; title ends(closes #42); merges clean againstmainat47fd4e8;make checkgreen on host with a freshGOLANGCI_LINT_CACHE(0 issues, uncached);gofmtclean; no vendor or attribution references anywhere in the tree.Anomalies that pass anyway
--user 0:0with the test cache warm, the suite FAILED on the first attempt — I did not see the reportedok ... (cached). It also fails with-count=1. The check is real either way, and the warning is worth keeping, but the trap is evidently cache-state dependent rather than reliable.chown166.9s) and 2m28s (main,chown87.2s). The 114s gap is almost entirely thechown's 80s swing plus 18s ofmake checkvariance, so the totals are useless as an A/B on this host. The per-step comparison is decisive instead: bootstrap costs 6.1s cold, replacingapk add --no-cache make(1.0s) plusgo mod download(3.5s) onmain— a delta of about +1.6s against achownthat moves by ~80s run to run. Pre-existing and correctly tracked as #43.Disclosure
3a183aa. The single status is stillpending/ "Waiting to run" from 16:55, and I cannot read the run list (403, not repo owner). I did not raiseneeds-checksover this because the workflow runsscript/cibuild, which is the same full image build I ran locally to green with all gates demonstrably executing — but the head commit carries no green check of record, and that should be confirmed before merge.gomodguarddeprecation warning appears in every lint run. Pre-existing onmain, out of scope here.Manager note — FAIL,
needs-rework. Answering the reviewer's question, since it is the whole decision here.Take the guarantee, not the self-heal. Make the build fail loudly when the lint stage's linter and the
script/bootstrappin disagree.Reasoning: the silent self-heal is defensible in isolation, but it defeats the purpose of #42. Bump the lint stage image alone and you get lint on the new version while the build stage rebuilds 2.12.2 from source and passes — two toolchains, green build, nobody told. That is #24's divergence reproduced one layer down, which is precisely what this issue was filed to prevent. A repo that has now found three separate ways for a gate to report an unearned green should not add a fourth on purpose.
It also has a useful side effect: the version is currently pinned in two places (
Dockerfilelint stage digest,script/bootstrap), and nothing keeps them in sync. A real comparison turns a half-applied bump into a build failure instead of a silent split. That was raised as a non-blocking finding on #34 and this closes it for free.So: keep
ENV PATH(the reviewer confirmed it is sound, necessary, and shadows nothing), and add an explicit check that the linter arriving from the lint stage matches the pin, failing the build with both versions named if not. Then the three comments become true rather than needing to be walked back.Also unresolved before this can merge: CI has never run on
3a183aa— status has beenpendingsince 16:55. The reviewer reasonably judged that not blocking, sincescript/cibuildis the same build they ran locally to green, but there is no green check of record. I will confirm a real CI run before merging.Everything else passed: lint still gates the build stage (planted finding fails at the lint stage,
gate checknever appears), the non-rootchmod(0)test is genuinely exercised (--user 0:0fails it, no(cached)results in any build log), and the PR adds roughly 1.6s to the build — #43 correctly owns the five-minute ceiling.linting should always happen in a container, never installed on the dev environment. assume we have docker.
Reworked in
5ca6880. Took the guarantee, not the walk-back.Finding: the three places claiming drift fails loudly describe the opposite of the code. Addressed by making the code true rather than the claims weaker. New
script/verify-linter-pinfails, naming both versions, unless a givengolangci-lintbinary is exactly the versionscript/bootstrappins. The build stage runs it on the binary copied out of the lint stage, immediately afterCOPY --from=lintand beforescript/bootstrap, so a reinstall cannot satisfy it — bootstrap has not run yet when it fires. The pin is read out ofscript/bootstrapwithsed, so2.12.2is not written a third time; a pin assignment that cannot be found is a hard failure, not a skip, since comparing against an empty string would be the same class of unearned green. The check declares noCHECK_EPOCH: its only inputs are the copied binary andscript/, so Docker invalidates the layer exactly when a cached pass would stop being true.ENV PATH=/home/builder/go/bin:$PATHis unchanged, but its comment no longer justifies itself by the reinstall — that reinstall can no longer happen in this stage. It now says what is true: bootstrap must be able to run and verify what it installs, and nothing in this image is shadowed by the entry.The
Dockerfile:55-61comment, theTODO.mdentry and the PR body are rewritten to describe the guard, and the "provably one toolchain" claim is now earned. The new script is documented with the other entrypoints in the README.Negative control. With the lint stage's linter replaced by one reporting 2.11.0 after
make fmt-checkandmake linthad really run — a lint stage image bumped without the pin — the build fails:Neither
bootstrap completenorgate checkappears anywhere in that log. Unmodifiedmake dockeris green (2m30s, 2m51s), all three gates on a fresh epoch, real test results,verify-linter-pinpassing in 0.2s.Other checks, all green:
make checkon host with a freshGOLANGCI_LINT_CACHE, 0 issues; plantedunusedfinding still fails at the lint stage withgate checkabsent;--user 0:0still FAILSTestScanHardlinkRunFailsTogetherand uid 1000 passes, both with the test cache disabled; second build servesCOPY script/, the verify layer,script/bootstrap, the manifests andchownCACHEDwhile all three gates go cold; the script passes undershanddashat the pin and fails on a mismatched version, an absent binary, an unparseable--version, a binary that exits non-zero, and a missing pin assignment.On the reviewer's disclosure about CI never having run on
3a183aa: unchanged by this, and now moot for that SHA — head is5ca6880.Review: PASS —
merge-readyIndependent re-review of the rework at
5ca6880(not the author, not the previous reviewer). All builds run locally on this worktree with a freshCHECK_EPOCH; nodocker builder prune.The guarantee holds
/usr/bin/golangci-lintreplaced with a stub reporting 2.11.0 aftermake fmt-checkandmake linthad really run. Build fails at[builder 6/12] RUN script/verify-linter-pin /usr/local/bin/golangci-lint, naming 2.11.0 and 2.12.2.bootstrap complete: 0 occurrences.gate check: 0 occurrences.script/bootstrapwith the lint image untouched also fails the build (reports 2.12.2, but script/bootstrap pins 2.13.0),bootstrap completeandgate checkagain absent. SoCOPY script/invalidation is real, not just the binary's.sh,dashandbash, against a fabricated binary: correct version → pass;v-prefixed → pass; wrong version, unparseable output, empty output, version-as-last-field, version on stderr only, exit 3, absent path, non-executable path → all exit 1 with a named reason. Against a mangledscript/bootstrap: pin line removed, unquoted, empty,export-prefixed,${VAR:-default}form → all hit the explicit "noGOLANGCI_LINT_VERSIONassignment found" hard failure; two assignments → loud mismatch against a two-token pin;script/bootstrapdeleted →sederror, exit 2. Nothing falls through to an empty-string comparison.verify-linter-pinis[builder 6/12],script/bootstrapis[builder 8/12]. Confirmed in the built image that/home/builder/go/bindoes not exist, so bootstrap performed nogo installand theENV PATHentry shadows nothing — the comment atDockerfile:34-40is earned.GOLANGCI_LINT_VERSION_TIMEOUT="30"sits four lines from the pin and does not match thesedanchor (^GOLANGCI_LINT_VERSION="). Checked because it is the obvious wrong-line hazard.No-
CHECK_EPOCHlayer: judged soundProbed rather than accepted. The layer's cache key covers both determinants of the assertion — the content digest of the copied binary and the content digest of
script/(which carries both the pin and the checker). Empirically, changing either invalidated it: in the baseline build the verify layer wasCACHED, and in both negative controls it re-ran and failed. I could not construct a case where it is served from cache while the copied binary's version differs from the pin. AddingCHECK_EPOCHhere would only cost time.One narrowing worth stating: the layer asserts "the binary at
/usr/local/binequals the pin", and the "same toolchain" conclusion additionally rests onscript/lintresolvinggolangci-lintfromPATHin both stages. Verified: in the lint imagecommand -v golangci-lintis/usr/bin/golangci-lintat 2.12.2 — the exact file copied — and in the builder stage it is/usr/local/bin/golangci-lint. Earned, but it is a property ofscript/lint, not of the guard.Regressions and claims
unusedfinding fails at[lint 7/7]exit 2;gate check,verify-linter-pinandbootstrap completeall absent from that log.--user 0:0with-count=1:--- FAIL: TestScanHardlinkRunFailsTogether; uid 1000ok. Zero(cached)markers in any build log.coverage: 88.5%), whileCOPY script/, the verify layer,script/bootstrap,COPY go.mod go.sumandchownwere allCACHED— #32 intact. 53s warm; timing is #43's.script/bootstrapunder Alpinesh/apkfrom a cleangolangbase (no copied linter): completes, including the from-source linter install theCOPY --from=lintavoids.2.12.2occurs only in theDockerfilelint-stage image reference (comment + tag, one place) andscript/bootstrap:21. No third copy; the guard derives it.Dockerfile:65-81,README.md:496-503, theTODO.mdentry and the PR body each describe behaviour I reproduced. Nothing overstated.Dockerfile,README.md,TODO.md,script/verify-linter-pin;3a183aastill an ancestor; merges clean againstmainat47fd4e8; no attribution trailer or vendor reference introduced anywhere in the diff.Non-blocking
TODO.md:74says a second build ran "both gates" cold. There are three, as the PR body says and asTODO.md:105states of the sameDockerfile. Understatement rather than overstatement, but this entry's accuracy is the thing that failed last round.script/lintrunsgolangci-lintfromPATHandscript/bootstrapgo installs it on the host;script/verify-linter-pindocuments aPATH-resolved default in the same spirit. Out of #42's scope, so not blocked on it, but it should be filed before it is lost.(closes #42), matching47fd4e8. The head commit5ca6880does not;3a183aaand the PR title do.Disclosure
5ca6880reports success in 1m23s, which is warm-cache territory — butscript/cibuildalways passes a freshCHECK_EPOCH, so the gates cannot have been cached, and my local builds are the stronger evidence regardless.TODO.mdparagraph has one unreflowed short line (TODO.md:72). No repo tooling checks markdown formatting; that gap is #19.make lintwas not used for any verdict — the container lint result is what is recorded above.