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: import organization sporadically failing #38669

Closed
atombender opened this issue Apr 26, 2020 · 33 comments
Closed

x/tools/gopls: import organization sporadically failing #38669

atombender opened this issue Apr 26, 2020 · 33 comments
Labels
Milestone

Comments

@atombender
Copy link

@atombender atombender commented Apr 26, 2020

What did you do?

Added a reference to a package in my project and hit "save" in VSCode.

What did you expect to see?

Import should be added automatically.

What did you see instead?

Nothing happened.

More information

This is a regression that appears to have been introduced in 0.4.0. I have no idea how to reproduce it, though it seems to happen > 50% of the time now. Project is building fine when this occurs.

Log looks normal (I am inserting a utils.Spew() function here): gopls.log.gz.

Also tested latest master.

Build info

golang.org/x/tools/gopls 0.4.0
    golang.org/x/tools/gopls@(devel)
    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/mod@v0.2.0 h1:KU7oHjnv3XNWfa5COkzUifxZmxp1TyI7ImMXqFxLwvQ=
    golang.org/x/sync@v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY=
    golang.org/x/tools@v0.0.0-20200407041343-bf15fae40dea => ../
    golang.org/x/xerrors@v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
    honnef.co/go/tools@v0.0.1-2020.1.3 h1:sXmLre5bzIR6ypkjXCDI3jHPssRhc8KD/Ome589sc3U=
    mvdan.cc/xurls/v2@v2.1.0 h1:KaMb5GLhlcSX+e+qhbRJODnUUBvlw01jt4yrjFIHAuA=

Go info

go version go1.14 darwin/amd64

@gopherbot gopherbot added this to the Unreleased milestone Apr 26, 2020
@gopherbot gopherbot added the Tools label Apr 26, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 26, 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 Apr 26, 2020
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 26, 2020

Based on the logs, I don't think the issue is import organization, and I suspect that other features may not be working for you as well. It looks like gopls isn't able to load the package for the file /Users/alex/Projects/Sanity/gradient/pkg/system/namespaces/group_cache.go. Was this a newly created file? What is the output of go list -e -json -test ./... in the /Users/alex/Projects/Sanity/gradient/pkg/system/namespaces/ directory?

@atombender
Copy link
Author

@atombender atombender commented Apr 26, 2020

No, not a newly created file. Here's the output of go list: golist.txt.gz. Note that I have since renamed the folder to groupcache.

Just now I noticed this in the gopls log, maybe related?

[Trace - 13:28:47.668 PM] Received notification 'window/logMessage'.
Params: {"type":3,"message":"2020/04/26 13:28:47 \n\tmessage=\"background imports cache refresh starting\"\n\tnil\n\tnil"}


[Info  - 1:28:47 PM] 2020/04/26 13:28:47 
	message="background imports cache refresh starting"
	nil
	nil
[Trace - 13:28:48.962 PM] Received notification 'window/logMessage'.
Params: {"type":3,"message":"2020/04/26 13:28:48 \n\tmessage=\"background refresh finished after 1.294148715s\"\n\tnil\n\tnil\n\terror=%!v(PANIC=Format method: runtime error: invalid memory address or nil pointer dereference)"}


[Info  - 1:28:48 PM] 2020/04/26 13:28:48 
	message="background refresh finished after 1.294148715s"
	nil
	nil
	error=%!v(PANIC=Format method: runtime error: invalid memory address or nil pointer dereference)
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 26, 2020

Does restarting gopls help?

