Skip to content

tools: tools version check logic is broken with go 1.18 #1939

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

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

tools: tools version check logic is broken with go 1.18 #1939

hyangah opened this issue Dec 10, 2021 · 3 comments
Assignees
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker must be fixed before the next release.
Milestone

Comments

@hyangah
Copy link
Contributor

hyangah commented Dec 10, 2021

As reported in golang/go#50085 go 1.18 changes the encoding of the build info.
That means, if a tool is compiled with go1.18 and VS Code is configured to work with different versions of Go, go version -m command will fail.

The extension currently relies on go version -m for two things

We thought about utilizing go version -m to improve our tool update logic, but now I am afraid that this is an evidence that we shouldn't rely on go version -m unless the go version is the latest.

Currently the extension does not have logic to install the latest go version, or allow users to use different toolchains for tool management (probably we should. #825).

@gopherbot gopherbot added this to the Untriaged milestone Dec 10, 2021
@hyangah hyangah added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 13, 2021
@hyangah hyangah modified the milestones: Untriaged, v0.31.0 Dec 13, 2021
@hyangah hyangah modified the milestones: v0.31.0, v0.32.0 Jan 18, 2022
@hyangah hyangah added the go1.18 label Jan 26, 2022
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/382159 mentions this issue: src/goMain: use inspectGoToolVersion instead of runGoVersionM

hyangah added a commit to hyangah/vscode-go that referenced this issue Feb 1, 2022
And reduce the verbosity of `Go: Locate Configured Go Tools`.
With go1.18 which includes more details in go version -m output,
reporting everything from `go version -m` will be too excessive.

`Go: Locate Configured Go Tools` used runGoVersionM to
retrieve the build info of each located tool. However,
the default inspectGoToolVersion also utilizes `go version -m`,
so, they overlap with each other.

As we also want to reduce the verbosity of the `Go: Locate
Configured Go Tools` output, runGoVersionM is not much different
from inspectGoToolVersion any more. This CL unifies them.

Since go1.18 changes the build info encoding, we will see
failed `go version -m` more often. So, let's not print the error
output in the output channel.

For golang#1939

Change-Id: I1f674b030ca97804d34df63c66272cddb42b1c65
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/382159 mentions this issue: src/goMain: use inspectGoToolVersion instead of runGoVersionM

hyangah added a commit to hyangah/vscode-go that referenced this issue Feb 8, 2022
And reduce the verbosity of `Go: Locate Configured Go Tools`.
With go1.18 which includes more details in go version -m output,
reporting everything from `go version -m` will be too excessive.

`Go: Locate Configured Go Tools` used runGoVersionM to
retrieve the build info of each located tool. However,
the default inspectGoToolVersion also utilizes `go version -m`,
so, they overlap with each other.

As we also want to reduce the verbosity of the `Go: Locate
Configured Go Tools` output, runGoVersionM is not much different
from inspectGoToolVersion any more. This CL unifies them.

Since go1.18 changes the build info encoding, we will see
failed `go version -m` more often. So, let's not print the error
output in the output channel.

For golang#1939

Change-Id: I1f674b030ca97804d34df63c66272cddb42b1c65
@hyangah hyangah added the release-blocker must be fixed before the next release. label Feb 8, 2022
gopherbot pushed a commit that referenced this issue Feb 10, 2022
And reduce the verbosity of `Go: Locate Configured Go Tools`.
With go1.18 which includes more details in go version -m output,
reporting everything from `go version -m` will be too excessive.

`Go: Locate Configured Go Tools` used runGoVersionM to
retrieve the build info of each located tool. However,
the default inspectGoToolVersion also utilizes `go version -m`,
so, they overlap with each other.

As we also want to reduce the verbosity of the `Go: Locate
Configured Go Tools` output, runGoVersionM is not much different
from inspectGoToolVersion any more. This CL unifies them.

Since go1.18 changes the build info encoding, we will see
failed `go version -m` more often. So, let's not print the error
output in the output channel.

For #1939

Change-Id: I1f674b030ca97804d34df63c66272cddb42b1c65
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/382159
Trust: Hyang-Ah Hana Kim <[email protected]>
Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-by: Suzy Mueller <[email protected]>
@hyangah
Copy link
Contributor Author

hyangah commented Feb 15, 2022

With the changes:

  • Diagnosis tool ("Go: Locate Configured Go Tools") will not crash, but report versions are unknown when tools were compiled with go1.18+.
	go:	/Users/hakim/sdk/go1.17.7/bin/go: go version go1.17.7 darwin/amd64

	go-outline:	/Users/hakim/go/bin/go-outline	(version: unknown - )
	gotests:	/Users/hakim/go/bin/gotests	(version: unknown - )
	gomodifytags:	/Users/hakim/go/bin/gomodifytags	(version: unknown - )
	impl:	/Users/hakim/go/bin/impl	(version: unknown - )
	goplay:	/Users/hakim/go/bin/goplay	(version: unknown - )
	dlv:	/Users/hakim/go/bin/dlv	(version: unknown - )
	staticcheck:	/Users/hakim/go/bin/staticcheck	(version: unknown - )
	gopls:	/Users/hakim/go/bin/gopls	(version: v0.7.5 built with go: go1.17.7)
  • Tools update & consistency check logic: shouldUpdateTools will be false and not insist on updating when the go version is older.

  • Gopls update logic will continue to use gopls version.

Closing.

@hyangah hyangah closed this as completed Feb 15, 2022
@hyangah hyangah self-assigned this Mar 3, 2022
@golang golang locked and limited conversation to collaborators Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker must be fixed before the next release.
Projects
None yet
Development

No branches or pull requests

2 participants