-
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 when using go.work replace directives #66425
Comments
Possibly related to golang/vscode-go#3258? |
Hmm, while I believe that there may be a bug here (this logic changed with gopls@v0.15.0+), I'm not yet able to reproduce. Specifically, I checked out https://go.googlesource.com/tools as well as https://go.googlesource.com/mod, and replaced golang.org/x/mod in the tools/go.work
Everything worked as expected, and jumping to the definition of something imported from x/mod goes to the correct directory. So, it seems there is something specific about your modules that exacerbate this bug -- it doesn't reproduce with an arbitrary go.work replace directive. One question, which may seem counterintuitive: do things like jump-to-definition work in these buffers, even though they have import errors? In other words, can you jump to definitions in nex-go? If so, do they go to the locally replaced module? |
Since it looks like you're working in open source, would it be possible to create two branches for your ongoing work? (one for the nex-protocols-go repo and another for the nex-go repo)? A personal fork should be fine. Then I can try to reproduce in your specific workspace. |
When using Additionally it seems like even some local imports, not in the replaced module, also do not work. Example:
This is not surprising to be honest. I forgot to mention it in my original post, but this is inconsistent even in my setup. Here is an example from the same module where these errors do not occur (though you can see in the same screenshot, other files do have these errors despite identical imports and usage)
Both of these repositories already have public branches with the latest changes. They can be found here:
Not mentioned here, as I assumed it was unnecessary, but there is also a 3rd repository with this same setup having the same issues. I did not mention it, as I felt only the 1 was enough to demonstrate the issue. But that 3rd repository can be found here https://github.com/PretendoNetwork/nex-protocols-common-go/tree/nex-go-rewrite It's also worth noting that this morning I started a new VS Code workspace from scratch, with ONLY those 2 repositories (the one shown here has many dozens of projects in it, some of which are confidential which is why they were not shown here), and in that new fresh workspace this error does not seem to happen. |
Thanks, this looks like a variant of #66145, which was fixed in gopls@v0.15.2, albeit in a rather tricky and defensive way. There may be another way to exercise the bug. Just to be clear: you are seeing this even with gopls@v0.15.2, right? If not, then the bug may have already been fixed. |
Yes. The output from
|
Understood, and I saw that, but given the timing of the release wondered if perhaps the screenshots were taken at a different time. Another question: do these errors persist even after running Also, I'm curious why you are using a go.work replace directive rather than adding the nex-go-rewrite directory to your workspace. Of course gopls should not be broken, but I wonder if you see the same errors when using, rather than replacing, nex-go-rewrite. |
Yet more questions: among the many other proprietary folders in your workspace do any have a local replace of nex-go or nex-go-protocol, or do they have a go.work file that uses either? Are any of your folders in a GOPATH directory? Sorry for the barrage of questions, but the nature of the bug is as follows: the diagnostics you see are from one of your other workspace folders, complaining that it can't load the import for the package in nex-go-protocol. The problem is that this folder shouldn't care about that package, and therefore shouldn't produce diagnostics for it. This is an unfortunately complicated problem, because with GOPATH directories, go.work files, and local replaces in both go.work and go.mod files, there are many ways for a file to be included in a build. In order to produce accurate and yet complete diagnostics, gopls must have accurate heuristics for when a package should be loadable from a workspace folder. |
Understood. To be clear then, all information and screenshots are from the time of writing unless otherwise specified
Yes. This persists between window reloads, between closing and reopening VS Code entirely, and I've even tried rebooting my whole machine. The errors are both persistent and consistent where they appear (it doesn't affect random files each time, it affects the same ones each time despite the errors themselves being inconsistent)
Though I don't believe that's really relevant. I have not tried directly importing the replaced module, mostly because this issue seems to not just be limited to that module. As stated in my first reply from this morning, this also affects locally imported packages from the same module (I was not aware of this until this morning). Here is the screenshot from earlier: Also when starting a new VS Code workspace from scratch these errors seem to not occur. I was planning to remake the workspace from scratch, seeing where it might break at, but have not found the time to do so yet.
Yes, many do have replaces for those 2, and many do not. Due to the aforementioned rewrite, many of these are currently broken for other reasons though so getting finer details on them would take some time. I did some touch ups on one just now for this question, and it looks like it's importing correctly. Here are 2 screenshots showing the rewrite directive being properly applied in this case (ignore all the other file errors, they are unrelated):
It's no trouble, really. Ask as many as you need, I'll answer them as best I can (though I am no expert on these things). Whatever helps get things working smoothly again, as this setup worked for several years before now |
Thanks. Let me think about the fastest way to figure this out. In the meantime, you should be able to get back to work by downgrading to v0.14.2:
Sorry about the breakage. |
No worries, thank you for taking the time to look into it. Just to confirm by the way, downgrading to 0.14.2 did indeed fix the issue 👍 |
replace
directive in go.work
seems to be ignored
This appears to be a gopls issue, so I transferred to the Go issue tracker. One possibility is that, among your workspace folders, there is a folder where these import errors are actually correct, i.e. there's a workspace that legitimately can't load this module. In gopls@v0.14.2, we didn't merge diagnostics from multiple workspaces, so it would have been easy to miss such errors. Since we don't have a simple repro, and your standard workspace setup sounds quite complicated, I think the thing to do is prepare a CL that more clearly identifies which folders are producing a diagnostic. Then we can perhaps narrow down the reproducer. I'll do this, but it might take a week or so as I'm traveling. I suspect that this issue only affects a very small number of users: only a small fraction of users have multi-root workspaces, and among them only another fraction use multiple go.work files, and among them only a fraction use go.work replace directives (these are quite rare). CC @adonovan for awareness. Alan: I don't think any action is required on your part unless this appears to be affecting a larger number of users, since @jonbarrow is unblocked by downgrading to gopls@v0.14.2. I want to get to the bottom of this, but it requires some work on my end to help diagnose. |
Change https://go.dev/cl/574718 mentions this issue: |
@jonbarrow, I wonder if https://go.dev/cl/574718 improves the situation for you. Could you test? Here's how to install that CL:
Does that version of gopls still have the spurious import errors? |
So far so good! 👍 |
Thanks for testing. I think this change is safe to make, so I've tentatively marked this as fixed. It would be good to understand exactly how the spurious diagnostics are being produced, but I think it's also safe to simply filter diagnostics to the most relevant builds, as I've done. |
Change https://go.dev/cl/577261 mentions this issue: |
… to "best" views Filter diagnostics only to the "best" view for a file. This reduces the likelihood that we show spurious import diagnostics due to module graph pruning, as reported by golang/go#66425. Absent a reproducer this is hard to test, yet the change makes intuitive sense (arguably): it is confusing if diagnostics are inconsistent with other operations like jump-to-definition that find the "best" view. Fixes golang/go#66425 Updates golang/go#66730 Change-Id: Iadb1a01518a30cc3dad2d412b1ded612ab35d6cc Reviewed-on: https://go-review.googlesource.com/c/tools/+/574718 Reviewed-by: Alan Donovan <adonovan@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> (cherry picked from commit f345449) Reviewed-on: https://go-review.googlesource.com/c/tools/+/577261 Auto-Submit: Robert Findley <rfindley@google.com>
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.21.3 linux/amd64
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.Though this happens on all tried versions, from 1.80 to 1.87
v0.41.2
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."go.toolsManagement.autoUpdate": true
Describe the bug
replace
directive seems to be ignored ingo.work
file. Failing to load the correct local package. Only occurs in VS Code,go vet
reports no issues.This behavior was working up until recently. I do not have an exact time/version this stopped working, as it has been a few days since I touched these modules.
Steps to reproduce the behavior:
go.work
in ModuleAreplace
directive in thego.work
to point to ModuleBScreenshots or recordings
go.work
contents:Broken import/wrong package being referenced leading to many errors:
The local package does exist:
go vet
command showing no errors (issue is only within VS Code):The text was updated successfully, but these errors were encountered: