x/tools/gopls: GOPACKAGESDRIVER calls happen at intervals that make the results incorrect

[JamyDev]

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

$ go version
1.20.3

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env

```
GO111MODULE="off"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOEXPERIMENT="nocoverageredesign,nounified"
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/user/go-code/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user/go-code"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/user/.cache/bazel/_bazel_jamy/b97476d719d716accead0f2d5b93104f/external/go_sdk_linux_amd64"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/user/.cache/bazel/_bazel_jamy/b97476d719d716accead0f2d5b93104f/external/go_sdk_linux_amd64/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20.3 X:nounified,nocoverageredesign"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2135155060=/tmp/go-build -gno-record-gcc-switches"
```

What did you do?

With GOPACKAGESDRIVER enabled, adding an import (whether by inline writing code and accepting an import suggestion; or manually adding it to the imports section) will trigger a query to the packages driver. However, this trigger will happen prior to the file being saved, yet the packagesdriver will only get a filename to load packages for.

This triggers a bit of a race condition in build systems like Bazel:

1. Current imports in file.go are a, b, c
2. I add import d
3. GoPLS immediately tries to look up package info from the driver using the filename ($ pkgdrvr file=file.go)
4. Since the file isn't saved at this point in time, the packagesdriver will only see imports a, b, c and thus only return info for those 3
5. GoPLS gets incomplete results back and flags import d as bad.
6. Even after save, there is no re-trigger of the packagesdriver, which means this bad imports stays bad until after the imports are changed again; or the language server is restarted.

What did you expect to see?

Since the packagesdriver works with files on disk (in the general case) we should not query the packagesdriver until the IDE has committed the changes on disk.

Alternatively we can extend the packages driver call to query directly for the information of the imported package, rather than indirectly querying for the file the import is added to.

What did you see instead?

Package import resolution fails until language server restart.

I will try to get a project where we can repro this situation in.

[hyangah]

Can you share which gopackagesdriver you are using. The default gopackage driver go list handles this case by using overlay. https://pkg.go.dev/golang.org/x/tools/go/packages#Config

Is it possible to make your gopackagesdriver recognize the overlay info?

@golang/tools-team I think we need documentation on gopackagesdriver https://github.com/golang/go/issues/34341

[JamyDev]

We use the rules_go one. I'll have a look at overlay info and see how to support it in the rules_go driver

[findleyr]

We discussed this a bit more in our team meeting, and decided it would probably be very difficult to support this in the bazel driver (based on our understanding of bazel).

So we'll leave this open: it would be nice to fix, but unlikely to get prioritized any time soon. The work to implement this would be approximately:

• accumulate invalidations in cache.snapshot.clone while the user types.
• only invalidate and reload packages when the user saves all files

...but this will be very tricky to implement, and even after implemented may have to a subtly confusing UX.

[li-jin-gou]

@findleyr Hello, What is the status of this support programme? We had a similar problem when we implemented a packagedriver ourselves and he would dynamically generate code to return package information. but because of caching issues, there is no way to get the latest generated code. Is there a recommended solution? Apart from rebooting

[findleyr]

@li-jin-gou I think this issue may have been superseded by #68002. I plan to implement #68002 this year. It is currently targetting the v0.17.0 release, which is tentatively planned for October.

[ViolaPioggia]

@findleyr Hello, What is the status of this support programme? We had a similar problem when we implemented a packagedriver ourselves and he would dynamically generate code to return package information. but because of caching issues, there is no way to get the latest generated code. Is there a recommended solution? Apart from rebooting

I have a similar problem to him. After my local dependencies using gopackagesdriver are changed, I hope gopls can get the latest data in time. Is there any gopls command that can achieve this? Or is there any other better way?

[li-jin-gou]

Thanks, until then we can move forward with the programme of trying to restart gopls. @findleyr

[JamyDev]

FWIW at Uber we did 2 things to fix this:

1. Support overlays on our internal driver. The overlay content contains the newly added import which fixes the initial issue (assuming you have some way of resolving said import without your dependency manager).
2. Added a "reload Imports" codelens above the imports section. Clicking this invalidates the target in our cache, and then slightly modifies the imports (adds a space at the front of the first import and removes it right after). The imports modification triggers gopls to request the imports from the packages driver. @li-jin-gou @ViolaPioggia

[li-jin-gou]

FWIW at Uber we did 2 things to fix this:

1. Support overlays on our internal driver. The overlay content contains the newly added import which fixes the initial issue (assuming you have some way of resolving said import without your dependency manager).
2. Added a "reload Imports" codelens above the imports section. Clicking this invalidates the target in our cache, and then slightly modifies the imports (adds a space at the front of the first import and removes it right after). The imports modification triggers gopls to request the imports from the packages driver.

thanks

[Skyenought]

FWIW at Uber we did 2 things to fix this:

1. Support overlays on our internal driver. The overlay content contains the newly added import which fixes the initial issue (assuming you have some way of resolving said import without your dependency manager).
2. Added a "reload Imports" codelens above the imports section. Clicking this invalidates the target in our cache, and then slightly modifies the imports (adds a space at the front of the first import and removes it right after). The imports modification triggers gopls to request the imports from the packages driver. @li-jin-gou @ViolaPioggia

sorry sir. In our case, the content of *packages.DriverResponse is unchanged, but the content of the file it maps to has changed. So the IDE is not aware of the fact that the prompts are no longer correct due to a change in the file

  {
    "ID": "service1/hello",
    "Name": "hello",
    "PkgPath": "rgo/service1/kitex_gen/hello",
    "GoFiles": [
      "/root/cache/service1/hello/hello.go"
    ],
    "CompiledGoFiles": [
      "/root/cache/service1/hello/hello.go"
    ],
    "Imports": {
      "context": "context",
      "fmt": "fmt"
    }
  },

In fact it really doesn't describe whether or not the specifics of the file change. Have you ever had this problem? @JamyDev

[li-jin-gou]

Dependencies may have been added or the original dependency code may have changed.