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

x/tools/gopls: reinitialize the workspace if the module directive changes #43186

Open
josharian opened this issue Dec 14, 2020 · 7 comments
Open

x/tools/gopls: reinitialize the workspace if the module directive changes #43186

josharian opened this issue Dec 14, 2020 · 7 comments
Labels
Milestone

Comments

@josharian
Copy link
Contributor

@josharian josharian commented Dec 14, 2020

I am working to eliminate a fork. The fork includes a change to the module import path. As a consequence, when I change git branches, the module import path changes. This causes gopls to be unable to load the workspace, and as a consequence nothing else works. (This includes saving my document without having to use my mouse to click a Cancel button and not having gofmt run). Once gopls is in this state, the only way I have found to recover is to restart VSCode.

Sample error message:

Error loading workspace: source error list: file:///Users/josh/t/wireguard-go/go.mod:0:0-0:33: go [-e -json -compiled=true -test=true -export=false -deps=true -find=false -- golang.zx2c4.com/wireguard golang.zx2c4.com/wireguard/device]: exit status 1: go: finding module for package github.com/tailscale/wireguard-go/ipc go: finding module for package github.com/tailscale/wireguard-go/wgcfg go: found github.com/tailscale/wireguard-go/ipc in github.com/tailscale/wireguard-go v0.0.20201118 go: golang.zx2c4.com/wireguard/device imports github.com/tailscale/wireguard-go/ipc: github.com/tailscale/wireguard-go@v0.0.20201118: parsing go.mod: module declares its path as: golang.zx2c4.com/wireguard but was required as: github.com/tailscale/wireguard-go
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Dec 15, 2020

Thanks for the report. We should treat a change to the module directive as something that forces a workspace reinitialization.

@stamblerre stamblerre changed the title x/tools/gopls: does not handle module directive changes gracefully x/tools/gopls: reinitialize the workspace if the module directive changes Dec 15, 2020
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Dec 15, 2020

@josharian: Can you share a repro case if you have one available? I saw you mentioned that you had one in your other issue.

@stamblerre stamblerre modified the milestones: Unreleased, vscode-g, gopls/vscode-go Dec 15, 2020
@heschik
Copy link
Contributor

@heschik heschik commented Dec 15, 2020

I don't understand precisely what happened here. I suppose a module, possibly a replace target, was renamed from github.com/tailscale to golang.zx2c4.com, and we still knew about old name, so tried to load it? And then it's a fork, so it causes a load failure without a replace?

If I had to guess it's because we failed to clear out workspace packages when the module name changed. But I'm really not sure.

Error loading workspace: source error list: file:///Users/josh/t/wireguard-go/go.mod:0:0-0:33:
go [-e -json -compiled=true -test=true -export=false -deps=true -find=false -- golang.zx2c4.com/wireguard golang.zx2c4.com/wireguard/device]:
exit status 1:
go: finding module for package github.com/tailscale/wireguard-go/ipc
go: finding module for package github.com/tailscale/wireguard-go/wgcfg
go: found github.com/tailscale/wireguard-go/ipc in github.com/tailscale/wireguard-go v0.0.20201118
go: golang.zx2c4.com/wireguard/device imports github.com/tailscale/wireguard-go/ipc:
    github.com/tailscale/wireguard-go@v0.0.20201118: parsing go.mod: module declares its path as: golang.zx2c4.com/wireguard but was required as: github.com/tailscale/wireguard-go
@stamblerre stamblerre added this to Needs Triage in vscode-go: gopls by default via automation Dec 16, 2020
@stamblerre stamblerre moved this from Needs Triage to Waiting for Info in vscode-go: gopls by default Dec 16, 2020
@josharian
Copy link
Contributor Author

@josharian josharian commented Dec 17, 2020

github.com/josharian/gopls-bug was what I wrote—change back and forth between the two commits. It doesn’t reproduce as easily in a small repo. You could also try GitHub.com/Tailscale/wireguard-go and switch back and forth between main and master branches.

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Dec 17, 2020

Thanks for the report. Was this the first time you had downloaded that repo and switched between those branches?

I see two issues here:

(1) The one @heschik described above--the module path change should cause us to delete workspace packages.
(2) After running go clean -modcache and switching between the two branches, I encountered:

[Trace - 00:57:26.984 AM] Received notification '$/progress'.
Params: {"token":"8674665223082153551","value":{"kind":"begin","title":"Error loading workspace","message":"go [-e -json -compiled=true -test=true -export=false -deps=true -find=false -- github.com/tailscale/wireguard-go github.com/tailscale/wireguard-go/conn github.com/tailscale/wireguard-go/device github.com/tailscale/wireguard-go/device/tokenbucket github.com/tailscale/wireguard-go/ipc github.com/tailscale/wireguard-go/ipc/winpipe github.com/tailscale/wireguard-go/ratelimiter github.com/tailscale/wireguard-go/replay github.com/tailscale/wireguard-go/rwcancel github.com/tailscale/wireguard-go/tai64n github.com/tailscale/wireguard-go/tun github.com/tailscale/wireguard-go/tun/tuntest github.com/tailscale/wireguard-go/tun/wintun/iphlpapi github.com/tailscale/wireguard-go/tun/wintun/namespaceapi github.com/tailscale/wireguard-go/tun/wintun/nci github.com/tailscale/wireguard-go/tun/wintun/registry github.com/tailscale/wireguard-go/tun/wintun/setupapi github.com/tailscale/wireguard-go/wgcfg]: exit status 1: go: golang.org/x/crypto@v0.0.0-20201124201722-c8d3bf9c5392: module lookup disabled by GOPROXY=off "}}

GOPROXY was set to off because it wasn't our first workspace load, but a branch change should probably be treated like a reinitialization. Unfortunately, we don't have a way of determining that the user has changed branches.

@josharian
Copy link
Contributor Author

@josharian josharian commented Dec 17, 2020

Was this the first time you had downloaded that repo and switched between those branches?

No, this has been happening for a while.

@stamblerre stamblerre moved this from Waiting for Info to Critical in vscode-go: gopls by default Dec 17, 2020
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Dec 23, 2020

Removing this from the gopls/v0.6.2 milestone because it raises issues closely related to #43305 and #42266. If we invalidate workspace packages when the module directive changes, but reinitialization only happens when the go.mod file is saved, we will have effectively invalidated the metadata by clearing out all of the workspace package paths without reloading.

This probably means that we should consider the workspacePackages map as part of "metadata" and also mark workspace packages invalid without deleting them, and then only delete them when metadata is fully invalidated on file save.

/cc @findleyr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.