go/internal/gcimporter: lookupGorootExport should use the go command from build.Default.GOROOT

[zhangfannie]

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

$ go version
go version devel go1.21-e8fe3b7757 Thu Apr 13 02:29:25 2023 +0000 linux/arm64

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="arm64"
GOBIN=""
GOCACHE="/home/fanzha02/.cache/go-build"
GOENV="/home/fanzha02/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/fanzha02/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/fanzha02/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/fanzha02/work/go_project/govscode/gomain/"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/fanzha02/work/go_project/govscode/gomain/pkg/tool/linux_arm64"
GOVCS=""
GOVERSION="devel go1.21-e8fe3b7757 Thu Apr 13 02:29:25 2023 +0000"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/fanzha02/work/go_project/govscode/gomain/src/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 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3584490879=/tmp/go-build -gno-record-gcc-switches"

What did you do?

1. cd go/src
2. run ./make.base
3. cd go/src/runtime
4. run go/bin/go test -c // compile the test binary to runtime.test
5. run runtime.test

What did you expect to see?

PASS

What did you see instead?

--- FAIL: TestAtomicAlignment (0.50s)
align_test.go:92: typechecking runtime failed: mprof.go:11:2: (resolver.go/importPackage) could not import internal/abi (can't find import: "internal/abi")

[zhangfannie]

The reason for the runtime.test failure is that the TestAtomicAlign test calls the lookupGorootExport function. The following codes in this function cause this error. The go in the command returned by exec.command is the $PATH/go, not the $GOROOT/bin/go.

func lookupGorootExport(pkgDir string) (string, bool) {
  ...
  cmd := exec.Command("go", "list", "-export", "-f", "{{.Export}}", pkgDir)
  output, err := cmd.Output()
  ...
}

Another way to pass this test, if I add go/bin to the PATH, the test passes.

But as a user, I use go/bin/go to compile test binary files, and then when running, it
should use the go that compiled it. So from this side, it should be a bug, we cannot assume
or force users to add $GOROOT/bin/go to PATH, and then run the test.

If you think this is a problem too, I'll submit the fix path.

[bcmills]

The go in the command returned by exec.command is the $PATH/go, not the $GOROOT/bin/go.

In general either you need to use go test runtime (instead of go test -c runtime followed by separately running the binary), or you need to set the environment carefully before running that binary.

As of CL 404134, go test ensures that the go command resolved by exec.LookPath is the one in $GOROOT/bin, so it is reasonable for the test to assume that that is the case too (see #51473, #57050).

[zhangfannie]

@bcmills

you need to set the environment carefully before running that binary.

For runtime.test, if we only set the $GOROOT environment without putting $GOROOT/bin in $PATH, the test still fails. Is this failure acceptable?

Have 'go test' put the Go bin directory at the front of $PATH before running the test binary. (No new API!)

I saw the above comment from #51473, if this comment is possible, then go test -c will also put $GOROOT/bin in the $PATH before executing pkg.test binary, right?

Next I want to talk about another problem, which is about the lookupGorootExport. Do we need to change the "go" to "$GOROOT/bin/go" in the exec.Command, this change is not to fix the failure of runtime.test, but to conform to the functionality of this function.

func lookupGorootExport(pkgDir string) (string, bool) {
  ...
  cmd := exec.Command("go", "list", "-export", "-f", "{{.Export}}", pkgDir)
  output, err := cmd.Output()
  ...
}

Thank you.

[bcmills]

Do we need to change the "go" to "$GOROOT/bin/go" in the exec.Command ⋯ to conform to the functionality of this function.

Ah. Yes! We should probably be using filepath.Join(build.Default.GOROOT, "bin", "go") instead of just "go" for that.

[gopherbot]

Change https://go.dev/cl/484756 mentions this issue: cmd/compile/internal/importer,go/internal/gcimporter: use the 'go' command from build.Default.GOROOT in lookupGorootExport

[bcmills]

@gopherbot, please backport to Go 1.20. This could cause the importer to silently import from the wrong GOROOT in some cases.

[gopherbot]

Backport issue(s) opened: #59637 (for 1.20).

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]

Change https://go.dev/cl/484758 mentions this issue: cmd/compile/internal/importer,go/internal/gcimporter: use the 'go' command from build.Default.GOROOT in lookupGorootExport