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: revise build stamp keys for Go 1.18 #49168

Closed
bcmills opened this issue Oct 26, 2021 · 8 comments
Closed

cmd/go: revise build stamp keys for Go 1.18 #49168

bcmills opened this issue Oct 26, 2021 · 8 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@bcmills
Copy link
Member

bcmills commented Oct 26, 2021

In #37475 (comment), @mpx notes that the keys used to record build stamp information are a bit inconsistent: we currently use gitrevision, hgrevision, etc. for each vcs, and uncommitted for all VCSs.

I discussed with @rsc and @matloob, and we are planning to revise these setting-names both to make the VCS settings a bit more uniform and to more clearly distinguish between flags and other metadata. (@rsc may do this as a further patch-set on CL 358539.)

@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Oct 26, 2021
@bcmills bcmills added this to the Go1.18 milestone Oct 26, 2021
@mpx
Copy link
Contributor

mpx commented Oct 27, 2021

To clarify: We always prepend the VCS name before revision, uncommitted, and committime. I think it would be better to set a vcs tag and stop prepending the name to other keys. committime and uncommitted (even revision) are basically the same across VCS, and using consistent names makes them much easier to consume. It's unlikely we ever want to support simultaneously tagging for multiple VCS.

@bcmills
Copy link
Member Author

bcmills commented Nov 12, 2021

This is mostly done in CL 358539, but it needs a test-fix before it can be merged.

@heschi
Copy link
Contributor

heschi commented Nov 24, 2021

The beta is getting close and this is currently marked as blocking the beta. Any news here?

@thepudds
Copy link
Contributor

thepudds commented Dec 1, 2021

Hi @heschi, FWIW, CL 358539 was merged earlier today.

@earthboundkid
Copy link
Contributor

I just ran this against gotip and didn't see the VCS stuff:

	info, ok := debug.ReadBuildInfo()
	if !ok {
		return
	}
	fmt.Println(info.Settings)
$ gotip version
go version devel go1.18-7ccbcc9056 Tue Nov 30 20:04:58 2021 +0000 darwin/amd64
$ gotip run test_run.go
[{-compiler gc} {CGO_ENABLED 1} {CGO_CFLAGS } {CGO_CPPFLAGS } {CGO_CXXFLAGS } {CGO_LDFLAGS } {GOARCH amd64} {GOOS darwin} {GOAMD64 v1}]

Is that still pending a CL?

@bcmills
Copy link
Member Author

bcmills commented Dec 1, 2021

This specific issue is fixed by CL 358539.

@carlmjohnson, if you are missing VCS stamps when you expect them, please open a new issue with details and/or steps to reproduce. (I suspect that what you're seeing may have something to do with module/repo boundaries..?)

@bcmills bcmills closed this as completed Dec 1, 2021
earthboundkid added a commit to earthboundkid/versioninfo that referenced this issue Dec 6, 2021
@gopherbot
Copy link

Change https://golang.org/cl/369743 mentions this issue: cmd/go: fix tests broken in CL 358539

gopherbot pushed a commit that referenced this issue Dec 7, 2021
CL 358539 revised the build-stamp format, and updated the git and hg
tests to match. However, the fossil and bzr tests were missed, and
were not caught on the builders due to the fact that none of the
builder images have the necessary VCS tools installed.

Updates #48802
Updates #49168

Change-Id: I6b9fd0e19b81cb539864c94ab0860f74e7be6748
Reviewed-on: https://go-review.googlesource.com/c/go/+/369743
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/378575 mentions this issue: cmd/go: fix TestScript/version_buildvcs_git_gpg

gopherbot pushed a commit that referenced this issue Jan 19, 2022
This test was missed in CL 358539, presumably because the 'longtest'
builders lack a 'gpg' executable.

Updates #49168
Fixes #50675

Change-Id: Ie3bfc761a5e4304531119625742f3def9df8af3f
Reviewed-on: https://go-review.googlesource.com/c/go/+/378575
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
jproberts pushed a commit to jproberts/go that referenced this issue Jun 21, 2022
This test was missed in CL 358539, presumably because the 'longtest'
builders lack a 'gpg' executable.

Updates golang#49168
Fixes golang#50675

Change-Id: Ie3bfc761a5e4304531119625742f3def9df8af3f
Reviewed-on: https://go-review.googlesource.com/c/go/+/378575
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Jan 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

6 participants