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" seems to now require git due to -buildvcs #51723

Closed
mvdan opened this issue Mar 16, 2022 · 12 comments
Closed

cmd/go: "go test" seems to now require git due to -buildvcs #51723

mvdan opened this issue Mar 16, 2022 · 12 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented Mar 16, 2022

go version go1.18 linux/amd64

I have a CI job that runs go test ./... in a git clone, but inside a Docker image where git isn't installed. This worked fine in Go 1.17.x, but breaks with 1.18 unless I add -buildvcs=false.

Reproducer:

#!/bin/bash

docker run -i golang:1.18-alpine <<-SCRIPT

	set -ex

	go version

	apk update
	apk add git

	git clone --depth=1 https://github.com/mvdan/sh
	cd sh

	apk del git

	go test ./...

SCRIPT

Output:

[...]
+ go test ./...
go: downloading github.com/google/renameio v1.0.1
go: downloading github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e
go: downloading golang.org/x/term v0.0.0-20210927222741-03fcf44c2211
go: downloading mvdan.cc/editorconfig v0.2.0
go: downloading github.com/rogpeppe/go-internal v1.8.1
go: downloading golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
go: downloading golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e
go: downloading github.com/creack/pty v1.1.17
go: downloading github.com/frankban/quicktest v1.14.0
go: downloading github.com/google/go-cmp v0.5.6
go: downloading gopkg.in/errgo.v2 v2.1.0
go: downloading github.com/kr/pretty v0.3.0
go: downloading github.com/kr/text v0.2.0
go: missing Git command. See https://golang.org/s/gogetcmd
error obtaining VCS status: exec: "git": executable file not found in $PATH
	Use -buildvcs=false to disable VCS stamping.
error obtaining VCS status: exec: "git": executable file not found in $PATH
	Use -buildvcs=false to disable VCS stamping.

I tried to reproduce this with a dummy module and no dependencies, even creating a dummy git repository, but all of that worked. So I imagine it has something to do with the dependencies. Note that the dependencies here are all downloaded from GOPROXY, so no VCS is needed.

cc @bcmills @matloob

@mvdan
Copy link
Member Author

mvdan commented Mar 16, 2022

Another theory: ./... matches some main packages, like ./cmd/shfmt. Perhaps they are being built and then buildvcs kicks in?

@rsc
Copy link
Contributor

rsc commented Mar 16, 2022

I suppose if git is not found we should proceed without the vcs info.
I think this should only trigger if we are in a tree with a .git directory, so one option would be to delete the .git directory.

Either way, we should set -buildvcs=false during builds in go test that will be thrown away, same as we set -ldflags=-w.

@heschi heschi added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 16, 2022
@heschi heschi added this to the Backlog milestone Mar 16, 2022
@ocket8888
Copy link

