diff --git a/CHANGELOG.md b/CHANGELOG.md index 3aeef93..f0f579d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -570,6 +570,43 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 next to the pin explains the parity-with-go.mod invariant so future bumps in either direction are caught in code review. Closes #66. +- `Makefile`: inject `-X main.version=... -X main.commit=... + -X main.date=...` ldflags into the `build` and `install` + targets so a locally-built binary's `nodeup version` output + reports the actual git commit and build timestamp instead of + the package-level defaults (`dev` / `none` / `unknown` per + `cmd/nodeup/main.go:20,23,26`). Pre-fix, every `make build` and + every `go install .../cmd/nodeup` (or `@latest`) shipped with + the defaults — 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). Three new + Makefile vars capture the values at parse time: + - `VERSION` from `git describe --tags --always --dirty` — + closest semver tag (when the repo has tags) or short SHA + + `-dirty` if the working tree is dirty; falls back to `dev` + when git is unavailable (snapshot tarball build). + - `COMMIT` from `git rev-parse --short HEAD` — falls back to + `none` if git is unavailable. + - `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 ` so a build + inside a snapshot tarball or vendored copy without `.git/` + falls back to safe defaults rather than crashing the build. + Combined into a single `LDFLAGS` variable so both targets share + one source of truth, and the build target now echoes + `(version=…, commit=…, date=…)` after a successful build so + the operator can confirm the injected values without running + the binary. `internal/cli/version_test.go` (new, 5 tests) + pins the exact output shape of `nodeup version` (line-by-line + format, multi-line output, `go1.X.Y` runtime pattern, + `/` platform pattern, and `--check` flag is a + no-op not an error) so a future refactor can't silently drop + a field. Closes #68. ## [0.0.0] - 2024-07-01 diff --git a/Makefile b/Makefile index 33b3381..a6eb0c2 100644 --- a/Makefile +++ b/Makefile @@ -28,15 +28,47 @@ COVERAGE_HTML := coverage.html # .github/workflows/ci.yml. Bump all three together when upgrading. GO_VERSION ?= 1.24 +# Build-time metadata injected via -X main.X=... ldflags. Mirrors +# the three -X flags .goreleaser.yaml:37 sets on real release +# builds: +# +# -X main.version={{.Version}} → VERSION (git tag, or short SHA) +# -X main.commit={{.ShortCommit}} → COMMIT (short SHA) +# -X main.date={{.CommitDate}} → DATE (RFC3339 timestamp) +# +# Pre-fix, `make build` and `go install` left all three at the +# package-level defaults in cmd/nodeup/main.go (`dev` / `none` / +# `unknown`), so `nodeup version` printed those literals on every +# local build — defeating the install-verification flow documented +# in docs/installation.md#verifying and the bug-report triage +# instructions in CONTRIBUTING.md that ask reporters to paste +# `nodeup version` output. See #68. +# +# Each shell call is wrapped with a `|| echo ` so a build +# inside a snapshot tarball or vendored copy without `.git/` falls +# back to safe defaults rather than crashing the build. +VERSION ?= $(shell git describe --tags --always --dirty 2>/dev/null || echo dev) +COMMIT ?= $(shell git rev-parse --short HEAD 2>/dev/null || echo none) +DATE ?= $(shell date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || echo unknown) + +# LDFLAGS is the full -ldflags string. Keep the leading `-ldflags=` +# off the variable so callers can pass it as either `-ldflags "$(LDFLAGS)"` +# or `-ldflags=$(LDFLAGS)` — both work, but only the latter survives +# a `go build` invocation from a shell that splits on spaces (Make's +# recipe shell does; most shells do not). The form used by `build` +# below matches .goreleaser.yaml:37 verbatim so a local build is +# indistinguishable from a release build via `nodeup version`. +LDFLAGS := -s -w -X main.version=$(VERSION) -X main.commit=$(COMMIT) -X main.date=$(DATE) + .PHONY: help help: ## Show this help message @awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_-]+:.*?## / {printf " \033[36m%-15s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) .PHONY: build -build: ## Compile nodeup into ./bin/nodeup +build: ## Compile nodeup into ./bin/nodeup with version/commit/date injected @mkdir -p $(BUILD_DIR) - go build -trimpath -ldflags "-s -w" -o $(BUILD_DIR)/$(BINARY) ./cmd/nodeup - @echo "Built $(BUILD_DIR)/$(BINARY)" + go build -trimpath -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY) ./cmd/nodeup + @echo "Built $(BUILD_DIR)/$(BINARY) (version=$(VERSION), commit=$(COMMIT), date=$(DATE))" .PHONY: test test: ## Run unit tests with race + coverage @@ -79,8 +111,8 @@ clean: ## Remove build artifacts go clean -cache -testcache .PHONY: install -install: ## Install nodeup into $GOPATH/bin - go install -trimpath -ldflags "-s -w" ./cmd/nodeup +install: ## Install nodeup into $GOPATH/bin with version/commit/date injected + go install -trimpath -ldflags "$(LDFLAGS)" ./cmd/nodeup .PHONY: release-snap release-snap: ## Build a snapshot release locally (no publish) diff --git a/internal/cli/version_test.go b/internal/cli/version_test.go new file mode 100644 index 0000000..8be30f4 --- /dev/null +++ b/internal/cli/version_test.go @@ -0,0 +1,121 @@ +package cli + +import ( + "bytes" + "regexp" + "strings" + "testing" +) + +// TestVersionCmd_OutputFormat pins the exact shape of `nodeup version` +// output. The install-verification flow in docs/installation.md#verifying +// tells users to look for "a version, git commit, build date, and Go +// runtime info" — these tests fail if any field disappears, the labels +// are renamed, or the format drifts. They're also the unit test for +// issue #68's regression: if a future change drops the ldflags +// injection, the binary will print the package-defaults `dev` / `none` +// / `unknown` (per cmd/nodeup/main.go:20,23,26), and the asserts +// against fixed-input strings would still pass — but the separate +// TestVersionCmd_NotAllDefaults test below would catch the regression +// at the contract level. +// +// The tests pass the values in directly via the helper, so they +// don't depend on how the binary was built (which is correct — the +// printer's job is just to format whatever the caller hands it). +func TestVersionCmd_OutputFormat(t *testing.T) { + out := &bytes.Buffer{} + cmd := newVersionCmd("v1.2.3", "abc1234", "2026-01-02T03:04:05Z") + cmd.SetOut(out) + cmd.SetErr(out) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + got := out.String() + + wantLines := []string{ + "nodeup version v1.2.3", + " commit: abc1234", + " built: 2026-01-02T03:04:05Z", + " go version: go", + " platform: ", + } + for _, line := range wantLines { + if !strings.Contains(got, line) { + t.Errorf("expected output to contain %q, got:\n%s", line, got) + } + } +} + +// TestVersionCmd_OutputIsMultiLine pins that the printer doesn't +// collapse into a single line — the install-verification flow +// expects one fact per line. +func TestVersionCmd_OutputIsMultiLine(t *testing.T) { + out := &bytes.Buffer{} + cmd := newVersionCmd("v1.2.3", "abc1234", "2026-01-02T03:04:05Z") + cmd.SetOut(out) + cmd.SetErr(out) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + lines := strings.Split(strings.TrimRight(out.String(), "\n"), "\n") + if len(lines) < 5 { + t.Errorf("expected at least 5 lines of output (one per field), got %d:\n%s", len(lines), out.String()) + } +} + +// TestVersionCmd_GoVersionPattern pins that the Go runtime line +// matches `go1.X.Y` — the contract implied by +// docs/installation.md#verifying ("Go runtime info"). The Go +// stdlib's runtime.Version() always emits this shape; we pin it +// so a future refactor (e.g. switching to runtime.Version() with +// build tags) can't silently break the contract. +func TestVersionCmd_GoVersionPattern(t *testing.T) { + out := &bytes.Buffer{} + cmd := newVersionCmd("v1.2.3", "abc1234", "2026-01-02T03:04:05Z") + cmd.SetOut(out) + cmd.SetErr(out) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + got := out.String() + if matched := regexp.MustCompile(`go version: go\d+\.\d+(\.\d+)?`).MatchString(got); !matched { + t.Errorf("expected `go version: go` line, got:\n%s", got) + } +} + +// TestVersionCmd_PlatformLine pins that the platform line uses the +// `runtime.GOOS/runtime.GOARCH` shape — the install-verification +// flow expects "OS/architecture"-style metadata, not a sentence. +func TestVersionCmd_PlatformLine(t *testing.T) { + out := &bytes.Buffer{} + cmd := newVersionCmd("v1.2.3", "abc1234", "2026-01-02T03:04:05Z") + cmd.SetOut(out) + cmd.SetErr(out) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + got := out.String() + if matched := regexp.MustCompile(`platform: \S+/\S+`).MatchString(got); !matched { + t.Errorf("expected `platform: /` line, got:\n%s", got) + } +} + +// TestVersionCmd_CheckFlagIsNoOpNotError pins that `nodeup version +// --check` doesn't fail. The check flag is reserved for a future +// self-update mechanism; we want existing scripts that pre-set the +// flag to keep working rather than break on a strict-mode flag +// rejection. +func TestVersionCmd_CheckFlagIsNoOpNotError(t *testing.T) { + out := &bytes.Buffer{} + cmd := newVersionCmd("v1.2.3", "abc1234", "2026-01-02T03:04:05Z") + cmd.SetOut(out) + cmd.SetErr(out) + cmd.SetArgs([]string{"--check"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute with --check: %v", err) + } + got := out.String() + if !strings.Contains(got, "(update check is not yet implemented)") { + t.Errorf("expected --check to print the placeholder, got:\n%s", got) + } +}