-
Notifications
You must be signed in to change notification settings - Fork 245
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
Use builtin go buildinfo for git version information #2186
Use builtin go buildinfo for git version information #2186
Conversation
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.
The linter spotted some issues worth checking
f8ac8e5
to
dcc91a2
Compare
yeah, clued me in on the tests for this. Unfortunately I can't think of an easy way to mock |
This makes things more workable with offbeat build procedures eg package managers that might not fetch the repo from git, golang autoembeds this info when available & fails gracefully when not, or when manually requested with `-buildinfo=false` or `-buildvcs=false` (this is what gentoo does)
I'm fine not testing that Commit part. We could have a function that takes Settings, but we really don't have much in that logic. |
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.
Can we bring back the branch name for dev builds? It's helpful to see which branch something was built on when doing a bunch of local dev.
For example, this is what I have right now after running make
on a branch:
% fly version
flyctl v0.0.0-1682385080+dev darwin/arm64 Commit: c62ae16a0a9fa41d3aedc56627632b7872bc4c95 (client-init-for-libs) BuildDate: 2023-04-25T01:11:20Z
we don't need (or probably want) the branch names in the builds that get released.
Hey tvdfly, Thanks for the request!, if this is a hard requirement on the version info, this PR is pretty much a non-starter. The git command used to embed this data is revision (and timestamp) only: The go team does seem amenable to adding more detailed data into Related issues:
I might end up filing a proposal to add branch name (or just custom information?) Will let y'all close out this PR if you decide to, but thanks for considering it! |
What if we make our own branch var in the place we used to set the commit sha, and then set it in the makefile? If the var is set we can show it and if not we ignore it. |
Part of the intent of the PR was moving this tagged-info logic outside of the makefile. Doing this works but brings us back to the same initial place where building without being inside of a git repo isn't supported nicely. Could also sunset this PR and just make something to better handle git-less builds |
@mjsir911 I made a branch on top of this one with what I'm thinking: #2284 (this commit in particular: e2a6baf6 ) The branch name just doesn't show up if git isn't installed or doesn't work or whatever. Is that still going to be a problem in the cases you outline? It doesn't seem like it to me, and I'm not very familiar with the issues that motivated the original change. Appreciate your help and this contribution! |
Sorry about the slow response, thanks for the merge! The change you linked makes everything work smoothly, yes. I appreciate you working with me through this one. |
This makes things more workable with offbeat build procedures eg package managers that might not fetch the repo from git, golang autoembeds this info when available & fails gracefully when not, or when manually requested with
-buildinfo=false
or-buildvcs=false
(this is what gentoo does)