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 version -m fails for target binaries compiled with go1.18 #50085

Closed
hyangah opened this issue Dec 10, 2021 · 9 comments
Closed

cmd/go: go version -m fails for target binaries compiled with go1.18 #50085

hyangah opened this issue Dec 10, 2021 · 9 comments
Labels
Documentation FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker Unfortunate
Milestone

Comments

@hyangah
Copy link
Contributor

hyangah commented Dec 10, 2021

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

go version go1.17.5 darwin/amd64

What did you do?

$ gotip version
go version devel go1.18-8ff254e30b Thu Dec 9 19:01:08 2021 +0000 darwin/amd64
$ gotip version -m ~/go/bin/gopls
/Users/hakim/go/bin/gopls: devel go1.18-8ff254e30b Thu Dec 9 19:01:08 2021 +0000
	path	golang.org/x/tools/gopls
	mod	golang.org/x/tools/gopls	v0.7.3	
h1:Lru57ht8vtDMouRskFC085VAjBAZRAISd/lwvwOOV0Q=
...

What did you expect to see?

go1.17.5 version -m ~/go/bin/gopls reports at least the go version and the main module version info.

What did you see instead?

$ go1.17.5 version -m ~/go/bin/gopls
/Users/hakim/go/bin/gopls: go version not found

Older version of Go cannot query the version info of the binary built with a newer version of Go at all.

vscode-go wanted to rely on the version info to inspect compatibility between go toolchains and other tools. For example, we could imagine vscode-go avoids requiring users to recompile gopls if gopls was compiled with go1.18 and a user still wants to use go1.17 temporarily, the user doesn't need to recompile gopls. But this issue shuts down the idea.

Would've been nice if the build info encoding format was backwards compatible - for example, depending on the encoded go version, parse more bytes.

(Another possibility is vscode-go always carries the latest go version with it and use it for version check)

@bcmills
Copy link
Member

bcmills commented Dec 10, 2021

This is probably due to CL 369977, which changed the encoding of the version string to fix a bug in go version on darwin/arm64 arising from linker relocations.

(CC @cherrymui @rsc)

@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 10, 2021
@cherrymui
Copy link
Member

cherrymui commented Dec 10, 2021

I looked at the parser in 1.17. It seems hard to make it completely backward compatible. It needs at least one bit to tell if the new format is used, but the 1.17 parser seems to need all bits? If this line

bigEndian := data[15] != 0

were

bigEndian := data[15]&1 != 0

then we'd have some free bits. Maybe there is a clever way. I'm not sure.

The problem for the old format is that in some configurations it cannot find the Go version at all, so we cannot rely on that.

When would one want to use a gopls built with the new toolchain while still using the old toolchain? Does vscode-go shell out the go command somehow? If so, could it hint the user to user to choose the new go command (even just temporarily for that one)?

@cherrymui
Copy link
Member

Or vscode-go could use the debug/buildinfo package (available in 1.18) to do its own parsing, without shelling out the go command. So if vscode-go is built with the new toolchain, it should be able to handle other tools built with the new toolchain, regardless of the go command's version.

@hyangah
Copy link
Contributor Author

hyangah commented Dec 10, 2021

Thanks for looking into it. That is unfortunate. Is the current format forwards-compatible? Would be nice if we can avoid repeating this in the future.

This is a user visible change so I think it's worth documenting this in the release note. Users need to update their Go to 1.18 if they want to inspect binaries built/distributed by other parties who potentially could selected go1.18.

When would one want to use a gopls built with the new toolchain while still using the old toolchain?

Switching between go versions is not uncommon. We've been recommending users to compile gopls with the latest Go version and keep using it when they work with older versions of Go.

Does vscode-go shell out the go command somehow? If so, could it hint the user to user to choose the new go command (even just temporarily for that one)?

The extension has allowed users to configure only one Go version at a time and used it both for checking the tools versions and building user's project. Maybe we should add another setting to separate them.

vscode-go could use the debug/buildinfo package (available in 1.18) to do its own parsing, without shelling out the go command.

vscode-go is a typescript project. :-/

The best path forward for us is to add another setting or pick the most recent Go versions it can find from the system.

Or, maybe we can use gopls for checking versions of other tools - because it's easier for vscode-go to manage gopls version than the go toolchain at this moment. (cc @findleyr )

@bcmills bcmills added okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 release-blocker labels Dec 13, 2021
@bcmills bcmills added this to the Go1.18 milestone Dec 13, 2021
@cherrymui cherrymui removed the okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 label Dec 14, 2021
@cherrymui
Copy link
Member

use gopls for checking versions of other tools

I think this is a good idea. It can use the debug/buildinfo package. And gopls can report its own version to vscode-go.

@rsc
Copy link
Contributor

rsc commented Dec 15, 2021

I don't believe it is reasonable to make a guarantee that older Go versions can understand binaries from newer Go versions. That restricts too much what we can change or fix in newer Go versions. In this case, the way the versions were encoded was fundamentally incompatible with certain kinds of binaries and was overly complicated for all kinds of binaries. We greatly simplified it in Go 1.18. If we had to preserve compatibility with 'go1.17 version', that would mean more complexity in the binaries that we carry around until the end of time. It doesn't seem like the right tradeoff. Instead, the rule is: to analyze new Go binaries, use a new Go toolchain.

If we could go back in time, I would suggest that gopls have a -V flag that makes it print its version and exit, same as 'go tool compile' and friends all do. Maybe that's a better future-proof way at least.

@rsc
Copy link
Contributor

rsc commented Dec 15, 2021

I also discussed this earlier today with @bcmills and @matloob and they agreed. Closing this as unfortunate but working as intended.

@hyangah
Copy link
Contributor Author

hyangah commented Dec 15, 2021

@rsc I left this issue open for documentation purpose - release note.

Gopls already has its own version command. My intention was to utilize go version -m to inspect all 3rd party tools' versions in an consistent way, not only gopls. I wish there was an handy way to inspect whether a binary given to me was compiled with go1.17 or go1.17 (not dependency or git commit), and grep the right version.

@gopherbot
Copy link

Change https://golang.org/cl/372214 mentions this issue: doc/go1.18: discuss embedded build info compatibility

gopherbot pushed a commit that referenced this issue Dec 15, 2021
Fixes #50085

Change-Id: I9be8ddb983fb4fe598becbb0b93bb5b7e1f8438f
Reviewed-on: https://go-review.googlesource.com/c/go/+/372214
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
@golang golang locked and limited conversation to collaborators Dec 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker Unfortunate
Projects
None yet
Development

No branches or pull requests

7 participants