x/tools/go/packages: use the correct go version for the analyzed project

[hyangah]

When the system Go version is newer than the GOTOOLCHAIN go version, we observed the x/tools/go/packages picks up the system Go version as the go version. This can cause the package to use the incompatible go list command. @matloob found this is because x/tools/internal/gocommand.GoVersion called by x/tools/go/packages.(*golistState).getGoVersion, uses GO111MODULE=off which prevents correct go version computation depending on toolchain switch.

This bug prevents gopls (v0.15.3, v0.16.0, v0.16.1) from operating with old go versions like go1.19 if the system Go version is newer. (e.g. using -pgo flag that doesn't exist in go1.19)

This will potentially break the future gopls versions if users rely on the GOTOOLCHAIN to switch to go versions and go/packages ends up using new go list flags that do not exist in the GOTOOLCHAIN version but exist in the system Go version.

We first need to understand why GoVersion uses GO111MODULE=off.

cc @golang/tools-team @matloob @findleyr

[gabyhelp]

Related Issues and Documentation

• x/tools/gopls: go list error opening Go source tree #35847 (closed)
• x/tools/go/packages: confusing error when loading source newer than toolchain used to build application #55883 (closed)
• cmd/go: go list incorrectly says that can only use path@version syntax with 'go get' #37218 (closed)
• x/tools/go/packages: New version of go-tools doesn't support go1.10 #32575 (closed)
• x/build/cmd/golangbuild: disable GOTOOLCHAIN=local for the x/tools/gopls module #67749 (closed)
• x/tools/go/packages: should it respect GOROOT? #41231 (closed)
• x/tools/gopls: gopls version output is incorrect #43256 (closed)
• x/tools/go/packages: error during the call to `packages.Load` with `-mod` other than empty. #43418 (closed)
• x/tools/gopls: add support for a target language version, and drop support for building at legacy Go versions #50825 (closed)
• x/tools/gopls: version from master needs metadata #37698 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[findleyr]

This is not going to get fixed for [email protected].

@matloob do you understand why GoVersion uses GO111MODULE=off?

[adonovan]

do you understand why GoVersion uses GO111MODULE=off?

The actual logic dates from https://go.dev/cl/263985, but that doesn't explain why.
@matloob, do you know?

[jakebailey]

I'm not matloob (😅), but I remember reading this code a really long time ago when it was first sent, and remember it being slightly faster to invoke go list and others without module mode enabled. When I test that with hyperfine, it's barely true, maybe 0.1ms.

However, actually digging, that code actually stemmed from CL 253799 where it was previously the code for modfileFlagExists, which traces back to CL 211937, then eventually all the way to CL 212477 where it was seemingly just to deal with the temp modfile back when that was a thing. (And I guess, not in this case for perf; I was probably thining of go env being slow, which is still true by a bit.)

[matloob]

I am matloob but I think @jakebailey has a better answer than I do!

I really can't think of a good reason to set GO111MODULE=off for go version. Before we had toolchain switching/selection, go version's behavior shouldn't have changed at all depending on the value of GO111MODULE.

[gopherbot]

Change https://go.dev/cl/666715 mentions this issue: internal/gocommand: don't disable Go modules

[adonovan]

Thanks. I'll mail you a CL to remove it.