The fix for the issue you mentioned has been merged (https://golang.org/cl/229985). Can you try rebuilding, if you're using master?

@atombender
Copy link
Author

@atombender atombender commented Apr 26, 2020

Restarting doesn't help. Thanks, I will try rebuilding.

@atombender
Copy link
Author

@atombender atombender commented Apr 26, 2020

No change after upgrading.

Some more details:

  • This problem only applies to some packages.
  • If I simply type utils, there's no autocompletion offered for it.
  • However, some packages have no autocompletion suggestion, but import-adding does work fine for them.
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 26, 2020

The issue seems to be related to the failure to load the package for /Users/alex/Projects/Sanity/gradient/pkg/system/namespaces/group_cache.go (now renamed to /Users/alex/Projects/Sanity/gradient/pkg/system/groupcache/groupcache.go), based on your logs. Is this a public repository? If so, I could try to reproduce the issue myself.

The go list output looked correct, but maybe go/packages is failing for some reason. Can you try the following?

$ go get golang.org/x/tools/go/packages/gopackages
$ cd /Users/alex/Projects/Sanity/gradient/pkg/system/groupcache
$ gopackages -test -json file=groupcache.go
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 Apr 26, 2020
@stamblerre stamblerre changed the title x/tools/gopls: Import organization sporadically failing x/tools/gopls: import organization sporadically failing Apr 26, 2020
@atombender
Copy link
Author

@atombender atombender commented Apr 26, 2020

Sorry, the repo isn't public. I can see if I can reproduce it with a public repo later.

$ go get golang.org/x/tools/go/packages/gopackages
go: found golang.org/x/tools/go/packages/gopackages in golang.org/x/tools v0.0.0-20200426102838-f3a5411a4c3b
go: golang.org/x/tools upgrade => v0.0.0-20200426102838-f3a5411a4c3b
$ cd pkg/system/groupcache
$ gopackages -test -json file=groupcache.go
{
	"ID": "github.com/sanity-io/gradient/pkg/system/groupcache",
	"Name": "groupcache",
	"PkgPath": "github.com/sanity-io/gradient/pkg/system/groupcache",
	"GoFiles": [
		"/Users/alex/Projects/Sanity/gradient/pkg/system/groupcache/entry.go",
		"/Users/alex/Projects/Sanity/gradient/pkg/system/groupcache/groupcache.go"
	],
	"CompiledGoFiles": [
		"/Users/alex/Projects/Sanity/gradient/pkg/system/groupcache/entry.go",
		"/Users/alex/Projects/Sanity/gradient/pkg/system/groupcache/groupcache.go"
	],
	"Imports": {
		"context": "context",
		"fmt": "fmt",
		"github.com/hashicorp/golang-lru": "github.com/hashicorp/golang-lru",
		"github.com/rs/zerolog/log": "github.com/rs/zerolog/log",
		"github.com/sanity-io/gradient/pkg/app/informer": "github.com/sanity-io/gradient/pkg/app/informer",
		"github.com/sanity-io/gradient/pkg/documents": "github.com/sanity-io/gradient/pkg/documents",
		"github.com/sanity-io/gradient/pkg/documents/datatypes/paths": "github.com/sanity-io/gradient/pkg/documents/datatypes/paths",
		"github.com/sanity-io/gradient/pkg/documents/schemas/schemaformat/builtins": "github.com/sanity-io/gradient/pkg/documents/schemas/schemaformat/builtins",
		"github.com/sanity-io/gradient/pkg/documents/storage": "github.com/sanity-io/gradient/pkg/documents/storage",
		"github.com/sanity-io/gradient/pkg/documents/systemdocuments": "github.com/sanity-io/gradient/pkg/documents/systemdocuments",
		"github.com/sanity-io/gradient/pkg/event/eventbus": "github.com/sanity-io/gradient/pkg/event/eventbus",
		"github.com/sanity-io/gradient/pkg/search/expressionopt": "github.com/sanity-io/gradient/pkg/search/expressionopt",
		"github.com/sanity-io/gradient/pkg/search/expressions": "github.com/sanity-io/gradient/pkg/search/expressions",
		"github.com/sanity-io/gradient/pkg/security": "github.com/sanity-io/gradient/pkg/security",
		"github.com/sanity-io/gradient/pkg/system/versioning": "github.com/sanity-io/gradient/pkg/system/versioning",
		"io": "io",
		"sync": "sync",
		"time": "time"
	}
}
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 26, 2020

No worries. Looks like the go/packages output is fine as well. Would you mind producing a new gopls log when you see utils not offering any completions and not adding imports? Please make sure to include the full log, from the time the server starts. If you can also add the following setting, that would make the output more detailed:

"gopls": {
	"verboseOutput": true,
}
@atombender
Copy link
Author

@atombender atombender commented Apr 26, 2020

New log: gopls.log.gz

gopls doesn't seem to be a VSCode setting, but I added -v to go.languageServerFlags, good enough?

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 26, 2020

VS Code will not recognize the setting, but it will still work (see these docs). The -v will not have the same effect, but this log is fine - thank you.

Just to confirm since I haven't asked this yet - does the utils package which you are trying to import already exist in your module cache (in $GOPATH/pkg/mod)? goimports cannot suggest anything that isn't already in your module cache, so if it's not there yet, you will either have to download it with a go get or manually type the import path so that gopls can download it.

@atombender
Copy link
Author

@atombender atombender commented Apr 26, 2020

This is about packages in my repo in the same module, not external modules.

VS Code will not recognize the setting, but it will still work (see these docs). The -v will not have the same effect, but this log is fine - thank you.

Thanks, I will turn it on.

@atombender
Copy link
Author

@atombender atombender commented Apr 26, 2020

see these docs

That link gives me "Permission denied".

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 26, 2020

Thanks for clarifying - I'll take another look at your log and see if I can spot the issue.

Regarding the verbose output setting, I'd actually suggest leaving it off in your case. It's only useful for debugging and since you have a lot of packages in your workspace it may slow things down.

Sorry about the link, this one will be better: https://github.com/golang/tools/blob/master/gopls/doc/vscode.md.

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 26, 2020

Change https://golang.org/cl/229988 mentions this issue: internal/lsp: debugging CL for golang/go#38669

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 26, 2020

Just uploaded https://golang.org/cl/229988 which may help isolate the issue. You can rebuild gopls with it with the following steps:

$ git clone https://go.googlesource.com/tools
$ cd tools
$ git fetch "https://go.googlesource.com/tools" refs/changes/88/229988/1 && git cherry-pick FETCH_HEAD
$ cd gopls
$ go install

If you could share another log with gopls at this CL, that would be very helpful. Thanks!

@atombender
Copy link
Author

@atombender atombender commented Apr 27, 2020

Thanks. Here's the log: gopls.log.gz

I'm suddenly not able to reproduce the autocomplete problem for that particular package anymore; autocompletion finds its symbols, though it's only adding the import if I follow the autocompletion. On-save "organize imports" still fails to add it.

Here's something potentially interesting, though:

gopls3

Here, I reloaded the VSCode window and starting typed utils, then deleting the last s and typing it again, repeatedly. As you see, the autocompletions come and go and change (presumably because gopls is asynchronously following all modules?). In the beginning, the right utils package is the first offered completion, but it quickly disappears.

Note that autocompletions appear to be in random order even after gopls has apparently found all modules. That seems wrong to me. Also, they shouldn't disappear!

But beyond that, obviously the first utils package offered should be the one in my module.

@atombender
Copy link
Author

@atombender atombender commented Apr 27, 2020

Another log for another package called protofy: gopls.log.gz. Difference here is that it does not autocomplete any symbols at all if I type protofy..

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 28, 2020

Thank you for the new logs. Looks like there are no errors - just empty results from import organization. It's very strange that unimported completions would work and imports wouldn't.

I'll let @heschik take over the investigation from here, but one thing to try is to see if gopls -rpc.trace -v imports -d path/to/file.go works. If not, this will be an easier way to get log output anyway. If you're able to reproduce in any public package, that would be very helpful, because then we'll be able to investigate on our own.

Also, the issues you're seeing with completion are being tracked in #38104.

@atombender
Copy link
Author

@atombender atombender commented Apr 28, 2020

I will find a public repo now and try it out. Meanwhile:

$ gopls -rpc.trace -v imports -d  pkg/system/groupcache/groupcache.go
2020/04/27 22:01:09 Info:2020/04/27 22:01:09
	message="Build info\n----------\ngolang.org/x/tools/gopls master\n    golang.org/x/tools/gopls@(devel)\n    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=\n    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=\n    golang.org/x/mod@v0.2.0 h1:KU7oHjnv3XNWfa5COkzUifxZmxp1TyI7ImMXqFxLwvQ=\n    golang.org/x/sync@v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY=\n    golang.org/x/tools@v0.0.0-20191130070609-6e064ea0cf2d => ../\n    golang.org/x/xerrors@v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=\n    honnef.co/go/tools@v0.0.1-2020.1.3 h1:sXmLre5bzIR6ypkjXCDI3jHPssRhc8KD/Ome589sc3U=\n    mvdan.cc/xurls/v2@v2.1.0 h1:KaMb5GLhlcSX+e+qhbRJODnUUBvlw01jt4yrjFIHAuA=\n\nGo info\n-------\ngo version go1.13.10 darwin/amd64\n\n"
	nil
	nil
2020/04/27 22:01:09 Info:2020/04/27 22:01:09
	message="go env for /Users/alex/Projects/Sanity/gradient\n(valid build configuration = true)\n(build flags: [])\nGO111MODULE=\"\"\nGOARCH=\"amd64\"\nGOBIN=\"\"\nGOCACHE=\"/Users/alex/Library/Caches/go-build\"\nGOENV=\"/Users/alex/Library/Application Support/go/env\"\nGOEXE=\"\"\nGOFLAGS=\"\"\nGOHOSTARCH=\"amd64\"\nGOHOSTOS=\"darwin\"\nGONOPROXY=\"github.com/t11e/*,github.com/sanity-io/gradient\"\nGONOSUMDB=\"github.com/t11e/*,github.com/sanity-io/gradient\"\nGOOS=\"darwin\"\nGOPATH=\"/Users/alex/.go\"\nGOPRIVATE=\"github.com/t11e/*,github.com/sanity-io/gradient\"\nGOPROXY=\"https://proxy.golang.org,direct\"\nGOROOT=\"/usr/local/Cellar/go@1.13/1.13.10_1/libexec\"\nGOSUMDB=\"sum.golang.org\"\nGOTMPDIR=\"\"\nGOTOOLDIR=\"/usr/local/Cellar/go@1.13/1.13.10_1/libexec/pkg/tool/darwin_amd64\"\nGCCGO=\"gccgo\"\nAR=\"ar\"\nCC=\"clang\"\nCXX=\"clang++\"\nCGO_ENABLED=\"1\"\nGOMOD=\"/Users/alex/Projects/Sanity/gradient/go.mod\"\nCGO_CFLAGS=\"-g -O2\"\nCGO_CPPFLAGS=\"\"\nCGO_CXXFLAGS=\"-g -O2\"\nCGO_FFLAGS=\"-g -O2\"\nCGO_LDFLAGS=\"-g -O2\"\nPKG_CONFIG=\"pkg-config\"\nGOGCCFLAGS=\"-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/qs/wpmg19r12_9_nz7pvvs2_82r0000gn/T/go-build089195236=/tmp/go-build -gno-record-gcc-switches -fno-common\"\n"
	nil
	nil
2020/04/27 22:01:10 Info:2020/04/27 22:01:10
	message="go/packages.Load"
	nil
	nil
	snapshot=0
	directory=/Users/alex/Projects/Sanity/gradient
	query=[./... builtin]
	packages=322
--- /Users/alex/Projects/Sanity/gradient/pkg/system/groupcache/groupcache.go.orig
+++ /Users/alex/Projects/Sanity/gradient/pkg/system/groupcache/groupcache.go
@@ -20,6 +20,7 @@
 	"github.com/sanity-io/gradient/pkg/security"
 	"github.com/sanity-io/gradient/pkg/system/informer"
 	"github.com/sanity-io/gradient/pkg/system/versioning"
+	"github.com/sanity-io/gradient/pkg/utils"
 )

 var logger = log.With().Str("component", "groupcache").Logger()
@atombender
Copy link
Author

@atombender atombender commented Apr 28, 2020

Just realized my environment has GOFLAGS="-tags=integration". This is because I work on a lot of integration tests a lot, and gopls doesn't work otherwise.

None of the files I've mentioned use build tags, but I see that if I run the above command with that environment variable set, it does not organize imports:

$ GOFLAGS="-tags=integration" gopls -rpc.trace -v imports -d  pkg/system/groupcache/groupcache.go
2020/04/27 22:08:11 Info:2020/04/27 22:08:11
	message="Build info\n----------\ngolang.org/x/tools/gopls master\n    golang.org/x/tools/gopls@(devel)\n    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=\n    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=\n    golang.org/x/mod@v0.2.0 h1:KU7oHjnv3XNWfa5COkzUifxZmxp1TyI7ImMXqFxLwvQ=\n    golang.org/x/sync@v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY=\n    golang.org/x/tools@v0.0.0-20191130070609-6e064ea0cf2d => ../\n    golang.org/x/xerrors@v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=\n    honnef.co/go/tools@v0.0.1-2020.1.3 h1:sXmLre5bzIR6ypkjXCDI3jHPssRhc8KD/Ome589sc3U=\n    mvdan.cc/xurls/v2@v2.1.0 h1:KaMb5GLhlcSX+e+qhbRJODnUUBvlw01jt4yrjFIHAuA=\n\nGo info\n-------\ngo version go1.13.10 darwin/amd64\n\n"
	nil
	nil
