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: improve error message when using wrong go command with Go source distribution #35520

Open
russelldavis opened this issue Nov 12, 2019 · 15 comments
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.

Comments

@russelldavis
Copy link

What did you do?

From the go/src/cmd dir:

gopls -rpc.trace -v check compile/main.go

What did you expect to see?

The command succeeds.

What did you see instead?

The command times out after 30s. Output: gopls.txt

Build info

golang.org/x/tools/gopls 0.2.0
    golang.org/x/tools/gopls@v0.2.0 h1:ddCHfScTYOG6auAcEKXCFN5iSeKSAnYcPv+7zVJBd+U=
    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/sergi/go-diff@v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
    golang.org/x/sync@v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
    golang.org/x/tools@v0.0.0-20191108194844-46f05828f2fe h1:FNzasIzfY1IIdyTs/+o3Qv1b7YdffPbBXyjZ5VJJdIA=
    golang.org/x/xerrors@v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc=
    honnef.co/go/tools@v0.0.1-2019.2.3 h1:3JgtbtFHMiCmsznwGVTUWbgGov+pVqnlf1dEJTNAXeM=

Go info

go version go1.13.4 darwin/amd64

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/russell/Library/Caches/go-build"
GOENV="/Users/russell/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/russell/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.13.4/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13.4/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/russell/dev/go/go/src/cmd/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/bk/1vdjn7vd19x41x0ztsbvmw400000gq/T/go-build918707310=/tmp/go-build -gno-record-gcc-switches -fno-common"
@gopherbot
Copy link

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 Issues related to the Go language server, gopls. label Nov 12, 2019
@russelldavis
Copy link
Author

I (partially) figured out what's going on here: the golang repo is special in that it can only be properly compiled or analyzed when GOROOT is set to its own location.

This is because much of its own code is importable as "standard packages" via GOROOT, and GOROOT packages take precedence. So if GOROOT points to a different installation of go, those packages will get imported instead, resulting in access errors for packages under internal directories (not to mention the code being the wrong version).

So, the fix is to set GOROOT when working with the golang repo.

I think this issue should remain open to fix the meta issue here, which is that gopls didn't output any useful info or errors - it just timed out after 30s. I'm still not totally sure exactly what it got hung up on -- I only discovered the solution after running go build inside the repo and seeing "use of internal package" errors. I'm guessing gopls might be running into something similar but failing to print the error.

@ianlancetaylor
Copy link
Contributor

Note that generally you should not set the GOROOT environment at all. It should only be needed in unusual circumstances.

@russelldavis
Copy link
Author

@ianlancetaylor thanks for jumping in. Can you clarify a bit? I just described a circumstance where setting GOROOT was needed -- are you just noting that this circumstance is unusual, or saying that it shouldn't be needed even in this case? (If the latter, can you elaborate on what the alternative would be?)

@ianlancetaylor
Copy link
Contributor

As far as I can see, it shouldn't be needed even in this case. I don't understand why it is needed. Is GOROOT set in the environment normally? If not, what does go env GOROOT print? Why is that different from the GOROOT value you want to use? For the GOROOT value you want to use, what is in GOROOT/bin?

@russelldavis
Copy link
Author

Ah, interesting, let's see if we can get to the bottom of it.

Is GOROOT set in the environment normally?

Nope

If not, what does go env GOROOT print?

/usr/local/Cellar/go/1.13.4/libexec
(the location of go I installed with homebrew)

Why is that different from the GOROOT value you want to use?

To be clear, it's not that I want to use a different GOROOT, just that things break if I don't. The value I need to change it to is the location of the local clone of https://github.com/golang/go/ that I'm trying to analyze with gopls.

For the GOROOT value you want to use, what is in GOROOT/bin?

Just go and gofmt, which were created by running src/make.bash.

@ianlancetaylor
Copy link
Contributor

Change your PATH to put the go tool that you want to use first. That is a better approach than setting GOROOT, as it means that the go tool and the GOROOT will be in synch.

@russelldavis
Copy link
Author

Ah that makes sense, thanks. (For posterity: gopls figures out the root by shelling out to go env GOROOT.)

With that resolved, any thoughts on ways we could make this easier for newcomers to the golang/go repo? If they're like me, they'll find a few things very broken without obvious reasons or solutions:

  • In VS Code, much of the go plugin functionality (like goto-definition) won't work.
  • In GoLand:
    • goto-definition works, but takes you to definitions at the locally installed go root, not in the repo itself
    • imports of internal packages get marked as errors (because they get resolved to the wrong root)
  • The gopls CLI has this issue

