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: version upgrade fails when go.mod files change on disk #40456

Closed
stamblerre opened this issue Jul 28, 2020 · 6 comments
Closed

x/tools/gopls: version upgrade fails when go.mod files change on disk #40456

stamblerre opened this issue Jul 28, 2020 · 6 comments
Labels
Milestone

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jul 28, 2020

@stamblerre stamblerre added this to the gopls/v1.0.0 milestone Jul 28, 2020
@danp
Copy link
Contributor

@danp danp commented Jul 29, 2020

I set up a small(er) reproduction of what I saw in this repo: https://github.com/danp/go-40456. Note this is not using vendoring as the project I mentioned in Slack is.

The main branch starts with k8s.io/client-go at v0.0.0-20190802021151-fdb3fbe99e1d and the change branch updates it to v0.18.6. The change branch also changes main.go to add the initial context.Context argument to List that v0.18.6 expects.

On changing from main to change, the call to List is highlighted with an error as if gopls/etc still expect the first (and only) arg to List to be a metav1.ListOptions.

Log from gopls with -rpc.trace attached. 09:08:xx is loading up main.go on the main branch, the entries beginning at 09:09:10.407 are when I changed to the change branch and the error appeared.

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Jul 29, 2020

Thank you so much for the repro case! We'll take a look.

@stamblerre stamblerre changed the title x/tools/gopls: add regression test for batched changes with go.mod changes included x/tools/gopls: version upgrade fails when go.mod files change on disk Jul 30, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 30, 2020

Change https://golang.org/cl/245779 mentions this issue: internal/lsp: invalidate all packages on go.mod file changes

@danp
Copy link
Contributor

@danp danp commented Jul 30, 2020

(Please let me know if you'd prefer this feedback on the CL)

I gave https://golang.org/cl/245779 a try with my repro and I'm still seeing the same behavior. Was wondering if it somehow relied on me having go.mod open but that didn't seem to matter. Added some quick logging around here to see what currentFH values appear and it was only ever related to main.go (with Kind == source.Go).

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Jul 30, 2020

I remember you had mentioned on Slack that you disabled file watching in some cases? You will need it enabled for your client to send gopls notifications when you switch branches. The problem no longer reproduces for me in your repro when using VS Code.

@danp
Copy link
Contributor

@danp danp commented Jul 30, 2020

It's correct that I had been disabling file watching for the original project I saw this in. Since it uses vendoring there are many files so I get the prompt asking whether I want to watch them all.

However, the reproduction project is much smaller and (I think) everything is watched by default as I'm not prompted. The testing/logging/etc I did above was done in the reproduction project.

Digging and debugging more, it's now looking like this has uncovered an issue in lsp-mode. It doesn't seem to be liking the glob change originally made in golang/tools@fd294ab. After changing GlobPattern back to **.go and adding a second FileSystemWatcher for **.mod, I'm getting the lsp-mode and gopls debug logging I expect.

Will open an lsp-mode issue for the globbing trouble. Thanks!

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
3 participants
You can’t perform that action at this time.