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.

Sign up for GitHub

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

Jump to bottom

cmd/go: include Go's base version in its devel version strings #44960

Closed
mvdan opened this issue Mar 12, 2021 · 8 comments
Closed

cmd/go: include Go's base version in its devel version strings #44960

mvdan opened this issue Mar 12, 2021 · 8 comments
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Go1.17

Comments

@mvdan
Copy link
Member

mvdan commented Mar 12, 2021

This is a continuation of #41116 (comment), since the original issue got closed by 1.16 having go env GOVERSION.

@rsc said:

There is a separate issue, which is that you can't tell from go version what the base version of Go is. Changing the output to do that seems fine to me. I think I would suggest changing the git hash to "go1.Version+Hash", as in:

go version devel go1.16+a8a337f1f4 Tue Oct 20 16:43:37 2020 -0400 darwin/amd64

There's some more discussion in the comments afterward, such as using - instead of +, to be semantically a bit closer to semver.

https://go-review.googlesource.com/c/go/+/264938 is ready, for the most part. It only got a -2 since it was too late for 1.16.

I'm marking as NeedsDecision for 1.17, since I plan to work on it and there should be plenty of time left before the freeze.

cc @bcmills @jayconrod @heschik @dmitshur
@mvdan mvdan added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. GoCommand cmd/go Tools This label describes issues relating to any tools in the x/tools repository. labels Mar 12, 2021
@mvdan mvdan added this to the Go1.17 milestone Mar 12, 2021
@zephyrtronium
Copy link
Contributor

What string would be used for e.g. dev.go2go or dev.typeparams (prior to the merge), which don't appear to reflect any particular Go version?

@mvdan
Copy link
Member Author

mvdan commented Mar 12, 2021

I don't think they'd behave in any special way. Since master is regularly merged into those, they'd report a very similar version to master; but instead of go1.17+master_hash, it would be like go1.17+dev_foo_hash.

@knweiss
Copy link

knweiss commented Mar 13, 2021

What about using the existing git describe version format which also shows the number of commits since the base version?

From the man page:

With something like git.git current tree, I get:

       [torvalds@g5 git]$ git describe parent
       v1.0.4-14-g2414721

i.e. the current head of my "parent" branch is based on v1.0.4, but since it has a few commits
on top of that, describe has added the number of additional commits ("14") and an abbreviated
object name for the commit itself ("2414721") at the end.

@mvdan
Copy link
Member Author

mvdan commented Mar 21, 2021

@knweiss this issue is about including Go's base version, which is not related to git or any of its tags.

@mvdan
Copy link
Member Author

mvdan commented Mar 24, 2021

Friendly bump, as we only have five weeks left before the freeze, and I think the sooner we get this in the better :) It could have unexpected effects on the builders or other places that do custom builds of Go.

@dmitshur
Copy link
Contributor

I'll copy what I wrote on the CL, just s/Go 1.16/Go 1.17/:

I'm okay with this for Go 1.17, with the understanding that we reserve the right to potentially improve the information exposed in the devel version string further in future Go versions.

I expect this change to be safe (and its effect is limited to Go developers only, users of released versions will never see this), but there is always a chance merging this will cause us to learn of some unexpected disruption somewhere. If so, we should be able to roll this back easily, and become better informed for the next try.

It seems people are in favor of this direction and I'm not seeing problems raised, so moving the issue to NeedsFix. You'll need to ping Russ to update the -2 on the existing CL.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Mar 24, 2021
@iwdgo
Copy link
Contributor

iwdgo commented Apr 12, 2021

CL 264938 in reference was merged but, apparently, Go Bot missed it. On tip:

>gotip version
go version devel go1.17-776d8d387c Sun Apr 4 00:01:03 2021 +0000 windows/amd64

It seems that issue can be closed.

@mvdan
Copy link
Member Author

mvdan commented Apr 12, 2021

Thanks, yes. This issue was forked from the original, and I forgot to update the commit message to link here.

@mvdan mvdan closed this as completed Apr 12, 2021
@hyangah hyangah mentioned this issue Apr 30, 2021
Closed
gopherbot pushed a commit to golang/vscode-go that referenced this issue May 3, 2021
@hyangah
src/util: adjust go dev version regex
b1cb6c5 
During the go1.17 dev cycle, the dev version string format was
changed to include the go base version.
See golang/go#44960.
Recognize the new format.

We can improve the version comparison logic utilizing this further
but this does not utilize it yet.

Fixes #1464

Change-Id: If2084ad9d86cb8b4c0f59c36b1347c0188452189
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/315853
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
lggomez pushed a commit to lggomez/vscode-go-1 that referenced this issue May 6, 2021
@hyangah @lggomez
src/util: adjust go dev version regex
c3ab262 
During the go1.17 dev cycle, the dev version string format was
changed to include the go base version.
See golang/go#44960.
Recognize the new format.

We can improve the version comparison logic utilizing this further
but this does not utilize it yet.

Fixes golang#1464

Change-Id: If2084ad9d86cb8b4c0f59c36b1347c0188452189
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/315853
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
@golang golang locked and limited conversation to collaborators Apr 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Go1.17
Development

No branches or pull requests
6 participants
@knweiss @zephyrtronium @dmitshur @mvdan @gopherbot @iwdgo