Stamp the build version into the binary (closes #253) #260
Reference in New Issue
Block a user
Delete Branch "issue-253-version-stamping"
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?
Closes #253.
main.versionwas a placeholder nothing ever set: neithermake buildnor theDockerfilepassed-X. A tagged release produced an artifact that could not say which commit it was.What changed
script/versionis the single source of the value:$VERSIONwhen set and non-empty, elsegit describe --tags --always --dirtyagainst this checkout, elseunknown. It refuses to use an enclosing repository's metadata (a tarball unpacked inside an unrelated working copy is not that copy's version). Nothing time-, host- or builder-dependent is stamped.Makefile:VERSION ?= $(shell script/version), plus aGO_LDFLAGShook;buildcompiles with-ldflags '-X main.version=$(VERSION) $(GO_LDFLAGS)'. Newmake versionprints what the checkout would stamp.Dockerfile:.dockerignoreexcludes.git/, so the builder stage cannot derive anything — the version enters asARG VERSION, defaulted tounknown.script/dockerfills it from the host checkout. Both compiles in the image now go throughmake build, the static relink contributing its-extldflagsthroughGO_LDFLAGSrather than replacing-ldflags, so it cannot drop the stamp. TheARGsits below the test and asset layers so a new version does not invalidate their cache.base.htmlrenders.Version, which nothing ever put in the template data, so it printed its literaldevfallback regardless of how the binary was built.renderTemplatenow supplies it on both the map and the wrapper path — one line ininternal/handlers/handlers.go, the only file touched outside the build files,README.mdand tests.Not changed: no timestamp, no commit date, no builder identity — see reproducibility below.
What each build reports
v1.0.0-3-g1a2b3c4-dirtyVERSIONunknownVerification
make checkgreen withGOFLAGS=-count=1: 21 packagesok, 572 tests passed,0 issues.from the containerized linter.make build, checkout at a real tag (throwaway local tagv0.0.0-impl253, deleted before pushing), run on port 18860:Exactly the tag, no suffix.
make dockerat the same tag, container run from the built image:Neither build layer was
CACHED. The stamp is read off the running binary, not the command line, so it survives the static relink — and it is running on Alpine, which a non-static binary could not do. Footer and startup log from the same container:Reproducibility. Two
make buildruns of the same commit,bin/webhookerremoved in between:Byte-identical, so the gate in #111 still holds.
No git metadata.
git archive HEADunpacked to a directory with no.git:And a bare
docker build .with no--build-arg(the context never carries.git):It builds, it runs, and it claims no tag.
Tests. New
internal/versionscriptpackage, mirroringinternal/ciscript: eleven tests over throwaway git repositories — clean-at-a-tag, past a tag, untagged, dirty, no.git, enclosing repository ignored,VERSIONoverride, emptyVERSION, stability across invocations — plus guards that theMakefilestill composes both halves of the linker flags and that no rawgo buildreappears in theDockerfile.TestFooterReportsStampedVersionininternal/handlersrenders the login page and asserts the footer carries the version rather than thedevfallback.Disclosures
script/dockeris one of the scripts REPO_POLICIES.md expects to stay byte-identical across repos. It now passes--build-arg VERSION="$(script/version)", because the version can only be resolved on the host. Its header comment records why.internal/handlers/handlers.gois outside the build files, but the footer is a version-reporting site named in the issue's definition of done and it was broken independently of the linker flags. The change is confined torenderTemplateand one struct field.{{else}}dev{{end}}fallback inbase.htmlis left in place; withrenderTemplatesupplying the value it is now unreachable in a wired application.PASS.
Verified by execution on
ec92992in an independent clone: all four version states across all three reporting sites (healthcheck, rendered footer HTML, startup log) — clean tag reports exactly the tag, untagged reports the short SHA, dirty carries-dirty, and both no-.gitpaths (git archivetarball,script/cibuild's baredocker build .) reportunknown. The image binary isstatically linked/not a dynamic executablewith-ldflags="-X main.version=... -extldflags \"-static\""recorded in it, so the relink kept both halves. Two hostmake buildruns and adocker build --no-cache-filter=builderrebuild at the same version were byte-identical; a changedVERSIONre-ran both build layers and genuinely restamped whileRUN make teststayedCACHED.make checkgreen withGOFLAGS=-count=1in 1m39s (21ok, 0FAIL, 0(cached), 0 skips,0 issues.from the containerized linter). CI green on the head commit; fast-forwards ontonext; no attribution trailers;(closes #253)present.The second half of the defect is real: reverting only
internal/handlers/handlers.gotonextmakesTestFooterReportsStampedVersionfail with<span>dev</span>in the rendered footer. Two further mutation probes caught — a revertedMakefilefailsTestMakefile_BuildComposesVersionAndExtraFlags, and neuteringin_this_checkoutinscript/versionfailsTestVersion_EnclosingRepositoryIsNotUsed(it then stamps the enclosing repo'sv9.9.9).One finding, non-blocking
Makefile:41— an explicitly emptyVERSIONreintroduces the exact defect this issue is about.make build VERSION=producesgo build -ldflags '-X main.version='. Run: healthcheck reports"version":"", the startup log reportsversion="", and the footer falls back to<span>dev</span>because{{if .Version}}is false on the empty string. The Docker path is equally reachable:--build-arg VERSION=overridesARG VERSION=unknownwith the empty string (verified against a scratch image), sodocker build --build-arg VERSION= .ships a binary that reports nothing at all three sites.Why it matters beyond the typo case: this is exactly the "set but unusable, silently defaults" shape the repo rejects elsewhere (
PORT=eightyaborts startup rather than substituting).script/versionalready guards it andTestVersion_EmptyOverrideFallsBackToGitcovers it — but the guard sits only on the env path, and aVERSION=handed tomakeor to--build-argnever reaches the script.Acceptable would be collapsing an empty override in the one place that composes the flag, e.g.
VERSION := $(if $(strip $(VERSION)),$(VERSION),unknown)after the?=, with a case ininternal/versionscript. Not reachable throughmake docker,script/cibuild, or any current workflow, so it does not block the merge.Notes and disclosures
VERSIONas an environment override needs no guard.REPO_POLICIES.md's own canonical Dockerfile prescribesARG VERSION, so a caller naming the build is the intended contract, not a hole.script/dockerdeviation: the justification holds. The value is only resolvable host-side, and the alternatives (shipping.gitin the build context, or deriving it in the image) are worse. It does leavescript/dockerno longer byte-identical across repos asREPO_POLICIES.mdstates; the durable fix is the model script atsneak/prompts, which is your call and not this PR's.script/cibuildis unchanged, so CI-built images stampunknown. Harmless today — no image-publishing workflow exists and the CI image is discarded — but it means no green CI run ever exercises a real stamp.ARG VERSION=unknowndiverges fromREPO_POLICIES.md's modelARG VERSION=dev. Deliberate, documented in the README table, and better (it does not collide withmain.version's own default); flagging the divergence only.-trimpathnor-s -w, both of whichREPO_POLICIES.md's model Dockerfile carries;-trimpathis the one that bears on reproducibility across build directories. Thegomodguarddeprecation warning is tracked elsewhere.go test -runwas invoked directly for the three mutation probes (nomaketarget selects a single test), anddocker build --no-cache-filter=builderfor the uncached-rebuild comparison. No repo file was left modified and the throwaway tag was deleted.No scope creep —
internal/handlers/handlers.gois a version-reporting site named in the definition of done of #253.ec92992450tofec6876c42Rework for the empty-override finding. Rebased onto
next(5fda446) and amended into the single commit, nowfec6876.The suggested
VERSION := $(if ...)does not work: a plain makefile assignment loses to a command-line definition, which is exactly the case being corrected. Verified — with that line,make version VERSION=still prints the empty string. The guard needsoverride, so the Makefile now carries:override VERSION := $(or $(strip $(VERSION)),$(shell script/version))One deviation from the suggested fix: an empty override resolves through
script/versionrather than landing on a literalunknown. That keeps one meaning of "empty" across both paths —TestVersion_EmptyOverrideFallsBackToGitalready defines empty as unset for the env path, and a second rule where a make override of empty discards a derivable version would contradict it. Where nothing is derivable it still lands onunknown, which is the case that matters: no.git, so the Docker path is unaffected.No Dockerfile guard is needed.
--build-arg VERSION=reaches make as a command-line definition viamake build VERSION="$VERSION", so the Makefile line closes it. The image built with--build-arg VERSION=links-ldflags="-X main.version=unknown -extldflags \"-static\""and isstatically linked; healthcheck, footer and startup log all reportunknown.New test
TestMakefile_EmptyOverrideResolvesLikeAnUnsetOneininternal/versionscript. It fails without the Makefile change — on theoverride VERSION :=assertion, and behaviourally too (make versionunder an empty VERSION printed[]before the fix,[09a5029-dirty]after).make checkgreen withGOFLAGS=-count=1after the rebase: 21 packages, 0 cached, lint 0 issues in Docker. Reproducibility unaffected — twomake buildruns of the same commit are byte-identical, and aVERSION=build hashes identically to the default one.README gained one sentence stating that an empty override means unset.
PASS. Re-verified by execution on
fec6876in an independent clone: all threemake versioncases correct (VERSION=resolves to the derived value,VERSION=v1.2.3still wins, whitespace-only also falls back); theoverrideis genuinely required, not cargo-culted — removed in a throwaway copy,make version VERSION=prints the empty string and the new test fails on both its textual and its behavioural assertion.docker build --build-arg VERSION=end to end reportsunknownon the healthcheck, in the footer and in the startup log, binaryNot a valid dynamic programwith both ldflags halves recorded;--build-arg VERSION=v0.0.0-rr260likewise reports that value at all three sites (author's stated gap closed) and re-ran only the two build layers,RUN make teststayingCACHED. Tag case on the host reports exactly the tag; no-.gittree reportsunknown, including under an empty override. Twomake buildruns byte-identical, and aVERSION=build hashes identically to the default one.make checkgreen withGOFLAGS=-count=1in 1m34s (21ok, 0(cached), 0FAIL,0 issues.from the pinned containerized linter). CI green on the head commit, fast-forwards ontonext,(closes #253)present, no attribution trailers.Empty-override semantics — resolving through
script/versionrather than to a literalunknown— accepted. I could not construct a path where it yields a misleading stamp: on the host the derived value describes the tree actually being compiled, and in the image.dockerignoreremoves.git/so it lands onunknownregardless. Nothing in the definition of done of #253 conflicts with it.Notes and disclosures:
override VERSION :=is simply-expanded, soscript/version(agit describe) now runs at parse time on everymakeinvocation, includingmake test/make lint/make check— one call per invocation, milliseconds. Flagging, not objecting.make checkfailed onTestVendoredAssetsMatchManifestandTestBaseTemplateScriptsAreServed: a fresh clone withoutmake assets. Pre-existing and self-diagnosing; the green run above is aftermake assets.git describe. The asserted invariant (two runs identical) holds.Makefilein a throwaway copy under/tmpand rango test -rundirectly against that copy, since nomaketarget selects a single test. The reviewed tree was left unmodified, the throwaway tag existed only in a copy and was never pushed, and everyrereview-260-container and image has been removed.