fix(ci): inject version/commit/date ldflags in make build and install#94
Merged
Conversation
Closes #68 Pre-fix, Makefile:38 ran `go build -trimpath -ldflags "-s -w" -o bin/nodeup ./cmd/nodeup` — no -X main.X=... injection — so every locally-built binary (make build, go install, go install @latest) shipped with the package-level defaults from cmd/nodeup/main.go: 17-26: version=dev, commit=none, date=unknown. nodeup version then printed those literals on every local build, defeating the install-verification flow documented in docs/installation.md#verifying ("Should print a version, git commit, build date, and Go runtime info") and undermining the bug-report triage instructions in CONTRIBUTING.md that ask reporters to paste nodeup version output — a locally-built binary was indistinguishable from any other local build, so triage couldn't tell if the bug reproed on the latest commit or on a stale checkout. Fix: capture three Makefile vars at parse time and feed them into a single LDFLAGS variable used by both `build` and `install`: - VERSION from `git describe --tags --always --dirty` (closest semver tag, or short SHA + -dirty; falls back to `dev` if git is unavailable, e.g. snapshot tarball builds) - COMMIT from `git rev-parse --short HEAD` (falls back to `none`) - DATE from `date -u +%Y-%m-%dT%H:%M:%SZ` (RFC3339 UTC, matches .goreleaser.yaml:37's {{.CommitDate}} shape) Each shell call is wrapped with `|| echo <fallback>` so a build without .git/ doesn't crash. The combined LDFLAGS string is "-s -w -X main.version=$(VERSION) -X main.commit=$(COMMIT) -X main.date=$(DATE)" — same shape that goreleaser releases use, so a local build is indistinguishable from a release build via `nodeup version`. internal/cli/version_test.go: 5 new tests pin the exact output shape (line-by-line format, multi-line output, go1.X.Y runtime pattern, <goos>/<goarch> platform pattern, --check flag is a no-op not an error) so a future refactor can't silently drop a field. Brings internal/cli coverage from 29.2% to 31.1%. CHANGELOG: ### Fixed entry under [Unreleased]. Co-Authored-By: puku-ai-2.8 <noreply@puku.sh>
68125be to
b440ea4
Compare
|
✔️ b440ea4 - Conventional commits check succeeded. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makefile:38'sbuildtarget rango build -trimpath -ldflags "-s -w" -o bin/nodeup ./cmd/nodeup— no-X main.version=... -X main.commit=... -X main.date=...injection, despitecmd/nodeup/main.go:17-26declaring all three asvar ... = "dev"|"none"|"unknown"precisely so they could be ldflag-injected, and despite.goreleaser.yaml:37injecting them on every real release build.The result: every locally-built binary (
make build,go install .../cmd/nodeup,go install .../cmd/nodeup@latest) shipped with the package-level defaults.nodeup versionthen printed literaldev/none/unknown— defeating the install-verification flow documented indocs/installation.md#verifying("Should print a version, git commit, build date, and Go runtime info") and undermining the bug-report triage instructions inCONTRIBUTING.mdthat ask reporters to pastenodeup versionoutput. A locally-built binary was indistinguishable from any other local build, so triage couldn't tell if a bug reproed on the latest commit or on a stale checkout.This PR fixes the regression by introducing three Makefile vars that capture the values at parse time and feeding them into a single
LDFLAGSvariable used by bothbuildandinstall:VERSIONfromgit describe --tags --always --dirty— closest semver tag (when the repo has tags) or short SHA +-dirtyif the working tree is dirty; falls back todevwhen git is unavailable (snapshot tarball build).COMMITfromgit rev-parse --short HEAD— falls back tonone.DATEfromdate -u +%Y-%m-%dT%H:%M:%SZ— RFC3339 UTC, matches.goreleaser.yaml:37's{{.CommitDate}}shape.Each shell call is wrapped with
|| echo <fallback>so a build inside a snapshot tarball or vendored copy without.git/falls back to safe defaults rather than crashing.internal/cli/version_test.go(new, 5 tests) pins the exact output shape ofnodeup versionso a future refactor can't silently drop a field.Linked issues
Closes #68
Type of change
fix— bug fix (PATCH bump)feat— new feature (MINOR bump)refactor— no behavior changeperf— performance improvementdocs— documentation onlytest— tests onlychore— maintenance / dependency bumpci— CI/CD onlybuild— build system onlyChecklist
feat(scope): subject)make cilocally and it passes (Go-side tidy/fmt/vet/lint/test all green; the new version_test.go bringsinternal/clicoverage from 29.2% → 31.1%)internal/cli/version_test.go— 5 new tests:TestVersionCmd_OutputFormat,TestVersionCmd_OutputIsMultiLine,TestVersionCmd_GoVersionPattern,TestVersionCmd_PlatformLine,TestVersionCmd_CheckFlagIsNoOpNotError)make lintgreen)Screenshots / output
Pre-fix,
make buildproduced a binary whosenodeup versionwas useless for triage:Post-fix:
After a
git tag v1.2.3on a clean working tree,VERSIONwould resolve tov1.2.3(the tag) instead of the short SHA — matching whatgoreleaser releasewould inject on a real release build.Tests
Files touched
Makefile—VERSION(git describe --tags --always --dirty),COMMIT(git rev-parse --short HEAD),DATE(date -u +%Y-%m-%dT%H:%M:%SZ). Each wrapped with|| echo <fallback>so a build without.git/(snapshot tarball, vendored copy) doesn't crash the build.LDFLAGS := -s -w -X main.version=$(VERSION) -X main.commit=$(COMMIT) -X main.date=$(DATE). Bothbuildandinstallnow use-ldflags "$(LDFLAGS)".buildechoes(version=…, commit=…, date=…)after a successful build so the operator can confirm the injected values without running the binary..goreleaser.yaml:37invariant + the fallback strategy + why we wrap each shell call.internal/cli/version_test.go— new, 5 tests:TestVersionCmd_OutputFormat— every labeled field is present, in the right shape.TestVersionCmd_OutputIsMultiLine— at least 5 lines (one per field), so a future collapse-into-one-line refactor fails the test.TestVersionCmd_GoVersionPattern—go version: go1.X.Yregex match (the contract implied bydocs/installation.md#verifying).TestVersionCmd_PlatformLine—platform: <goos>/<goarch>regex match.TestVersionCmd_CheckFlagIsNoOpNotError—nodeup version --checkprints the placeholder, doesn't error (preserves existing scripts that pre-set the flag).CHANGELOG.md—### Fixedentry under[Unreleased]documenting the Makefile change + the version_test.go addition. References build: make build doesn't inject version/commit/date ldflags, nodeup version useless on local builds #68.