-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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 errors with multi-root workspace and gopls@v0.15.0+ #66145
Comments
Noticed the same bug in my multiroot workspace setup. Manually reverting EDIT: a bit more info:
|
Hi @andig, a few questions:
|
Also: needless to say, but if any of this is in an open source repo (or set of repos), a reproducer would be the fastest way to investigate. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Multi root for me, will add more diag when back at machine |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
It turns out that @danielpcox and @itsaerie both work at Google, and we have a Google-internal go/packages driver. The presence of this driver caused the "zero config" logic of gopls@v0.15.0 and later to throw up its hands: if the user has a go/packages driver we can't make any assumptions about how packages are laid out... Of course, this regression for Googlers was unintended. For now, the workaround is to explicitly set GOPACKAGESDRIVER=off in the environment of the gopls process, by setting {
"go.toolsEnvVars": {
"GOPACKAGESDRIVER": "off"
}
} I will fix this in a subsequent release (#66041). Marking comments related to GOPACKAGESDRIVER as resolved, as I think they represent a separate issue. All other comments relate to multi-root workspaces. |
In my case all roots are Go projects and have a go.mod (I have removed the two that were not, problem remains). There is not problem when closing the workspace and opening a single project folder. |
Thanks all for following up. What this sounds like is that one of the workspace folders is producing erroneous diagnostics for another, and gopls is merging the diagnostics. This logic definitely changed in the most recent gopls release: previously, gopls would only show diagnostics for one workspace folder at a time, whichever changed last. However, being aware of the potential for this type of error, I thought that I had guarded against this type of error (in short, it is important that folder A never tries to load a file from folder B). Clearly, there is a bug, but my naive attempts to reproduce have failed: when I add multiple workspace folders, things work as expected. To confirm my hypothesis, I think it would suffice to check that core operations work in the file with import errors, despite the errors: do things like jump to definition or hover work on a symbol that is imported from the import that is supposedly broken? Other questions:
The answers to these questions will give me an idea of where to head next. If folks here are willing to go back and forth a bit, I can guarantee that we will fix this expediently. Absent a reproducer, the fastest way to diagnose this will be to run a patched version of gopls with additional debugging output. Unfortunately, today is a Google holiday, so I won't be able to put together such a patch until Monday. In the meantime, please downgrade to v0.14.2, or work in a single workspace folder. Sorry for the breakage. |
yes 1/2/3: no |
Thank for those responses. @ErwanDL do you also have a local replace directive? |
Sorry for not answering earlier, I don't use Github at work so I need to come check in manually more often.
Yes, they do work.
Yes, in my case I have 5 roots to my multi-root workspace, and 4 Go projects nested in there. This is the project structure:
No.
No.
Yes, we have several replace directives.
and
In the meantime I have downgraded |
Thanks all, I finally understand the bug, have a repro and should be able to fix. Transferring to the Go issue tracker as this is a gopls bug. |
Change https://go.dev/cl/569836 mentions this issue: |
Change https://go.dev/cl/569876 mentions this issue: |
…stics in multi-root workspaces In golang/go#66145, users reported spurious import errors in multi-root workspaces. The problem was related to scenarios where module A had a local replace of module B, and the user opened a file F in module B that wasn't in the forward dependencies of module A. In this case, if the View of module A tried to load F, it would get real packages (not command-line-arguments), but due to module graph pruning the View of module A would not have access to the full set of dependencies for module B, resulting in the potential for import errors. Even this would not be a problem, as long as the package that module A loaded for F was not considered a 'workspace' package. Unfortunately a couple of incorrect heuristics in gopls added along with the zero-config work of gopls@v0.15.0 allowed us to diagnose these broken packages: 1. In resolveImportGraph, we called MetadataForFile for each overlay. As a result, the import graph optimization caused gopls to attempt loading packages for each open file, for each View. It was wrong for an optimization to have this side effect. 2. In golang/go#65801, we observed that it was inconsistent to diagnose changed packages independent of whether they're workspace packages. To fix that, I made all open packages workspace packages. It was probably wrong for the set of workspace packages to depend on open files. To summarize a rather long philosophical digression: open files should determine Views, not packages. Fixing either one of these incorrect heuristics would have prevented golang/go#66145. In this CL, we fix (2) by building the import graph based on existing metadata, without triggering an additional load. For (1), we check IsWorkspacePackage in diagnoseChangedFiles to enforce consistency in the set of diagnosed packages. It would be nice to also remove the heuristic that "all open packages are workspace packages", but we can't do that yet as it would mean no diagnostics for files outside the workspace, after e.g. jumping to definition. A TODO is left to address this another day, when we can be less conservative. Fixes golang/go#66145 Change-Id: Ic4cf2bbbb515b6ea0df24b8e6e46c725b82b4779 Reviewed-on: https://go-review.googlesource.com/c/tools/+/569836 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alan Donovan <adonovan@google.com> (cherry picked from commit 93c0ca5) Reviewed-on: https://go-review.googlesource.com/c/tools/+/569876 Auto-Submit: Robert Findley <rfindley@google.com>
|
Just did and added the offending root. No more errors- LGTM! Much appreciated! |
Indeed, looks like it works in |
Great, thanks for confirming. gopls@v0.15.2 will be released soon. |
I still seem to get this error :/
My setup looks like this.
If I open my application via the |
I am also still getting this error |
What version of Go, VS Code & VS Code Go extension are you using?
Version Information
go version
to get version of Go from the VS Code integrated terminal.go version go1.22.0 darwin/arm64
gopls -v version
to get version of Gopls from the VS Code integrated terminal.code -v
orcode-insiders -v
to get version of VS Code or VS Code Insiders.Go: Locate Configured Go Tools
command.Share the Go related settings you have added/edited
Run
Preferences: Open Settings (JSON)
command to open your settings.json file.Share all the settings with the
go.
or["go"]
orgopls
prefixes.Describe the bug
For some days, gopls has started complaining about missing imports.
go.mod
andgo.sum
are clean and project compiles.Extension host shows errors:
Screenshots or recordings
If applicable, add screenshots or recordings to help explain your problem.
The text was updated successfully, but these errors were encountered: