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: automatic toolchain upgrades make the combination of go version and go install <package>@<version> confusing #66518

Open
dominikh opened this issue Mar 25, 2024 · 14 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

@dominikh
Copy link
Member

dominikh commented Mar 25, 2024

Go version

that's a good question!

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/dominikh/.cache/go-build'
GOENV='/home/dominikh/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/dominikh/prj/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/dominikh/prj'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/dominikh/prj/pkg/mod/golang.org/[email protected]'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/dominikh/prj/pkg/mod/golang.org/[email protected]/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.1'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/dominikh/bla/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 -ffile-prefix-map=/tmp/go-build3151298505=/tmp/go-build -gno-record-gcc-switches'

What did you do?

As the author of Staticcheck, I tell my users that to support Go 1.22 code, they have to build Staticcheck with Go 1.22. To assure me that they did, they show me this:

$ go version
go version go1.22.1 linux/amd64

$ go install honnef.co/go/tools/cmd/staticcheck@latest

The problem is that the user's local toolchain is Go 1.21.8, but the module they're currently in is specifying go 1.22.1 in its go.mod. Running go version downloads and runs the newer toolchain, as does go install ., but go install honnef.co/[...] does not.

This is problematic in a multitude of ways.

  • I don't know of any straightforward way to get the local toolchain's version without having to change the current working directory
  • Users wouldn't even know to use it if they aren't aware of this footgun
  • This interaction is just a symptom; the actual problem is that users have no good intuition for which version of Go they're actually using. In fact, this behavior confused 3 of 3 experienced Go users that I showed it to, as well as a dozen users of mine who struggled to build Staticcheck with Go 1.22. Most of them misdiagnosed the problem as the build cache not being invalidated correctly, because they thought they were building Staticcheck with 1.22 when they were in fact using their older, local toolchain.

That go install behaves this way is documented in go help install as

If the arguments have version suffixes (like @latest or @v1.0.0), "go install"
builds packages in module-aware mode, ignoring the go.mod file in the current
directory or any parent directory, if there is one. This is useful for
installing executables without affecting the dependencies of the main module.

This, however, predates the automatic upgrades. In my experience, users who are aware of automatic upgrades think of it as "First, go downloads the specified toolchain, then it runs the subcommand", which is true in most, but not all cases.

The issue I am filing is really a mix of two user reports, one from the perspective of a tool maintainer, and one from the perspective of a user. What they have in common is confusion over the effective Go version.

To avoid any confusion I've included a sample shell session demonstrating the current behavior.


chulak ~ ^$ go version
go version go1.21.8 linux/amd64
chulak ~ ^$ mkdir bla
chulak ~ ^$ cd bla
chulak ~/bla ^$ go mod init example.com
go: creating new go.mod: module example.com
chulak ~/bla ^$ go get [email protected]
go: updating go.mod requires go >= 1.22.1; switching to go1.22.1
go: upgraded go 1.21.8 => 1.22.1
chulak ~/bla ^$ go version
go version go1.22.1 linux/amd64
chulak ~/bla ^$ echo "package main\nfunc main(){}" >main.go
chulak ~/bla ^$ rm $GOPATH/bin/example.com
chulak ~/bla ^$ go install
chulak ~/bla ^$ go version $GOPATH/bin/example.com
/home/dominikh/prj/bin/example.com: go1.22.1
chulak ~/bla ^$ rm $GOPATH/bin/staticcheck 
chulak ~/bla ^$ go install honnef.co/go/tools/cmd/[email protected]
chulak ~/bla ^$ go version $GOPATH/bin/staticcheck 
/home/dominikh/prj/bin/staticcheck: go1.21.8
@seankhliao
Copy link
Member

go env has a read only env GOVERSION=.
perhaps go env should get a new env GOLOCALVERSION, and go version get a second line prefixes with local: ?

@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Mar 25, 2024
@thediveo
Copy link

thediveo commented Mar 25, 2024

go env has a read only env GOVERSION=. perhaps go env should get a new env GOLOCALVERSION, and go version get a second line prefixes with local: ?

To me, that would make it even more confusing. Please don't try to fix the opaque toolchain behavior by bolting on even more complexity.

@matloob
Copy link
Contributor

matloob commented Apr 4, 2024

I would expect go install package@version to use the toolchain in the go.mod file of the module version being installed. In this case it looks like the latest released version of go-tools is 0.4.7 whose go.mod declares 1.19. Since it's older that 1.21 there's no switching and it uses the local toolchain.

I see that your latest go.mod declares 1.22. When that's released users who build @latest will use go1.22, even if they were running 1.21 (and, let's say their current directory was in a 1.23 module). Is that not what you'd expect to happen with toolchain switching?

To put this another way, I think what makes this the most confusing is that we're crossing the 1.21 toolchain switching version boundary here with a 1.19 module, and a local 1.21 toolchain. This should be less confusing once we're on the other side of that boundary.

@dominikh
Copy link
Member Author

dominikh commented Apr 4, 2024

But if we assume that tools support the last two versions of Go, then the latest release of my module will have a go directive specifying an older version of Go than the latest. Users who are using a newer version of Go would still need to build with that newer version, which brings us back to this issue.

The behavior you describe is generally fine for most software written in Go, but not tools that operate on other Go code.

@matloob
Copy link
Contributor

matloob commented Apr 4, 2024

Hm I think I still need to understand something. Now that 1.21 and 1.22 are the last two versions of Go, can we assume that everyone (that we support) has a version of Go that's capable of switching to a newer toolchain and build the tool using that newest toolchain? Will your tool work with 1.21 code if it's built with 1.22?

@dominikh
Copy link
Member Author

dominikh commented Apr 4, 2024

Will your tool work with 1.21 code if it's built with 1.22?

Yes, but not vice versa.

Is your point that with automatic tool switching, there is no reason to support older versions of Go, and that go-tools should always specify the latest version of Go in its go.mod? Because I think that's problematic.

  • Updating the Go version can change the semantics of the code (cf. the for range loop variable scope change.)
  • It forces everyone who doesn't use automatic upgrades (such as Linux distributions packaging my tools) to support the newer version of Go.
  • If a new version of Go releases and I don't update my go.mod and make a new release in time, we're back to this very issue.

I don't want to be forced to depend on a newer version of Go for users to have an easier time go installing it.

@findleyr
Copy link
Member

First of all, I agree that the newly versioned world we live in is confusing, and I don't think we've done enough to mitigate that confusion. For example: the fact that go version depends on the directory from which it is run, yet this is not mentioned by go help version. Nor is it mentioned that go version can fail if the version in the go.mod file is a language version and not a toolchain version. IMO, it may have made more sense for go version to print its local version, and let go list -m go to be the way to get the toolchain version.

However, with respect to forcing toolchain upgrades:

In other words, I am hopeful that in the future most tools can upgrade their toolchain requirements frequently. This will allow for faster iteration with the standard library, and will make it more likely that fixes or new APIs get upstreamed rather than worked around. In another world, it would have been nice for go/parser, go/types, etc. to live in a separate module, but it's too late for that, and forward compatibility provides a similar outcome.

@matloob matloob added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 11, 2024
@dominikh
Copy link
Member Author

Can't you just set toolchain go1.22.2, and avoid the change to language semantics?

The toolchain directive is ignored when using go install ...@master.

$ cat $(GOTOOLCHAIN=auto ~/prj/go1.22/bin/go mod download -json github.com/dominikh/tmp@1eedde66af6d2824093a5d094a73cd0a954fb719 | jq -r .GoMod)
module github.com/dominikh/tmp

go 1.22

toolchain go1.23rc1

$ GOTOOLCHAIN=auto ~/prj/go1.22/bin/go version                                                                 
go version go1.22.4 linux/amd64

$ GOTOOLCHAIN=auto ~/prj/go1.22/bin/go install github.com/dominikh/tmp@1eedde66af6d2824093a5d094a73cd0a954fb719

$ GOTOOLCHAIN=auto ~/prj/go1.22/bin/go version $GOPATH/bin/tmp                                                 
/home/dominikh/prj/bin/tmp: go1.22.4

@findleyr
Copy link
Member

findleyr commented Jul 2, 2024

The toolchain directive is ignored when using go install ...@master.

Indeed, I misunderstood the toolchain directive as a requirement, but it is a suggestion. I'm surprised; as a suggestion is seems to have limited utility.

It it too late to change the behavior such that go install honors the toolchain switch relative to where it is first invoked? That seems like the most consistent behavior. A subsequent toolchain switch may be required to install the target, but we would preserve an invariant that the build Go version is at least the version implied by running go version.

IMO it's not too late to change this behavior, since module authors can't be relying on version downgrading.

@findleyr
Copy link
Member

findleyr commented Jul 3, 2024

Discussed this with @matloob, and here is my updated perspective :)

  • While I agree it is generally confusing that the output of go version depends on the module you are working in, @matloob points out that it is also confusing and inconsistent for go install to behave differently depending on where you are in the file system. After all, one of the points of having go install replace go get was to not depend on the working directory.
  • The new world of versioning we live in is complicated. I hope that we get used to using GOTOOLCHAIN= and go version $(which myprogram) the same way we got used to module versioning.

So I'm not sure there's anything we can do for this issue. I'm going to make sure gopls complains loudly and clearly when it is operating on code that it can't understand, including an instruction to reinstall @latest with GOTOOLCHAIN= ....

@rsc
Copy link
Contributor

rsc commented Jul 3, 2024

The solution here if you need Go 1.22 code is to set something like 'go 1.22.0' in your go.mod.

Updating the Go version can change the semantics of the code (cf. the for range loop variable scope change.)

This is a one-time event, and almost certainly staticcheck doesn't have any such bugs.

It forces everyone who doesn't use automatic upgrades (such as Linux distributions packaging my tools) to support the newer version of Go.

You could set go 1.21.0 in your go.mod. That's not the newest version of Go, but Go 1.21 is the oldest supported version, so it seems reasonable for you to require that.

If a new version of Go releases and I don't update my go.mod and make a new release in time, we're back to this very issue.

We work hard to ensure transition periods for new functionality like this. For example for the new AST alias node we added the node one release before we started using it (actually two but the plan was one). Our goal is that setting the go line to the oldest supported Go version is reasonable.

I also agree that 'go version' could potentially print more information, but that will take some extra thought.

@dominikh
Copy link
Member Author

dominikh commented Jul 3, 2024

Russ, this issue isn't about which version of Go Staticcheck needs per se, but about which version it gets built with when someone does go install. The version of go/types that Staticcheck (or any other code analysis tool) uses must be at least that of the module under analysis.

The problem is that the user might have some old version of Go installed, which gets automatically upgraded to a newer version when they work in a module that requires said newer version, except when they go install 3rd party tooling they need to work in the module.

As an extreme hypothetical, the user might have a local Go 1.21 and be working in a module that uses Go 1.40. Staticcheck, because it has great backwards compatibility, still can be built with Go 1.21. Built with Go 1.21, it can analyze Go 1.21 code. Built with Go 1.40, it can analyze Go 1.40 code. The user doesn't have to care about their local toolchain when building the module, thanks to automatic upgrading. But the moment the user go install's 3rd party tools, their local toolchain very much matters, and what they install may not work for their module.

To me, this contradicts the point of automatic upgrading.


Below are individual responses to parts of your comment; I wrote them before I wrote the above text, so they're fairly redundant. Kept here for completeness sake.

The solution here if you need Go 1.22 code is to set something like 'go 1.22.0' in your go.mod.

I don't need Go 1.22 code, unless I am to analyze Go 1.22 code, to get the latest go/types. If I only have to analyze Go 1.21 code, then I only need Go 1.21.

You could set go 1.21.0 in your go.mod. That's not the newest version of Go, but Go 1.21 is the oldest supported version, so it seems reasonable for you to require that.

I'm afraid we're going in circles. The suggestion was that I depend on the latest version of Go so that a user running an older version of Go, but working in a module requiring a newer version of Go, who uses go install gets Staticcheck built with the version required to work on their module.

We work hard to ensure transition periods for new functionality like this. For example for the new AST alias node we added the node one release before we started using it (actually two but the plan was one).

Yes, I can depend on Go 1.21 and already prepare for changes coming in Go 1.22. That's not what the problem is about, though.

Our goal is that setting the go line to the oldest supported Go version is reasonable.

That is very reasonable for the majority of Go code. For tools that work on Go code, it leads to the problem outlined above.

@seankhliao seankhliao removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 14, 2024
@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 16, 2024
@cherrymui cherrymui added this to the Backlog milestone Jul 16, 2024
@hyangah
Copy link
Contributor

hyangah commented Jul 19, 2024

VS Code Go project encountered the same problem golang/vscode-go#3168. The issue mentions staticcheck, but the problem applies to all tools VS Code Go depends on.

Currently there are two different paths for users to influence on go's toolchain switch behavior during development.
(other than adjusting PATH or using 3rd party version managers).

  1. specify the desired "go" or "toolchain" version in go.mod or go.work of the user's project module.
  2. specify the desired go version using GOTOOLCHAIN.

Option 1) is the easiest and probably a better option between the two.

With the option 1), users expect they are using the chosen go version when they run 'go' inside the module directory. Most go commands indeed meet the expectation: go version, go test, go generate, go get, go list, ...
And, with #68005, go run will soon be fixed to behave consistently.

However, go install <pkg>@<version> does not meet users' expectation, which is surprising.

As @dominikh explained, this does not work nicely if the tool's module cannot set its go.mod to use the latest go version. I can think of various reasons the module that includes the tool cannot update their go.mod more aggressively.

I understand that go install <pkg>@<version> installs outside the project module` to avoid the project's dependencies influencing the tool's dependencies. I think that makes sense for the third party dependencies of the tool. Many tool authors indeed want to pin their 3rd party dependencies. But do they also want to pin the go version to a specific version as well? I guess the answer is most likely no.

Then, I think this is a bug. The fix can be, I think, something similar to how go test and go generate handles - adjusting the PATH or GOROOT ... The tool continues to be built outside the module (modload.NoRoot) but the build is done by the same toolchain the users see when they run go version. (EDIT: the implementation in go install may be more involved, since unlike go test and go generate, go install still need to consider the tool's own module's go version requirement, too.)

  • Is this a niche problem that only tools like staticcheck will encounter?
    Originally, I thought so, but I was wrong.
    It can happen to any projects including binaries that choose to build differently based on the "go" version while using go.mod go version to indicate their minimum compatible version. (e.g. go version build tag)

@matloob
Copy link
Contributor

matloob commented Jul 25, 2024

Hi, after discussing with @hyangah and with @rsc and @samthanawalla, we've decided to change the toolchain switching behavior for go install package@version and go run package@version:

Currently it essentially runs in a new empty module and adds a dependency to package@version to that module.
We'd change the behavior so that the command essentially runs in a new empty module with the Go version set to the Go version of the module containing the current directory before adding the dependency to package@version to the module. (The process of adding the dependency would increase the go version of the module to the version of the module being installed if it's lower.)

This would ensure that the go version used to install the toolchain is at least the version reported by running go version in the directory go install is run in.

This has the same behavior proposed by @hyangah in #66518 (comment)

I think this should clear up the confusion around running go version and then go install package@version

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

8 participants