It's required in general by go build, it seems, whenever go detects that it's in a git repository. That's breaking my automation build systems that don't have git installed (because they didn't need to). I assume it'll probably break some others.

@mvdan
Copy link
Member Author

mvdan commented Mar 17, 2022

@ocket8888 This issue is about go test; I think you want #51748, which is about go build. They are different because VCS build stamping shouldn't be needed for running the tests.

Either way, we should set -buildvcs=false during builds in go test that will be thrown away, same as we set -ldflags=-w.

I'd agree with that. go test ./... shouldn't produce any output binaries for the user, so I can't see why -buildvcs=true should ever be part of the equation.

I've also narrowed down the cause to a package main. I imagine that go test builds all packages it needs, and for a main package, -buildvcs kicks in as per the default. So, presumably, swapping the default for go test to false should be enough.

$ cat repro.sh
#!/bin/bash

docker run -i golang:1.18-alpine <<-SCRIPT

	set -ex

	go version

	mkdir test
	cd test
	go mod init test
	mkdir .git

	cat >main.go <<-EOF
		package main

		func main() {}
	EOF

	go test

SCRIPT
$ bash repro.sh 
+ go version
go version go1.18 linux/amd64
+ mkdir test
+ cd test
+ go mod init test
go: creating new go.mod: module test
+ mkdir .git
+ cat
+ go test
go: missing Git command. See https://golang.org/s/gogetcmd
error obtaining VCS status: exec: "git": executable file not found in $PATH
	Use -buildvcs=false to disable VCS stamping.

tklauser added a commit to tklauser/numcpus that referenced this issue Mar 17, 2022
Explicitly disable buildvcs for build and test on FreeBSD. See
golang/go#51723 and
golang/go#51748.
ymkins pushed a commit to gwos/tcg that referenced this issue Mar 17, 2022
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 17, 2022
@bcmills bcmills self-assigned this Mar 17, 2022
@bcmills bcmills modified the milestones: Backlog, Go1.19 Mar 17, 2022
@bcmills
Copy link
Contributor

bcmills commented Mar 17, 2022

@gopherbot, please backport to Go 1.18. Invoking git for each go test of a package main is surprising and wastes users' time and battery power.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #51767 (for 1.18).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 17, 2022
tklauser added a commit to tklauser/numcpus that referenced this issue Mar 17, 2022
Explicitly disable buildvcs for build and test on FreeBSD. See
golang/go#51723 and
golang/go#51748.
Code-Hex added a commit to basemachina/bridge that referenced this issue Mar 18, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/393894 mentions this issue: cmd/go: avoid stamping VCS metadata in test binaries

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/393878 mentions this issue: [release-branch.go1.18] cmd/go: avoid stamping VCS metadata in test binaries

zrma added a commit to zrma/1d1go that referenced this issue Mar 19, 2022
…t requires git due to -buildvcs" backport

- golang/go#51767
- golang/go#51723

> @bcmills requested issue #51723 to be considered for backport to the next 1.18 minor release.
>
> @gopherbot, please backport to Go 1.18. Invoking git for each go test of a package main is surprising and wastes users' time and battery power.
zrma added a commit to zrma/1d1go that referenced this issue Mar 19, 2022
…ving "go test requires git due to -buildvcs" backport

- golang/go#51767
- golang/go#51723

> @bcmills requested issue #51723 to be considered for backport to the next 1.18 minor release.
>
> @gopherbot, please backport to Go 1.18. Invoking git for each go test of a package main is surprising and wastes users' time and battery power.
zrma added a commit to zrma/1d1go that referenced this issue Mar 19, 2022
… requires git due to -buildvcs" backport

- golang/go#51767
- golang/go#51723

> @bcmills requested issue #51723 to be considered for backport to the next 1.18 minor release.
>
> @gopherbot, please backport to Go 1.18. Invoking git for each go test of a package main is surprising and wastes users' time and battery power.
ymkins pushed a commit to gwos/tcg that referenced this issue Mar 23, 2022
dppattison pushed a commit to gwos/tcg that referenced this issue Mar 28, 2022
gopherbot pushed a commit that referenced this issue Apr 4, 2022
…inaries

Invoking a VCS tool requires that the VCS tool be installed, and also
adds latency to build commands. Unfortunately, we had been mistakenly
loading VCS metadata for tests of "main" packages.

Users almost never care about versioning for test binaries, because
'go test' runs the test in the source tree and test binaries are only
rarely used outside of 'go test'. So the user already knows exactly
which version the test is built against, because the source code is
right there — it's not worth the overhead to stamp.

Fixes #51767.
Updates #51723.

Change-Id: I96f191c5a765f5183e5e10b6dfb75a0381c99814
Reviewed-on: https://go-review.googlesource.com/c/go/+/393894
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Trust: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 67f6b8c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/393878
Reviewed-by: Cherry Mui <cherryyz@google.com>
@ruchi-dhore-tw
Copy link

ruchi-dhore-tw commented Apr 6, 2022

Is this issue still there?

I am on latest go version:
go version go1.18 darwin/amd64

Yet, I am still getting:

go: missing Git command. See https://golang.org/s/gogetcmd
error obtaining VCS status: exec: "git": executable file not found in $PATH
Use -buildvcs=false to disable VCS stamping.

I tried providing -buildvcs=false in following command, in that case as well I am getting above errors and below kafka errors:
go test ./... -race -buildvcs=false -covermode=atomic -coverprofile=coverage.out

Errors of kafka:

github.com/confluentinc/confluent-kafka-go/kafka
/usr/lib/gcc/x86_64-alpine-linux-musl/10.3.1/../../../../x86_64-alpine-linux-musl/bin/ld: /go/pkg/mod/github.com/confluentinc/confluent-kafka-go@v1.8.2/kafka/librdkafka_vendor/librdkafka_glibc_linux.a(rdkafka_txnmgr.o): in function `rd_kafka_txn_set_fatal_error':
much more such ones..

@DasSkelett
Copy link

Is this issue still there?

I am on latest go version: go version go1.18 darwin/amd64

Yes of course, Go 1.18.1 hasn't been released yet.

tried providing -buildvcs=false in following command, in that case as well I am getting above errors and below kafka errors:

That's something different, but it's missing important lines from the output to know what's wrong. You should open a new issue with the complete output.

@ruchi-dhore-tw
Copy link

@mvdan adding -buildvcs=false is not working for me [I am getting same errors the one you posted in thread], am i missing anything in the command? I am running this command inside a docker container.
go test -buildvcs=false ./... -race -covermode=atomic -coverprofile=coverage.out

@orsinium
Copy link

adding -buildvcs=false is not working for me

Are you on Go 1.18? Are you sure you've added it to the command that failed? If you have go build invoked, it needs the flag too. The following patch solved the issue for me: life4/gweb@2090dd2

zrma added a commit to zrma/1d1go that referenced this issue Apr 27, 2022
… requires git due to -buildvcs" backport

- golang/go#51767
- golang/go#51723

> @bcmills requested issue #51723 to be considered for backport to the next 1.18 minor release.
>
> @gopherbot, please backport to Go 1.18. Invoking git for each go test of a package main is surprising and wastes users' time and battery power.
@rsc rsc unassigned bcmills Jun 22, 2022
@golang golang locked and limited conversation to collaborators Jun 22, 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.
Projects
None yet
Development

No branches or pull requests

9 participants