2020/04/27 22:08:11 Info:2020/04/27 22:08:11
	message="go env for /Users/alex/Projects/Sanity/gradient\n(valid build configuration = true)\n(build flags: [])\nGO111MODULE=\"\"\nGOARCH=\"amd64\"\nGOBIN=\"\"\nGOCACHE=\"/Users/alex/Library/Caches/go-build\"\nGOENV=\"/Users/alex/Library/Application Support/go/env\"\nGOEXE=\"\"\nGOFLAGS=\"-tags=integration\"\nGOHOSTARCH=\"amd64\"\nGOHOSTOS=\"darwin\"\nGONOPROXY=\"github.com/t11e/*,github.com/sanity-io/gradient\"\nGONOSUMDB=\"github.com/t11e/*,github.com/sanity-io/gradient\"\nGOOS=\"darwin\"\nGOPATH=\"/Users/alex/.go\"\nGOPRIVATE=\"github.com/t11e/*,github.com/sanity-io/gradient\"\nGOPROXY=\"https://proxy.golang.org,direct\"\nGOROOT=\"/usr/local/Cellar/go@1.13/1.13.10_1/libexec\"\nGOSUMDB=\"sum.golang.org\"\nGOTMPDIR=\"\"\nGOTOOLDIR=\"/usr/local/Cellar/go@1.13/1.13.10_1/libexec/pkg/tool/darwin_amd64\"\nGCCGO=\"gccgo\"\nAR=\"ar\"\nCC=\"clang\"\nCXX=\"clang++\"\nCGO_ENABLED=\"1\"\nGOMOD=\"/Users/alex/Projects/Sanity/gradient/go.mod\"\nCGO_CFLAGS=\"-g -O2\"\nCGO_CPPFLAGS=\"\"\nCGO_CXXFLAGS=\"-g -O2\"\nCGO_FFLAGS=\"-g -O2\"\nCGO_LDFLAGS=\"-g -O2\"\nPKG_CONFIG=\"pkg-config\"\nGOGCCFLAGS=\"-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/qs/wpmg19r12_9_nz7pvvs2_82r0000gn/T/go-build774541309=/tmp/go-build -gno-record-gcc-switches -fno-common\"\n"
	nil
	nil
2020/04/27 22:08:12 Info:2020/04/27 22:08:12
	message="go/packages.Load"
	nil
	nil
	snapshot=0
	directory=/Users/alex/Projects/Sanity/gradient
	query=[./... builtin]
	packages=331
@atombender
Copy link
Author

@atombender atombender commented Apr 28, 2020

...Yup. I can reproduce it consistently in a small repo with just two files.

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 28, 2020

Ah ok, glad you were able to figure this out! So this is really working as intended - gopls currently has minimal support for build tags (#29202).

We intend to improve it in future, but for now the best way to work with build tagged files is by setting GOFLAGS, as you're doing. However, this means that gopls will not "see" files that are excluded, so that's what was happening here. In the meantime, if all of your integration tests are in one directory, you may able to add the directory as a new workspace folder in your project and apply workspace-specific settings to it in VS Code.

@atombender
Copy link
Author

@atombender atombender commented Apr 28, 2020

I'm not sure I agree with "working as intended". These files don't contain any tags!

I may be misunderstanding how Go works here, but -tags shouldn't exclude anything that has no build tags; they're used to select which files are included in a build, and a file without tags is always included. None of the files I'm having problems with contain build tags. In other words, in the absence of any // +build comments, I expect go build -tags nope to work exactly as if I ran go build.

Secondly, I understand, and I was aware, that gopls doesn't have a good solution for working with sets of _conflicting build tags. For example, if I have two files foo_one.go (containing // build +one) and foo_two.go (with // build +two), both containing the same names, then I understand it's a tough ask for gopls to handle this correctly. However, I expect gopls to work correctly when there are no conflicting tags, because the output should be the same as though there were no tags at all.

For example, we have the tag integration for integration tests. This is to prevent go test ./... from including them by default, because they require a specific running environment. But the build tag is used only for exclusion, not to provide multiple versions of the same files like you would do with, say, multiple OS architectures.

Lastly, I think you ought to emit an prominent error that shows up in VSCode informing the user that gopls is completely disabled for files containing build tags. Otherwise people are going to (continue to) waste time chasing nebulous bugs caused by the absence of support.

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 28, 2020

Sorry, I should've been more clear in my response. You are correct in your understanding and right to say that this should work. I tested setting GOFLAGS in my own environment, and it worked fine.

My intention was really to convey that we are aware that gopls doesn't work well with build tags, and we expect there to be rough edges, so I wasn't surprised by this. I'm afraid that without a concrete repro case, we won't be able to invest time in figuring out what exactly is going wrong here. @heschik may have some ideas, but this bug seems complicated, and if unsetting GOFLAGS provides a fix, I think this will have to be sufficient until #29202 is resolved.

@atombender
Copy link
Author

@atombender atombender commented Apr 28, 2020

Here is an isolated repo that I believe reproduces the problem; running gopls imports with GOFLAGS fails to add the missing import. You can view the run output here.

@heschik
Copy link
Contributor

@heschik heschik commented Apr 28, 2020

Thanks for the repro. Somehow -tags=dummy is turning into just -tags and breaking all of our go invocations. I'll track it down.

@heschik
Copy link
Contributor

@heschik heschik commented Apr 28, 2020

Got it. Fix as soon as I write the test.

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 28, 2020

Change https://golang.org/cl/230560 mentions this issue: internal/lsp/cache: correctly split env vars

@atombender
Copy link
Author

@atombender atombender commented Apr 28, 2020

Thanks, but closing is maybe premature? My repro repo still fails, and so does gopls when I run it with VSCode.

@heschik
Copy link
Contributor

@heschik heschik commented Apr 28, 2020

I tested the fix with your repro, so that's pretty surprising. Are you sure you have master? (Maybe GOPROXY=direct to avoid proxy.golang.org caching it?)

@atombender
Copy link
Author

@atombender atombender commented Apr 28, 2020

I keep forgetting there's a module cache now. GOPROXY=direct did the trick, thanks.

By the way, I tried without the =, and it looks like gopls doesn't handle this correctly:

$ GOFLAGS='-tags dummy' gopls -rpc.trace -v imports -d ./main.go
panic: err: exit status 1: stderr: go: parsing $GOFLAGS: non-flag "dummy"


goroutine 1 [running]:
golang.org/x/tools/internal/lsp/debug.PrintVersionInfo.func2()
	/go/pkg/mod/golang.org/x/tools@v0.0.0-20200428140416-006b16f6cf7f/internal/lsp/debug/info.go:58 +0x231
golang.org/x/tools/internal/lsp/debug.section(0xe6b8a0, 0xc000098cf0, 0x0, 0xd0f5da, 0x7, 0xc000260920)
	/go/pkg/mod/golang.org/x/tools@v0.0.0-20200428140416-006b16f6cf7f/internal/lsp/debug/info.go:69 +0x319
golang.org/x/tools/internal/lsp/debug.PrintVersionInfo(0xe7fd80, 0xc00032a630, 0xe6b8a0, 0xc000098cf0, 0x1, 0x0)
	/go/pkg/mod/golang.org/x/tools@v0.0.0-20200428140416-006b16f6cf7f/internal/lsp/debug/info.go:52 +0x1a7
golang.org/x/tools/internal/lsp.(*Server).initialize(0xc0001e3200, 0xe7fd80, 0xc00032a630, 0xc000336000, 0x0, 0x0, 0x0)
	/go/pkg/mod/golang.org/x/tools@v0.0.0-20200428140416-006b16f6cf7f/internal/lsp/general.go:78 +0x4c8
golang.org/x/tools/internal/lsp.(*Server).Initialize(0xc0001e3200, 0xe7fd80, 0xc00032a630, 0xc000336000, 0xc0000bc1a8, 0xc00032a660, 0xc00032a7e0)
	/go/pkg/mod/golang.org/x/tools@v0.0.0-20200428140416-006b16f6cf7f/internal/lsp/server_gen.go:108 +0x49
golang.org/x/tools/internal/lsp/cmd.(*connection).initialize(0xc0003340a0, 0xe7fd80, 0xc00032a630, 0xd711e8, 0xd03a00, 0xc000315a80)
	/go/pkg/mod/golang.org/x/tools@v0.0.0-20200428140416-006b16f6cf7f/internal/lsp/cmd/cmd.go:267 +0x2b2
golang.org/x/tools/internal/lsp/cmd.(*Application).connect(0xc0002368c0, 0xe7fd80, 0xc00032a330, 0x0, 0x0, 0x0)
	/go/pkg/mod/golang.org/x/tools@v0.0.0-20200428140416-006b16f6cf7f/internal/lsp/cmd/cmd.go:202 +0x7f8
golang.org/x/tools/internal/lsp/cmd.(*imports).Run(0xc0002c5470, 0xe7fd80, 0xc00032a330, 0xc000032230, 0x1, 0x1, 0x0, 0x0)
	/go/pkg/mod/golang.org/x/tools@v0.0.0-20200428140416-006b16f6cf7f/internal/lsp/cmd/imports.go:51 +0x7e
golang.org/x/tools/internal/tool.Run(0xe7fd80, 0xc00032a330, 0xe83b40, 0xc0002c5470, 0xc000032220, 0x2, 0x2, 0x0, 0x0)
	/go/pkg/mod/golang.org/x/tools@v0.0.0-20200428140416-006b16f6cf7f/internal/tool/tool.go:152 +0x29d
golang.org/x/tools/internal/lsp/cmd.(*Application).Run(0xc0002368c0, 0xe7fd80, 0xc00032a330, 0xc000032220, 0x3, 0x3, 0x0, 0x0)
	/go/pkg/mod/golang.org/x/tools@v0.0.0-20200428140416-006b16f6cf7f/internal/lsp/cmd/cmd.go:146 +0x2cc
golang.org/x/tools/internal/tool.Run(0xe7fd00, 0xc000038060, 0xe83900, 0xc0002368c0, 0xc0000321f0, 0x5, 0x5, 0x0, 0x0)
	/go/pkg/mod/golang.org/x/tools@v0.0.0-20200428140416-006b16f6cf7f/internal/tool/tool.go:152 +0x29d
golang.org/x/tools/internal/tool.Main(0xe7fd00, 0xc000038060, 0xe83900, 0xc0002368c0, 0xc0000321f0, 0x5, 0x5)
	/go/pkg/mod/golang.org/x/tools@v0.0.0-20200428140416-006b16f6cf7f/internal/tool/tool.go:91 +0x12f
main.main()
	/go/pkg/mod/golang.org/x/tools/gopls@v0.0.0-20200428140416-006b16f6cf7f/main.go:25 +0xdb

Interestingly, go help build doesn't even say that you can use =.

@heschik
Copy link
Contributor

@heschik heschik commented Apr 28, 2020

Yeah, it's strange but not gopls:

$ GOFLAGS='-tags foo' go mod tidy
go: parsing $GOFLAGS: non-flag "foo"

Feel free to file a cmd/go bug for that, though I feel like I've seen it somewhere.

@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v0.4.1 May 13, 2020
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.