-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/go: consider including Go's own version in 'go env' #41116
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
Comments
I'm very in favour of this. When I'm
debugging builds on a remote machine or helping
a colleague, I call |
That's a good point; just having to call |
On our last golang-tools call, @bcmills, @jayconrod, and @heschik raised a good point. The version reported by At the time of writing, if one runs Go 1.15.2, one could imagine:
That seems simple enough. In the case of a stable release, they'll always share the first two semver components. However, it gets more interesting for devel builds. For example, with my current master build, as per
We could bikeshed about the names, but it seems clear to me that both versions are useful to have, and they're clearly different. |
It seems like everyone agrees this would be a good change, so I'll do a CL this week in the hopes to get it into 1.16. We can bikeshed about the var names on Gerrit. |
We shouldn't have two different variables. One should be enough, it should be GOVERSION, which should record the output of There is a separate issue, which is that you can't tell from
|
We could close the not-quite-semver gap a little bit by using |
I'm OK with '-' because the string means "before Go 1.16" not "after Go 1.16". |
Just for the record, I don't think it reliably means either before or after. We do bump the version about the same time the tree reopens these days, but if you're building 1.16.1 before it's been tagged, it's definitely not before 1.16, right? Adding the "base version" to the version string should work for gopls as long as it's enforced by |
Do we agree to do both changes at once for 1.16? I feel like if we change the format of |
Change https://golang.org/cl/264938 mentions this issue: |
Making @mvdan I wanted to ask if it's in scope of this issue to make it work for minor versions on release branches too? For example, if Go is built at commit
Making that work may require doing something different, because unlike the main branch, release branches have a |
I don't think so - it would report
Thanks, I didn't know that (just like I didn't know that builders similarly had a hard-coded version in them). The question then is - should such a build result in If the worry is that some people might build with a source tarball without the git directory present, then perhaps the |
I see, so it's in scope of this issue to expose the major Go version but not minor. I think that's fine as long as it leaves the door open to be able to also expose the minor version in the future. Those who are interested in just the Go language version can ignore the minor if it is added in the future (although code that just takes the whole string until the first I can't remember what use case I had for exposing the minor version. I think it was about being exposing precise version information for informational purposes. But let me see if there are other good reasons for it. @mvdan Are you okay with leaving the door open for exposing the minor (i.e., Go x.y.Z) version too in the future? |
As far as I can tell, the minor version (or bugfix in semver) is only useful for the end user when they run a tagged release. If they're running a development source build, I don't think we should expose any minor version at all. I'm not sure what you mean by leaving the door open, but I prefer not to expand the scope of the issue since there isn't a clear use case and it has no precedent that I can see. The precedents for exposing the major version are the build tag version and the language version, both of which alter how tools behave, but the minor version does not. |
I'm okay with not expanding scope for this issue. I think it will help to clarify what is in scope. In #41116 (comment), Russ said:
That suggests a hypothetical future Go 1.19.3 version would print:
And a hypothetical future non-released version would print:
And if the |
Yes, @dmitshur. |
That seems reasonable to me. I think any development version string changes are better to leave to another cycle, since there's more discussion to be had. If there are concerns that it might become harder to change its format in the future, then for Go 1.16 you can consider making |
@mvdan I think at this point you should probably just put the version in |
I realise that the freeze is just one week away, but I admit I don't agree that there's more discussion to be had. This issue is two months old, has involved all major stakeholders including cmd/go owners and Russ, and we've talked about this in detail in the two last golang-tools calls (you can see the notes and recordings at https://github.com/golang/go/wiki/golang-tools, if you're curious). I guess more discussion can't hurt, but it seems like we already had consensus for a solution in #41116 (comment) and the earlier comments.
Indeed, this is my plan B, and part of why I wanted to do this issue in two CLs - so that at least |
To clarify, my comment was trying to answer your question of "Do we agree to do both changes at once for 1.16?" from 4 days ago, since I saw this issue was still in NeedsDecision state and I didn't see an answer to that here. I hadn't been aware of discussion outside of this issue, thanks for letting me know. It seems this issue should be moved to NeedsFix. |
Indeed, I was trying to get some extra confirmation before the merges happened. I think we can safely move the issue to NeedsFix at least. |
Change https://golang.org/cl/265637 mentions this issue: |
This way, "go version" will report the "base version" or major version that the tool corresponds to. This is the same version number that is matched against build tags such as "go1.14" or "!go1.16". Obtaining this version being built is non-trivial, since we can't just import a package or query git. The added comments document the chosen mechanism, based on a regular expression. It was chosen over AST parsing as it would add a significant amount of code without much gain, given how simple the goversion.go file is. For #41116. Change-Id: I653ae935e27c13267f23898f89c84020dcd6e194 Reviewed-on: https://go-review.googlesource.com/c/go/+/264938 Run-TryBot: Daniel Martí <[email protected]> Trust: Daniel Martí <[email protected]> Trust: Jay Conrod <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Russ Cox <[email protected]>
go env -json
tells me a lot about the user's Go setup and environment, and in a format that's easy to parse. Unfortunately,go version
is missing there, so if I want to fetch that I need a separate exec call.I wonder if we could add it to go env, similar to other "not really an env var" lines like
GOMOD
,GOEXE
, orGOHOSTARCH
. For example:It would not include the string prefix
go version
, since it's redundant, nor thelinux/amd64
pair, since that's already asGOHOSTOS/GOHOSTARCH
ingo env
.I'm not making this a proposal for now, since the idea seems pretty simple.
/cc @bcmills @jayconrod @matloob for cmd/go
The text was updated successfully, but these errors were encountered: