gopls caching error when updating go version

[nilskch]

Check for existing issues

• Completed

Describe the bug / provide steps to reproduce it

First of all, thank you very much for this awesome IDE! I am just getting started using Zed but I already love it!

As discussed with @mrnugget in #7902, I opened this Issue.

I installed the new go version (go1.22.0), and I ran go install golang.org/x/tools/gopls@latest to build gopls with the newest go version on my system. In VSCode, everything works and gopls supports the new language features, but not Zed. Zed installed gopls when I had an older version of go installed and cached it in ~/Library/Application\ Support/Zed/languages/gopls/gopls_0.14.2.

0.14.2 is the newest version of gopls, but it was not installed using the newest version of go and, therefore, does not support the newest language features (in my case, it was iterating over integers for i := range 10). If you install a new version of go, you must run go install golang.org/x/tools/gopls@latest again using the new version, which Zed didn't.

Deleting ~/Library/Application\ Support/Zed/languages/gopls/gopls_0.14.2 forces Zed to reinstall gopls using the new version of go and everything works.

One solution would be to cache gopls differently: Instead of naming the cached file gopls_{GOPLS_VERSION} (see here or here), we could cache it like this: gopls_{GOPLS_VERSION}_{GO_VERSION}. By doing that, we also cache with which go version gopls was built. Changing/Updating the go version then triggers a new gopls installation.

Checking if a globally available version of gopls is available would also be a nice idea and is somewhat related to this issue (See #7902 for more details).

Environment

Zed: v0.122.2 (Zed)
OS: macOS 14.2.1
Memory: 18 GiB
Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

If you only need the most recent lines, you can run the zed: open log command palette action to see the last 1000.

No response

[mrnugget]

One solution would be to cache gopls differently: Instead of naming the cached file gopls_{GOPLS_VERSION} (see here or here), we could cache it like this: gopls_{GOPLS_VERSION}_{GO_VERSION}. By doing that, we also cache with which go version gopls was built. Changing/Updating the go version then triggers a new gopls installation.

I like that! I think that's also orthogonal to the rest of what's discussed in #7902.

Just to challenge the idea though: what are the downsides of that? For example, yesterday we found out that vscode-go checks whether the Go version that was used to compile gopls is the same as the go version and if not, it offers to reinstall gopls. If you accept that, it actually overwrites the existing gopls. I'm not sure whether I like that, but the upside of that is that you don't collect old gopls versions over time.

[nilskch]

I'm not sure whether I like that, but the upside of that is that you don't collect old gopls versions over time.

Yes, this is a good point! However, I think we already have this problem every time a new version of gopls gets released.

Every time Zed does not find the correct binary in ~/Library/Application\ Support/Zed/languages/gopls/, we could clean up every old version first (removing every file in ~/Library/Application\ Support/Zed/languages/gopls/) and install the new gopls version afterwards.

This does not interfere with #7902 and would solve the bug.

If you are happy with the proposed solution, I would love to implement it myself and create a PR?

[mrnugget]

If you are happy with the proposed solution, I would love to implement it myself and create a PR?

I am happy with the proposed solution, BUT we're also right now heavily discussing how we want to proceed with language extensions and PATH handling and all that, which would influence the work here. So if it's fine with you, how about we hold off for a bit until we have more concrete plans?

(btw. Grüße aus Nähe Aschaffenburg nach Darmstadt! Sorry, couldn't resist!)

[nilskch]

So if it's fine with you, how about we hold off for a bit until we have more concrete plans?

Yes of course! Sounds good to me.

(Haha Grüße zurück!)

[mrnugget]

Just to record this here, because I just ran into the code: we do cleanup other binaries in our folder

zed/crates/zed/src/languages/go.rs

Lines 121 to 124 in dfcf72b

	remove_matching(&container_dir, |entry| {
	entry != binary_path && entry.file_name() != Some(OsStr::new("gobin"))
	})
	.await;

[nilskch]

This PR fixed the issue. Thanks @mrnugget!

[mrnugget]

Glad to hear, @nilskch!

[zacharysyoung]

I just got to this issue because I recently installed Go 1.23 and just tried to use an iterator for the first time. Zed (v0.160.7) and gopls (v0.15.3) complain that I cannot range over an iterator function.

gopls is out of date, v0.16.2 is current.

I ended up doing go install golang.org/x/tools/gopls@latest and restarting Zed and all is good.

I did just check my Zed/languages/gopls directory:

ll ~/Library/Application\ Support/Zed/languages/gopls/
total 179584
drwxr-xr-x@  6 zyoung  staff       192 Jul  2 16:38 .
drwxr-xr-x@ 13 zyoung  staff       416 Jul  6 22:45 ..
drwxr-xr-x@  2 zyoung  staff        64 Jul  2 16:38 gobin
-rwxr-xr-x@  1 zyoung  staff  30429704 Jan 30  2024 gopls_0.14.2
-rwxr-xr-x@  1 zyoung  staff  30182610 Mar 17  2024 gopls_0.15.2
-rwxr-xr-x@  1 zyoung  staff  31327650 Jul  2 16:38 gopls_0.16.1

Should Zed have done something special so that I wouldn't see the error? I don't quite follow what this issue and the accompanying PR accomplished.

[nilskch]

Oh yes you are right, #8188 only checks for gopls in PATH but doesn't address the issue with caching.

we're also right now heavily discussing how we want to proceed with language extensions and PATH handling and all that, which would influence the work here. So if it's fine with you, how about we hold off for a bit until we have more concrete plans?

Would you accept a PR with the proposed caching solution now?

[mrnugget]

Wait, I'm not sure I follow — gopls_0.16.1 was downloaded but not used by Zed? Or is the issue that it was built with the wrong Go version?

[nilskch]

it was built with the wrong go version. the go version you build gopls with is crucial.

[nilskch]

Just to record this here, because I just ran into the code: we do cleanup other binaries in our folder

ll ~/Library/Application\ Support/Zed/languages/gopls/
total 179584
drwxr-xr-x@  6 zyoung  staff       192 Jul  2 16:38 .
drwxr-xr-x@ 13 zyoung  staff       416 Jul  6 22:45 ..
drwxr-xr-x@  2 zyoung  staff        64 Jul  2 16:38 gobin
-rwxr-xr-x@  1 zyoung  staff  30429704 Jan 30  2024 gopls_0.14.2
-rwxr-xr-x@  1 zyoung  staff  30182610 Mar 17  2024 gopls_0.15.2
-rwxr-xr-x@  1 zyoung  staff  31327650 Jul  2 16:38 gopls_0.16.1

this looks like something is wrong with the cleanup logic as well :/

[mrnugget]

it was built with the wrong go version. the go version you build gopls with is crucial.

Alright, that makes sense. Yeah, in that case we should probably overhaul our caching logic.

[nilskch]

#20922 implements the proposed solution of caching the go version as well