These are all simple once you know that, when using any kind of tooling on the contents of the golang repo, you must use the go binary built from that same repo (or set GOROOT to that repo). But AFAICT, this isn't mentioned anywhere -- should we add it somewhere, maybe https://golang.org/doc/contribute.html?

Even better would be if things could be changed so that wasn't a requirement -- in a perfect world, I'd be able to analyze the code in the golang repo without having to build that repo or mess with my path. But I assume that would be a huge/difficult change at this point.

@stamblerre
Copy link
Contributor

@russelldavis: I think you may have been looking at a file that is specifically broken with gopls (see #33548). For other files in the Go repo, gopls should work fine, but you're right in saying that your local modifications won't be visible through gopls unless you modify your PATH (something like export PATH=/path/to/go/bin:$PATH).

@russelldavis
Copy link
Author

@stamblerre sadly it's not just that file. It seems to be broken for everything under https://github.com/golang/go/tree/master/src/cmd. As I mentioned above, tooling in IDEs like VSCode and GoLand is broken for all of those files as well, for the same reason.

@stamblerre
Copy link
Contributor

GoLand doesn't use gopls, and I just tested gopls in VS Code with a few files in different directories under go/src/cmd, and it seems to work fine with everything except for go/src/cmd/main.go. This leads me to believe that there is something about your setup that's causing this issue. Can you share the output of gopls -rpc.trace -v check /path/to/file.go for another file that's broken for you?

@russelldavis
Copy link
Author

The larger issue I'm talking about here isn't specific to gopls (though I suspect gopls is being affected by it). I can create a new issue for it not tagged with gopls if you'd prefer.

The point is that an import of, e.g., cmd/compile/internal/gc, always gets resolved to the cmd module in GOROOT, even when used inside your own module named cmd (which of course happens in the golang repo). So the all the cmd/... imports in the golang repo get resolved to GOROOT's cmd rather than its own, and that causes things to break unless GOROOT happens to point back into the same repo.

This is definitely why there are issues in GoLand, and I'm pretty sure the cause of the issues in VSCode and gopls.

With that in mind, hopefully my suggestions at #35520 (comment) make more sense?

Here's a trace of gopls -rpc.trace -v check doc.go from go/src/cmd/compile:
gopls.txt

@stamblerre
Copy link
Contributor

I think probably that the best option here is for us to make error messages more explicit in gopls to indicate the user needs to add the GOROOT that they are inspecting to their PATH. Maybe the editor could notice that it looks like the user is looking at the Go repo and suggest updating their configurations.

One option that is currently available to you (to avoid mucking with your PATH), is creating a workspace setting in VS Code that sets the GOROOT just for this repository ("go.goroot": "/path/to/go/").

Even better would be if things could be changed so that wasn't a requirement -- in a perfect world, I'd be able to analyze the code in the golang repo without having to build that repo or mess with my path. But I assume that would be a huge/difficult change at this point.

I don't think it's possible to achieve this through the go command or go/packages alone. We can certainly investigate options in gopls for making the experience with the Go repository a bit more seamless.

@stamblerre stamblerre changed the title gopls: check on golang repo files times out x/tools/gopls: improve experience editing GOROOT with multiple GOROOTs Nov 14, 2019
@gopherbot gopherbot added this to the Unreleased milestone Nov 14, 2019
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Nov 14, 2019
@stamblerre stamblerre changed the title x/tools/gopls: improve experience editing GOROOT with multiple GOROOTs x/tools/gopls: improve experience editing golang/go with multiple copies Nov 14, 2019
@stamblerre stamblerre changed the title x/tools/gopls: improve experience editing golang/go with multiple copies x/tools/gopls: improve experience editing golang/go Dec 4, 2019
@stamblerre stamblerre modified the milestones: Unreleased, gopls v1.0 Dec 4, 2019
@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.5.0 May 21, 2020
@pjweinb
Copy link

pjweinb commented Jun 28, 2020

This just bit me too. There is a somewhat cryptic reference to GOROOT in the logs, but the message doesn't say what the user should do. If the user isn't looking at the log right then, the relevant message is hard to find in among all the other log messages.

Perhaps opening a source distribution with mismatched GOROOT deserves a special check and a visible message.

@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v1.0.0 Jul 22, 2020
@stamblerre stamblerre removed this from the gopls/v1.0.0 milestone Jul 29, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
@stamblerre stamblerre changed the title x/tools/gopls: improve experience editing golang/go x/tools/gopls: improve error message when using wrong go command with Go source distribution Nov 13, 2020
@stamblerre
Copy link
Contributor

The last thing remaining to improve here are the error messages when someone is using the wrong go command while working on the Go source distribution--leaving this issue open to track that work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants