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

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

Open
mvdan opened this issue Mar 12, 2021 · 6 comments
Open

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

mvdan opened this issue Mar 12, 2021 · 6 comments
Milestone

Comments

@mvdan
Copy link
Member

@mvdan 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 this to the Go1.17 milestone Mar 12, 2021
@zephyrtronium
Copy link
Contributor

@zephyrtronium zephyrtronium commented Mar 12, 2021

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 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 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 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 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
Member

@dmitshur dmitshur commented Mar 24, 2021

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 and removed NeedsDecision labels Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants