Skip to content
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

BuildDate is always the current time #366

Closed
NickLarsenNZ opened this issue Jan 23, 2022 · 6 comments · Fixed by #367
Closed

BuildDate is always the current time #366

NickLarsenNZ opened this issue Jan 23, 2022 · 6 comments · Fixed by #367
Labels
bug Something isn't working good first issue Good for newcomers
Milestone

Comments

@NickLarsenNZ
Copy link
Contributor

Describe the bug

The build date is always the current time.

To Reproduce

Run argocd-image-updater version twice in a row (at least a second apart).

Expected behavior

The actual binary build date time.

Additional context

This line gets the runtime datetime:

buildDate = time.Now().UTC().Format(time.RFC3339)

Version

/ $ argocd-image-updater version
argocd-image-updater: v0.11.3+f62b7d7
  BuildDate: 2022-01-23T10:22:17Z
  GitCommit: f62b7d7841a42030fe4c75de1951c46b0e7fca2e
  GoVersion: go1.16.13
  GoCompiler: gc
  Platform: linux/amd64
/ $ argocd-image-updater version
argocd-image-updater: v0.11.3+f62b7d7
  BuildDate: 2022-01-23T10:27:13Z
  GitCommit: f62b7d7841a42030fe4c75de1951c46b0e7fca2e
  GoVersion: go1.16.13
  GoCompiler: gc
  Platform: linux/amd64

Logs
Please paste any relevant logs here

@NickLarsenNZ NickLarsenNZ added the bug Something isn't working label Jan 23, 2022
@NickLarsenNZ NickLarsenNZ changed the title Build Date is always the current time BuildDate is always the current time Jan 23, 2022
@NickLarsenNZ
Copy link
Contributor Author

Here's a possible way of injecting build data: https://blog.alexellis.io/inject-build-time-vars-golang/#overridegobuild

version.go

package main

import (
   // ...
)

var BuildDate string

func newVersionCommand() *cobra.Command {
   // ...
}
go build -ldflags "-X main.BuildDate=$(datetime command here)"

@NickLarsenNZ
Copy link
Contributor Author

Ah ok, so argocd-image-updater does the same:

https://github.com/argoproj-labs/argocd-image-updater/blob/master/Makefile#L32-L35

It's just being overwritten by time.Now().UTC().Format(time.RFC3339). Is that because it is no longer static, so the linker is not updating it (or it is, and the runtime is still updating it)?

@jannfis
Copy link
Contributor

jannfis commented Jan 23, 2022

Good catch. It should have been overriden at build-time.

-X ${VERSION_PACKAGE}.buildDate=${BUILD_DATE}

and later

CGO_ENABLED=0 GOOS=${OS} GOARCH=${ARCH} go build -ldflags '${LDFLAGS}' -o ${OUTDIR}/${BINNAME} cmd/*.go

But for some reason, it doesn't 🤔

@jannfis
Copy link
Contributor

jannfis commented Jan 23, 2022

It's just being overwritten by time.Now().UTC().Format(time.RFC3339). Is that because it is no longer static, so the linker is not updating it (or it is, and the runtime is still updating it)?

If buildDate in the version package is set to a static string (e.g. "unknown"), the override works as expected. Probably we should set it to an empty string, and the return the value of time.Now().UTC().Format(time.RFC3339) in BuildDate() when buildDate is the empty string (so that it will print a date when run by go run ...).

@jannfis jannfis added this to the v0.12.0 milestone Jan 23, 2022
@jannfis jannfis added the good first issue Good for newcomers label Jan 23, 2022
@NickLarsenNZ
Copy link
Contributor Author

Sounds good @jannfis. Just wondering, should we follow how argocd does theirs, with a static string of unix epoch?

https://github.com/argoproj/argo-cd/blob/b37eee1054e42c873699460dd5e2447c2f9fe5a6/common/version.go#L12

Or is it quite useful to have the current time when using go run?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants