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: errors in go.mod file prevents initial workspace load #36531

Closed
ridersofrohan opened this issue Jan 13, 2020 · 7 comments
Closed

x/tools/gopls: errors in go.mod file prevents initial workspace load #36531

ridersofrohan opened this issue Jan 13, 2020 · 7 comments

Comments

@ridersofrohan
Copy link

@ridersofrohan ridersofrohan commented Jan 13, 2020

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

$ go version
go version devel +8ac98e7b3f Thu Jan 9 18:00:06 2020 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

-- go.mod --
module mod.com

require golang.org/x/tools v0.0.0-20200107184032-11e9d9cc0042

yo

-- main.go --
package main

import "golang.org/x/tools/go/packages"

func Yo() {
	var _ packages.Config
}

What did you expect to see?

I expected the go.mod error to surface with the initial workspace load.

What did you see instead?

I saw that gopls logged the error due to the initial call packages.load call when the view is created. However, since there was an error with that initial load all the LSP features break. The error shows up once you save a .go file or start making edits in the go.mod file.

/cc @stamblerre

@gopherbot gopherbot added this to the Unreleased milestone Jan 13, 2020
@gopherbot gopherbot added the Tools label Jan 13, 2020
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 13, 2020

Thank you for filing a gopls issue! Please take a look at the Troubleshooting guide, and make sure that you have provided all of the relevant information here.

@gopherbot gopherbot added the gopls label Jan 13, 2020
@ridersofrohan

This comment has been minimized.

Copy link
Author

@ridersofrohan ridersofrohan commented Jan 13, 2020

There is another place where the error message should be checked if it is related to a go.mod parse error: internal/lsp/cache/load.go#L72. Adding the same check as on internal/lsp/cache/builtin.go#L40 here would cause the go.mod error to show up on initial load of the workspace.

However, even after the error is addressed and the go.mod file is saved, gopls does not seem to call packages.load again and the error diagnostic does not disappear from the go.mod file.

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Jan 14, 2020

As a follow-up to https://golang.org/cl/214417, I will look into making one go/packages.Load call for "builtin" and the workspace. This should at least reduce the amount of special-cases needed. We do still have to figure out (1) what gopls should do when it sees an invalid go.mod file on start, and (2) if gopls should try to re-initialize if initialization failed.

@stamblerre stamblerre modified the milestones: Unreleased, gopls v1.0 Jan 14, 2020
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 15, 2020

Change https://golang.org/cl/214877 mentions this issue: internal/lsp/cache: refactor initialization for builtins

@stamblerre stamblerre modified the milestones: gopls v1.0, gopls/v0.3.0 Jan 15, 2020
gopherbot pushed a commit to golang/tools that referenced this issue Jan 15, 2020
This change combines the two packages.Load calls that happen on view
creation. Builtins can be loaded along with the rest of the workspace.

To avoid race conditions, create a builtinPackageHandle type for
builtins and use it to create the data.

Updates golang/go#36531

Change-Id: I7aa342c463a0b7718e1ad5fee507622310d8443b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214877
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Jan 16, 2020

Now that the above CL has been merged, the main question remains: How should gopls behave when your go.mod is broken. Realistically, we can't do anything until the initial workspace load succeeds, but we have to recover gracefully.

The problem is that we don't want to keep trying to initialize if it keeps failing. I propose that, if we have an initialization error, we retry after every change to the go.mod.

@stamblerre stamblerre added the NeedsFix label Jan 16, 2020
@ridersofrohan

This comment has been minimized.

Copy link
Author

@ridersofrohan ridersofrohan commented Jan 16, 2020

Yeah, I think that approach works, since we will be able to surface go.mod errors even when the initial workspace load breaks.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 23, 2020

Change https://golang.org/cl/216037 mentions this issue: internal/lsp: recreate the view when needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.