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: go test -buildvcs=true does not include vcs information #52648

Open
carlmjohnson opened this issue May 2, 2022 · 5 comments
Open

cmd/go: go test -buildvcs=true does not include vcs information #52648

carlmjohnson opened this issue May 2, 2022 · 5 comments
Labels
GoCommand NeedsDecision Thinking
Milestone

Comments

@carlmjohnson
Copy link
Contributor

@carlmjohnson carlmjohnson commented May 2, 2022

What version of Go are you using (go version)?

go version go1.18.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/Users/adhoc/bin"
GOCACHE="/Users/adhoc/Library/Caches/go-build"
GOENV="/Users/adhoc/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/adhoc/src/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/adhoc/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.18.1/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.18.1/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.18.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/p7/jc4qc9n94r3f6ylg0ssh1rq00000gs/T/go-build2620312267=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Run go test -buildvcs=true .

What did you expect to see?

Have debug.ReadBuildInfo() include vcs information.

What did you see instead?

Information was not included.

Compare #52338, cc @bcmills

carlmjohnson added a commit to carlmjohnson/versioninfo that referenced this issue May 2, 2022
carlmjohnson added a commit to carlmjohnson/versioninfo that referenced this issue May 2, 2022
carlmjohnson added a commit to carlmjohnson/versioninfo that referenced this issue May 2, 2022
@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented May 2, 2022

@carlmjohnson
Copy link
Contributor Author

@carlmjohnson carlmjohnson commented May 2, 2022

I agree that the VCS information should not be included by default. But if you specifically add -buildvcs=true, it should either include the information or error out. In my case, I'm testing a package that parses debug.ReadBuildInfo, which is kind of a weird thing to do, but I don't think it's so weird that the VCS information should never be includable.

@bcmills
Copy link
Member

@bcmills bcmills commented May 2, 2022

I suppose that now that we have -buildvcs=auto, we could theoretically make -buildvcs=true add stamping for tests. 🤔

@bcmills bcmills added Thinking NeedsDecision GoCommand labels May 2, 2022
@bcmills bcmills added this to the Go1.19 milestone May 2, 2022
@seankhliao seankhliao changed the title test: go test -buildvcs=true does not include vcs information cmd/go: go test -buildvcs=true does not include vcs information May 2, 2022
carlmjohnson added a commit to carlmjohnson/versioninfo that referenced this issue May 2, 2022
@alexedwards
Copy link

@alexedwards alexedwards commented May 8, 2022

I'd like to see consistency in behavior across the different go tools. If go test -buildvcs=true adds stamping for tests, then go run -buildvcs=true should add stamping too.

I wouldn't want to see us end up in a scenario like this:

  • go build adds stamping by default (if a VCS repo is present)
  • go test only adds stamping if -buildvcs=true
  • go run never adds stamping, even if -buildvcs=true (which is where things were left on #52338)

@carlmjohnson
Copy link
Contributor Author

@carlmjohnson carlmjohnson commented May 8, 2022

I think it’s fine for go test to default to omitting and go run to disallow it, but it should error out if you request an impossible option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand NeedsDecision Thinking
Projects
None yet
Development

No branches or pull requests

4 participants