Skip to content

feat(go-app/release): inject main.buildTime ldflag#74

Merged
CybotTM merged 1 commit intomainfrom
feat/ldflags-buildtime
Apr 20, 2026
Merged

feat(go-app/release): inject main.buildTime ldflag#74
CybotTM merged 1 commit intomainfrom
feat/ldflags-buildtime

Conversation

@CybotTM
Copy link
Copy Markdown
Member

@CybotTM CybotTM commented Apr 20, 2026

Copilot review on ldap-manager#561 noted that internal/version.BuildTimestamp stays "n/a" in release builds because the template only injects main.version + main.build.

Extend the ldflags with -X main.buildTime=${{ github.event.head_commit.timestamp }} so repos that declare var buildTime string in package main can surface the commit timestamp. Repos that don't declare the var pay a silent no-op.

Empty when github.event.head_commit is unavailable (e.g. some workflow_dispatch backfills); empty is harmless — consumers test if buildTime != "" before forwarding.

Copilot review on ldap-manager#561 noted that internal/version.BuildTimestamp
stays 'n/a' in release builds because the template only injects
main.version + main.build. Extend the ldflags with
-X main.buildTime=${{ github.event.head_commit.timestamp }}
so repos that declare 'var buildTime string' in package main can
surface the commit timestamp (ldap-manager.FormatVersion()'s third
metadata field). Repos that don't declare the var pay a silent
no-op.

Empty when github.event.head_commit is unavailable (e.g. certain
workflow_dispatch backfills); an empty value is harmless because
consumers test 'if buildTime != ""' before forwarding.

Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
Copilot AI review requested due to automatic review settings April 20, 2026 19:22
@CybotTM CybotTM merged commit 4e23028 into main Apr 20, 2026
@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@CybotTM CybotTM deleted the feat/ldflags-buildtime branch April 20, 2026 19:22
CybotTM added a commit to netresearch/ldap-manager that referenced this pull request Apr 20, 2026
## Summary

Addresses copilot-review follow-ups from
[#564](#564).

### 1. BuildTimestamp actually populates

`FormatVersion()` previously reported `built at n/a` even on tagged
releases because the template injected only `main.version` +
`main.build`.
[netresearch/.github#74](netresearch/.github#74)
extended the template with `-X main.buildTime=<commit-timestamp>`; this
PR adds `var buildTime string` and forwards it into
`internal/version.BuildTimestamp`.

### 2. Testable forwarding helper

Extracted the three `if X != "" { ... }` assignments into
`forwardBuildMetadata(v, b, t)`. Three new test cases in `main_test.go`:

- **all values forwarded** — confirms the happy path mutates
internalversion.*
- **empty inputs preserve existing values** — confirms local `go run` /
untagged `go build` leaves defaults intact
- **partial injection** — confirms only the non-empty inputs land (e.g.
buildTime empty on workflow_dispatch backfill still populates version +
build)

Tests use snapshot/restore via `t.Cleanup` so they don't leak state.

### 3. Sync release.yml to latest template

Picks up the `-X main.buildTime=` ldflag from #74.

## Test plan

- [x] `go test -run TestForwardBuildMetadata -v ./cmd/ldap-manager/`
passes (3 subtests).
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the go-app release workflow template to inject an additional Go -ldflags -X value (main.buildTime) so consumer apps can surface build timestamp metadata in release binaries.

Changes:

  • Switch ldflags to a multiline YAML scalar for readability.
  • Add -X main.buildTime=${{ github.event.head_commit.timestamp }} to the release build ldflags.
  • Add explanatory comments documenting the intended fleet convention around main.* build metadata variables.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread templates/go-app/.github/workflows/release.yml
CybotTM added a commit to netresearch/raybeam that referenced this pull request Apr 20, 2026
## Summary

Copilot-review follow-up on
[#222](#222): the `build`
shim var in main.go was reserved for the `-X main.build` ldflag target
but never actually surfaced. Also makes `internal/build` expose
CommitHash symmetrically to Version.

### Changes

- **internal/build**: add `CommitHash` var, factor the ReadBuildInfo
lookup into `vcsRevisionOr()` for reuse. Both Version and CommitHash
self-describe from VCS when ldflags aren't applied.
- **main.go**: extract `forwardBuildMetadata(v, b)` from init() so
forwarding is unit-testable; forward the previously-dead `build` var
into `buildpkg.CommitHash`.
- **cmd/root.go**: assemble `rootCmd.Version` via `formatVersion()`,
which renders `<tag> (<short-commit>)` and dedupes when Version equals
CommitHash (the local VCS-fallback case).
- **cmd/root_test.go**: 5 test cases covering tagged / untagged /
short-commit / dedupe / unknown paths.
- **release.yml** sync to template revision
[netresearch/.github#74](netresearch/.github#74)
(adds `main.buildTime` ldflag — silent no-op here since raybeam has no
BuildTimestamp consumer, but keeps the file byte-identical with the
fleet).

## Test plan

- [x] `go test -v ./cmd` — 5 subtests pass.
- [x] `go build ./...` — OK.
CybotTM added a commit to netresearch/ldap-selfservice-password-changer that referenced this pull request Apr 21, 2026
…ion (#568)

## Summary

ldap-ssp's `release.yml` was left behind on the old 2-flag ldflags
because the whole file is on the intentional-drift list (Fiber v3 32-bit
matrix narrow). That drift entry covers the matrix narrowing only;
staying on the old ldflags was collateral, not intentional.

- **`release.yml`**: bring ldflags inline with the
[netresearch/.github#74](netresearch/.github#74)
template — three `-X` targets (`main.version`, `main.build`,
`main.buildTime`). Narrowed matrix + intentional-drift comment stay.
Only real difference from template now: `linux/amd64,linux/arm64`
platforms.
- **`main.go`**: declare `var buildTime string` as a receiver, include
in the structured startup log next to `version` + `build`. Empty here
(no BuildTimestamp consumer yet) — cheap insurance vs. any future linker
strictness.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants