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: binary does not contain VCS information when compiled in git worktree dir #58218

Open
rogpeppe opened this issue Feb 1, 2023 · 11 comments
Assignees
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@rogpeppe
Copy link
Contributor

rogpeppe commented Feb 1, 2023

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

$ go version
go version go1.20rc3 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GO111MODULE="auto"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/rogpeppe/.cache/go-build"
GOENV="/home/rogpeppe/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/rogpeppe/src/go/pkg/mod"
GONOPROXY="github.com/influxdata/idpe"
GONOSUMDB="github.com/influxdata/idpe"
GOOS="linux"
GOPATH="/home/rogpeppe/src/go"
GOPRIVATE="github.com/influxdata/idpe"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/rogpeppe/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/rogpeppe/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20rc3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/rogpeppe/src/cuelabs/cue2/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3009011258=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run testscript on the following file:

# set up git repository and build first binary
cd d1
exec git init
exec git add .
exec git commit -m 'initial commit'
exec go build -o $TMPDIR/prog1 .

# set up worktree and build second binary
exec git worktree add -b main1 ../d2 HEAD
cd ../d2
exec go build -o $TMPDIR/prog2 .

# ensure first binary has VCS info
exec go version -m $TMPDIR/prog1
stdout 'vcs.revision='

# ensure second binary has VCS info
exec go version -m $TMPDIR/prog2
stdout 'vcs.revision='

-- d1/go.mod --
module example

-- d1/main.go --
package main

func main() {}

What did you expect to see?

A successful test.

What did you see instead?

The binary built in a git worktree directory does not contain VCS information.
I suspect the logic doesn't understand that .git can be a file was well as a directory.

# set up git repository and build first binary (0.096s)
# set up worktree and build second binary (0.088s)
# ensure first binary has VCS info (0.002s)
# ensure second binary has VCS info (0.002s)
> exec go version -m $TMPDIR/prog2
[stdout]
$WORK/.tmp/prog2: go1.20rc3
	path	example
	mod	example	(devel)	
	build	-buildmode=exe
	build	-compiler=gc
	build	CGO_ENABLED=1
	build	CGO_CFLAGS=
	build	CGO_CPPFLAGS=
	build	CGO_CXXFLAGS=
	build	CGO_LDFLAGS=
	build	GOARCH=amd64
	build	GOOS=linux
	build	GOAMD64=v1
> stdout 'vcs.revision='
FAIL: /tmp/testscript498691614/x.txtar/script.txtar:19: no match for `vcs.revision=` found in stdout
error running /tmp/x.txtar in /tmp/testscript498691614/x.txtar
@mknyszek
Copy link
Contributor

mknyszek commented Feb 1, 2023

CC @bcmills @matloob

@mknyszek mknyszek added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Feb 1, 2023
@mknyszek mknyszek added this to the Backlog milestone Feb 1, 2023
@mknyszek
Copy link
Contributor

mknyszek commented Feb 1, 2023

Does this reproduce with earlier releases?

@mvdan
Copy link
Member

mvdan commented Feb 1, 2023

It does; I have a fix ready. It's a rather simple one, we were just assuming that .git was always a directory in -buildvcs=auto.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/463849 mentions this issue: cmd/go: support git worktree checkouts in -buildvcs=auto

@tie
Copy link
Contributor

tie commented Mar 16, 2023

As mentioned in #58978 and #59068, this issue also affects work trees inside bare Git repositories where running git status --porcelain fails since it must be run in a work tree. That is, go build with VCS stamping (including auto mode) fails in such setup.

  1. Go looks for .git, which is currently only allowed to be a directory.
  2. Finds .git directory from the bare repository (see also git config core.bare).
  3. Runs git status there, outside of the work tree.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/480635 mentions this issue: cmd/gomote: don't push .git even if it is a file

gopherbot pushed a commit to golang/build that referenced this issue Mar 30, 2023
In `git worktree` checkouts, rather than a .git directory, .git is a
file containing the path of the real repository. Presence of this file
will confuse git invoked by the go tool, so don't push it, just as we
don't push a .git directory.

For golang/go#58218.

Change-Id: I2d316d5672a87ba9cb7175a07e06e99a3392a85d
Reviewed-on: https://go-review.googlesource.com/c/build/+/480635
Reviewed-by: Carlos Amedee <[email protected]>
Run-TryBot: Michael Pratt <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Michael Pratt <[email protected]>
sharnoff added a commit to neondatabase/autoscaling that referenced this issue Oct 2, 2023
Noticed while working on #371, but it appears that golangci-lint
will sometimes hide errors that it *should* be showing if compiling
succeeds. So if, hypothetically, you're impacted by golang/go#58218 and
normally you're able to get by with 'go build -buildvcs=false ./...' but
golangci-lint doesn't have that functionality, then this is able to
reproduce on main.
@cehoffman
Copy link

Thanks for the start of the patch process @mvdan. I looked at the thread there, and I'm not sure what the remaining work is. Is the ask for a revert of the original logic change that broke worktrees with a regression test?

@tie
Copy link
Contributor

tie commented Mar 1, 2024

@cehoffman, hi, I don’t think there is any work to do here aside from getting CL 463849 rebased, reviewed and merged, ideally also backported to stable release.

I’ve also added comments with regression test suggestions for my particular use case (i.e. bare Git repository with work trees), but that shouldn’t be that different from the regression test already present in the CL.

I’ve been using Go toolchain with GOFLAGS=-buildvcs=false because of this issue, so I’d appreciate if it finally gets resolved 😅

@CodingMinds
Copy link

The discussion in https://go.dev/cl/463849 seems to be stalled. Does anyone know what's the state of this?

@matloob
Copy link
Contributor

matloob commented Sep 3, 2024

@mvdan Do you have context on this? Did you send out a fix with regression test?

@mvdan
Copy link
Member

mvdan commented Sep 3, 2024

I didn't do anything beyond https://go-review.googlesource.com/c/go/+/463849; writing the git worktree test and doing the revert is somewhere on my TODO list. I honestly don't use git worktrees at the moment so it's not my top priority. I'm happy for someone else to do this and I'll close my CL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

9 participants