-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
cmd/go: stamp the pseudo-version in builds generated by go build
#50603
Comments
At least speaking personally, for cases like mvdan/sh#519, my intent is to show something like It's true that something like a proper module version might be more useful; a git commit hash doesn't give any hint as to how old a version is, whereas a semver version prefix or a timestamp can give a starting point. So, in principle, I agree with you: 1.18 is a big step forward, but it's still unfortunate that the main module version remains as However, in practice, I still agree with Jay's comment in #29228 (comment); we shouldn't make such a "locally inferred version" look like a normal version, because it's reasonably likely to be wrong or cause confusion with users.
Could you give some examples? I can only think of very unlikely scenarios, such as manually corrupting the module download cache after downloading some dependencies. That cache is read-only by default, and With the main module in a git checkout, I can think of multiple scenarios which seem more likely:
I think that, if we are to implement something like this, the versions must be somehow different from the canonical and unique versions that get computed from fully published commits and tags. This would make it very clear that the versions are inferred from local state, and not guaranteed to be correct. As a simplistic example, imagine that tagging |
To add a more concrete example: if we made the change proposed here, and locally inferred versions looked like fully published versions, I would have a harder time trusting the output of |
I was thinking of the case where since Go itself doesn't expose its own concept of a version to the program, the users themselves are forced to create their own concepts of a version, either through things like Let me rephrase my point. Go can't enforce any useful properties for the user's notion of a version because it doesn't know about it, and as such if we make The user does gain something in the latter case though. They don't have to create build wrappers.
I very much agree with this, with one caveat. If the locally checked-out version is identical to a published release, I would expect the version to match the release. If the locally checked-out version can not be guaranteed to match any release, then yes, it should be published with something like Unfortunately, I can't imagine how this would work without internet access, and quite often a prerequisite of automated systems running |
Hold on, another thought. If we always add the commit hash, and some other metadata to the main module version for local builds, essentially always making them a fully qualified Go pseudo-version, then they will always be different from the published version, so there's no potential for confusion there. Even better, in semver terms these builds will sort before the published version, which is probably what people want. For this, what I said earlier about
can no longer be true, but perhaps that is ok as long as we come up with a documented and stable convention that describes versioning for local builds (as opposed to just dumping a "devel" in the metadata field). |
The main caveat here, I think, is unpublished tags. If I create a local, unpublished tag for, say, That may or may not be a significant issue, though: if we always use a pseudo-version, we'll at least have the commit hash as a common point of reference even if the base versions differ. |
@4ad right, a local build can't always know what is or isn't published, as requiring a network roundtrip takes us back to square one. Your idea of trying to stick to semver, and always using some form of pseudo-version which includes a hash, sounds good. With one caveat, though: the commit hash isn't enough to make the version unambiguous, because I can have infinite kinds of uncommitted changes that do not change the HEAD commit hash. @bcmills good point about tags still messing with pseudo-versions, but at least if we always include a timestamp and some form of unique hash, then I think we're good. With the caveat above about uncommitted changes :) |
We do have another hash available to us, though, which changes whenever any input Go code changes: the build IDs used for the build cache. I seem to recall that one such ID is embedded into binaries, too. Not ideal, as such a hash also includes build parameters like GOOS or -tags, which don't normally affect versions. But at least it fixed the problem with uncommitted files in VCS. |
Yes, uncommitted changes should be explicit in the pseudo-version, but I think we can suffix |
the new buildinfo already records whether the workspace is clean with |
we could use one of So main will always have a version like
|
What is the main motivation of encoding the local version in pseudo-version style rather than keeping those extra info (timestamp?) as extra metadata fields - if it's not guaranteed that they are always available in the origin or proxies? It seems like the BTW, I feel like the main module's version isn't sufficient to describe a tool's behavior in certain cases - |
The main motivation is that
Emphasis mine. It's not rather than, It doesn't replace the existing metadata fields. If you want to read the metadata, you should read it from those fields instead of parsing the pseudo-version. However, that metadata is useful in disambiguating builds produced by
This sounds like an argument to always use the build ID as the version suffix instead of the VCS hash.
I hope so too, but again, I think that discussion is out of scope for this thread, which is more about bringing |
(devel)
in binaries, which is not very usefulgo build
This proposal has been added to the active column of the proposals project |
As a kind of experience report, I'm using custom build-scripts for years, solely to embed version information of the main module into the executable. When building, I attach the following information:
With this kind of information, I'm able to build version-strings however I like. Usually I try to match go's pseudo-version strings, but when I need to stay compatible with the version-scheme from other, non-go projects, I can do so as well.
The version is printed when the application is started with a I don't have anything against adding a build ID, but it wouldn't really solve my problem. My use cases are:
The previously raised issue that git-tags might only be local was never really an issue in my experience. Version-tags aren't made lightheartedly and are always immediately pushed to the server in the projects I work on. I really hope we can this kind of information into a go-executable one day, because I would finally be able to get rid of all my build- and tool-scripts. |
It sounds like @mvdan and @bcmills have some hesitation around the fact that these pseudo-versions would not correspond to any publicly available version, even though they look like those. That does seem like a reason not to do this. We now have Git VCS info separately in the builds (as of Go 1.18; try go version -m). Do we need to add a second way to record that information? |
go build
go build
We can make it unambiguously distinct, for example instead of
No, we certainly only need one way to encode VCS info. The suggestion to put VCS info in the metadata field of the pseudo-version was to match the |
With replace statements in go.mod and the new workspace mode in Go 1.18 it is possible to build Go programs that include local versions of modules besides the main module. For those modules the go tool also lists The new build vcs metadata only helps identify the main module. Adding vcs metadata for all local modules seems valuable and not currently supported. The format suggested above by @4ad would be more informative than |
This may be true, but the concern above seems to be adding vcs metadata that looks like a pseudo-version. It need not, and it probably should not. We can always add that separately; maybe that should be a different proposal. (I think this is the first comment to bring up VCS info for replaced modules that point to other local repos.) |
A thought: if we're only concerned about having a reliable way to always get some useful version for the main module, I think it could be an API of its own, like I personally will be implementing logic like that to replace Another option, if we want this to also work for library modules, would be If the above sounds interesting, I'll happily develop the idea further and create a new proposal. I realise it's not the same as this proposal, but I also think it could be a different solution to the same end-user problem :) |
@mvdan I would very much like to see something like that to replace the boiler place build lines that exist in code at work. |
We use the semantic version of the binaries in order to compute API compatibility between different binaries. I am afraid that if your Now, one might object to using the binary version in this way and perhaps recommend using a separately maintained API version instead that is separate from the binary version. I would tend to agree except this is outside my control. I do not have the operational liberty to change this. |
https://go.dev/cl/596035 is merged which adds version stamping for git only. This will come out in 1.24. I have not looked into the other VCS's hg(Mercurial), svn(Subversion), bzr(GNU Bazaar), or fossil and unsure whether they will be added by 1.24. I will leave this issue open for now. |
Thanks @samthanawalla, I just gave it a try and it seems to be working as advertised! I'm still slightly surprised at the appearance of I also notice that the version stamping does not happen with |
@mvdan While vcs.modified technically captures this information, the decision to surface it within the version is driven by making it immediately visible to those who primarily focus on the version. It's not necessarily motivated by trust but rather transparency and better 'book keeping'. It more directly conveys the relationship between tag information and how VCS changes can influence the build. Does +dirty create any problems for tooling? My understanding is that it can easily be chopped off for those who don't need it but is useful for those who may intentionally or unintentionally ignore vcs.modified and only look at the version. If there are specific use cases where +dirty causes issues for tooling then we can still change that as there is plenty of time till the 1.24 release. However we believe the enhanced visibility it provides into the build's status outweighs any potential drawbacks. Of course, we're open to hearing further feedback and specific examples :)
Yes correct. Sounds good I will be sure to document that aspect as well. |
Change https://go.dev/cl/605615 mentions this issue: |
I don't feel strongly about the presence of a However, I do find it an odd default because I often end up with Another similar scenario that I'm running into is building binaries for many platforms at once. I tend to build the binaries in the local directory, inside the module, to then upload them as release archives. And once again they are not gitignored - which is fine, as I delete them soon after, and they don't participate in the build. But once again,
Arguably this is all an issue affecting Even though this was already an issue for Speaking of... what if I had a new uncommitted Go file which did participate in the Go build, but was gitignored, so it wouldn't show up in |
@samthanawalla Thanks for this feature! This will help simplify the future release workflow of vscode go release greatly. One more tuning request: I noticed that some projects use the However, tools built this way can be very different from the tools built with For example,
This
Once the tool dependency proposal #48429 is implemented, some users may attempt to pick a specific version of govulncheck (or gopls) and such unexpected and untested versions of tools may appear more often. Currently golang.org/x/telemetry uses the main module's version string as the tool's version. The crash reports and counters from such untested versions are not distinguishable, and can add much noise. Is it possible to use a different version string ( |
@hyangah I had a similar (though slightly different concern) that the build with What do you think about adding a buildinfo field for the module path of the main module (or whether the build was done from a workspace, or from a pkg@version context) and telemetry could treat the binary different depending on which it is? |
This comment was marked as off-topic.
This comment was marked as off-topic.
@mvdan I see your point. Multiple sequential builds will create a +dirty tag because of the binary that gets created which does not seem ideal. I will see about making an exception for that. As far as the dirtiness of files which do not participate in the Go build affecting +dirty, we discussed that above: #50603 (comment) We think the most common case is a clean build from CI. Otherwise can clone & build from a clean repo. Yes an uncommitted file that is gitignored will produce a build without a +dirty tag. This is a drawback of the current implementation. However if this is an uncommon situation, then perhaps we may need to compromise somewhere. |
Change https://go.dev/cl/609155 mentions this issue: |
@samthanawalla @matloob I feel like whether the builds happen locally or in CI is a red herring. Whether I do the builds locally or in CI, placing the binaries inside the same cloned repository seems perfectly fine to me. They don't affect the build in any way, nor do they touch any committed files.
And as explained before, I think it's a mistake for I realise my arguments are against
That is, Go builds should only be marked as "dirty" if we can tell that they used a different set of source files compared to what is committed. That is honestly what I thought |
This subcommand cross-compiles the tagged version of vscgo for all targets we want to release prebuilt vscgo binaries for. It uses `go install github.com/golang/vscode-go/vscgo@<TAG_NAME>` to compile the binary, in order to embed useful build info (so, we can have meaningful telemetry). After go1.24 (golang/go#50603) we will be able to use `go build` instead. The output of this subcommand is "vscgo.zip" in the -out directory. The test involves running the `go install` and checking the compiled binaries included in the vscgo.zip file have expected GOOS/GOARCH. For hermetic unit testing, the test creates a file-based module proxy using the code copied from golang.org/x/telemetry/internal/proxy. For #3186 Change-Id: I45848fdb676d094c634786ac939481f1778b4995 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/595376 Reviewed-by: Hongxiang Jiang <[email protected]> kokoro-CI: kokoro <[email protected]> Reviewed-by: Robert Findley <[email protected]> Commit-Queue: Hyang-Ah Hana Kim <[email protected]>
@mvdan |
Change https://go.dev/cl/611916 mentions this issue: |
Since we added a local context to git lookups, we need to be more careful about fetching from remote. We should not fetch when we are stamping a binary because that could slow down builds. For #50603 Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-windows-amd64-longtest Change-Id: I81a719b7609e8d30b32ffb3c12a05074c5fd0c22 Reviewed-on: https://go-review.googlesource.com/c/go/+/611916 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
This CL adds a local only VCS lookup for Mercurial. It fixes a bug in pkg.go by passing in the repo directory to the LookupLocal function instead of the module directory. It could be the case that a binary is built in a subdirectory of the repo. For: #50603 Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-windows-amd64-longtest Change-Id: Ic36b5a361a8ba3b0ba1a6968cde5f5263c9c8dd0 Reviewed-on: https://go-review.googlesource.com/c/go/+/609155 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
This comment was marked as off-topic.
This comment was marked as off-topic.
Change https://go.dev/cl/627295 mentions this issue: |
This CL adds support for tagging binaries in a bzr vcs environment. For: #50603 Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-windows-amd64-longtest Change-Id: I81eb72d9e0e15dbec8778dd06613ca212820a726 Reviewed-on: https://go-review.googlesource.com/c/go/+/627295 Auto-Submit: Sam Thanawalla <[email protected]> Reviewed-by: Michael Matloob <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
Change https://go.dev/cl/630195 mentions this issue: |
This work related to the original issue is completed. Separate proposals will be needed for:
|
@samthanawalla This new capability seems noteworthy enough that it should be mentioned in Go 1.24 release notes, is that right? Reopening as a release blocker to track that. Thanks. |
NOTE: The accepted proposal is #50603 (comment).
cmd/go embeds dependency version information in binaries, which is very useful. From Go 1.18 onwards, cmd/go also embeds VCS information in binaries, which makes it even more useful than it was before.
As #37475 mentions, people place version information in binaries using
-ldflags='-X foo=bar'
, which requires an additional build wrapper. The new VCS stamping feature of cmd/go should alleviate the need for external wrapper, but I am afraid it comes short.The version information, in the sense of Go's pseudo version is not recorded for the main module when doing
go build
:The version is recorded as expected when doing
go install
:I am afraid this limitation of cmd/go will continue to force people to use external build wrappers that set
-ldflags
, which is rather unfortunate.I am not the first to want main module version information in binaries, this has been already asked for in various issues, for example in #29814, which was closed as a duplicate of #37475, but it really wasn't a duplicate, as #37475 is about VCS information, and #29814 is about semantic versioning. Other examples of people asking for this feature are mvdan/sh#519 and #29228 (comment) where various workarounds were proposed.
Speaking of workarounds, the only workaround that I know that currently works would be to create a local module proxy and pass
GOPROXY
togo install
, but that is an extremely high-overhead workaround, andgo install
is not a replacement forgo build
anyway, sincego install
comes with some rather severe limitations regarding how vendoring works and what you can put in go.mod, andgo install
doesn't support controllingGOBIN
when cross-compiling.I realize that Git tags are a local concept, and by doing the "wrong" git operations one could come up with a different pseudo-version for the same source code. I am afraid I don't have any solution or suggestion regarding this git misfeature, except to note that even in this case the hash information is recorded correctly, and in every case by the virtue of having access to the local source code the programmer can always do some local operation that has the potential to cause a version mislabeling. Git is just more prome to do this by accident, but the ability is there, always.
I don't have any stats to back this up, but from my experience most corporate source code is built by
go build
, notgo install
, and it would be great if somehow Go's notion of versioning would be stamped bygo build
.CC @bcmills @mvdan @rsc
The text was updated successfully, but these errors were encountered: