Skip to content

release: fix version ldflags package path#33

Merged
taoky merged 1 commit into
ustclug:masterfrom
yaoge123:fix-version-ldflags
May 28, 2026
Merged

release: fix version ldflags package path#33
taoky merged 1 commit into
ustclug:masterfrom
yaoge123:fix-version-ldflags

Conversation

@yaoge123
Copy link
Copy Markdown
Contributor

Summary

The .goreleaser.yml ldflags line targets github.com/ustclug/rsync-proxy/pkg/info.{Version,BuildDate,GitCommit}, but no such package exists. The actual variables live in the cmd package (cmd/cmd.go):

var (
    Version   = "0.0.0"
    GitCommit = "$Format:%H$"
    BuildDate = "1970-01-01T00:00:00Z"
)

Go's linker silently ignores -X for unresolved symbols, so the released binary keeps the source defaults.

Reproduction

The v0.1.4 release binary:

$ rsync-proxy_linux_amd64 version
{"GitCommit":"$Format:%H$","BuildDate":"1970-01-01T00:00:00Z","Version":"0.0.0",...}

Fix

Update the package path in .goreleaser.yml to match the variable locations and the existing Makefile (which uses github.com/ustclug/rsync-proxy/cmd.Version).

Verification

Local build with the corrected ldflags reports the injected values correctly.

The ldflags reference github.com/ustclug/rsync-proxy/pkg/info.Version, but the version variables actually live in the cmd package (github.com/ustclug/rsync-proxy/cmd.Version). The Go linker silently ignores -X flags for unresolved symbols, so v0.1.4 binaries report Version=0.0.0, GitCommit=$Format:%H$, BuildDate=1970-01-01T00:00:00Z.

Update the package path to match Makefile and the actual variable definitions in cmd/cmd.go.
Copilot AI review requested due to automatic review settings May 26, 2026 21:37
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

Note

Copilot was unable to run its full agentic suite in this review.

Updates the ldflags path in the GoReleaser config to point to the new location of the version variables (cmd package instead of pkg/info).

Changes:

  • Updates -X ldflag paths from pkg/info to cmd package for Version, BuildDate, and GitCommit variables.

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

@yaoge123
Copy link
Copy Markdown
Contributor Author

Added regression tests in 49139e5:

  • cmd/cmd_test.go: drives printVersion directly with overridden package vars to confirm Version/GitCommit/BuildDate flow through to the JSON output, and that --pretty indents.
  • test/release/version_test.go: builds the root package with the same -X package path declared in .goreleaser.yml and execs rsync-proxy version from the resulting binary. The path lives in a single constant so the test and the goreleaser config cannot silently drift.

Verified by temporarily flipping the constant to the old pkg/info path: the release test fails with messages explicitly pointing at .goreleaser.yml. Restoring the correct cmd path makes both packages green.

@taoky taoky requested a review from iBug May 27, 2026 04:30
@taoky
Copy link
Copy Markdown
Member

taoky commented May 27, 2026

这里的测试我感觉是没有必要的?

@yaoge123
Copy link
Copy Markdown
Contributor Author

这里的测试我感觉是没有必要的?

那就简单点只改 .goreleaser.yml

@taoky
Copy link
Copy Markdown
Member

taoky commented May 27, 2026

那就简单点只改 .goreleaser.yml

@yaoge123 yaoge123 force-pushed the fix-version-ldflags branch from 49139e5 to d4ccfc9 Compare May 28, 2026 06:52
@taoky taoky merged commit 234b52b into ustclug:master May 28, 2026
2 checks passed
@yaoge123 yaoge123 deleted the fix-version-ldflags branch May 29, 2026 06:21
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.

3 participants