-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[bug] Fix the version module path in ldflags #6990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Batuhan Apaydin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Is there a way to have this code fail on the wrong paths?
I'm not sure how we could test this sorry :( |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6990 +/- ##
=======================================
Coverage 95.99% 95.99%
=======================================
Files 346 346
Lines 20429 20429
=======================================
Hits 19611 19611
Misses 616 616
Partials 202 202
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
we could write another script where we could test the build time injection like the following:
then we could run it as another step in the release workflows. |
would you like to add this in another PR? It doesn't need to be a separate script, just an extra check in the build process, perhaps in |
## Which problem is this PR solving? - Resolves #7090 ## Description of the changes - Add validation to the build step that the version number is correctly embedded in the binary ## How was this change tested? - When reverting the fix done in #6990 the build fails: ``` $ make build-jaeger echo "building binary for $(go env GOOS)-$(go env GOARCH)"; CGO_ENABLED=0 installsuffix=cgo go build -trimpath -o ./cmd/jaeger/jaeger-darwin-arm64 -ldflags "-X github.com/jaegertracing/jaeger/pkg/version.commitSHA=687207be86edb688436f8ef6c4383968a0ec1855 -X github.com/jaegertracing/jaeger/pkg/version.latestVersion=v2.5.0 -X github.com/jaegertracing/jaeger/pkg/version.date=2025-05-02T18:47:48Z" ./cmd/jaeger building binary for darwin-arm64 ❌ ERROR: version mismatch: want=v2.5.0, have= make: *** [build-jaeger] Error 1 ``` --------- Signed-off-by: Yuri Shkuro <[email protected]>
I noticed that you have changed the version module path so we have to update the path in the ldflags too: https://github.com/jaegertracing/jaeger/blob/06840acda4e6ea5cec5afaa19356125595904231/CHANGELOG.md?plain=1#L86 Signed-off-by: Batuhan Apaydin <[email protected]>
…racing#7092) ## Which problem is this PR solving? - Resolves jaegertracing#7090 ## Description of the changes - Add validation to the build step that the version number is correctly embedded in the binary ## How was this change tested? - When reverting the fix done in jaegertracing#6990 the build fails: ``` $ make build-jaeger echo "building binary for $(go env GOOS)-$(go env GOARCH)"; CGO_ENABLED=0 installsuffix=cgo go build -trimpath -o ./cmd/jaeger/jaeger-darwin-arm64 -ldflags "-X github.com/jaegertracing/jaeger/pkg/version.commitSHA=687207be86edb688436f8ef6c4383968a0ec1855 -X github.com/jaegertracing/jaeger/pkg/version.latestVersion=v2.5.0 -X github.com/jaegertracing/jaeger/pkg/version.date=2025-05-02T18:47:48Z" ./cmd/jaeger building binary for darwin-arm64 ❌ ERROR: version mismatch: want=v2.5.0, have= make: *** [build-jaeger] Error 1 ``` --------- Signed-off-by: Yuri Shkuro <[email protected]>
I noticed that you have changed the version module path so we have to update the path in the ldflags too: https://github.com/jaegertracing/jaeger/blob/06840acda4e6ea5cec5afaa19356125595904231/CHANGELOG.md?plain=1#L86 Signed-off-by: Batuhan Apaydin <[email protected]> Signed-off-by: amol-verma-allen <[email protected]>
…racing#7092) ## Which problem is this PR solving? - Resolves jaegertracing#7090 ## Description of the changes - Add validation to the build step that the version number is correctly embedded in the binary ## How was this change tested? - When reverting the fix done in jaegertracing#6990 the build fails: ``` $ make build-jaeger echo "building binary for $(go env GOOS)-$(go env GOARCH)"; CGO_ENABLED=0 installsuffix=cgo go build -trimpath -o ./cmd/jaeger/jaeger-darwin-arm64 -ldflags "-X github.com/jaegertracing/jaeger/pkg/version.commitSHA=687207be86edb688436f8ef6c4383968a0ec1855 -X github.com/jaegertracing/jaeger/pkg/version.latestVersion=v2.5.0 -X github.com/jaegertracing/jaeger/pkg/version.date=2025-05-02T18:47:48Z" ./cmd/jaeger building binary for darwin-arm64 ❌ ERROR: version mismatch: want=v2.5.0, have= make: *** [build-jaeger] Error 1 ``` --------- Signed-off-by: Yuri Shkuro <[email protected]> Signed-off-by: amol-verma-allen <[email protected]>
I noticed that you have changed the version module path so we have to update the path in the ldflags too:
jaeger/CHANGELOG.md
Line 86 in 06840ac