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: fix import organization in a multi-module workspace #41836

Closed
stamblerre opened this issue Oct 7, 2020 · 10 comments
Closed

x/tools/gopls: fix import organization in a multi-module workspace #41836

stamblerre opened this issue Oct 7, 2020 · 10 comments

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Oct 7, 2020

Tracking issue for the work to fully support goimports in a multi-module workspace.
This will require tracking all go.mods in a workspace in the ProcessEnv, and possibly writing the workspace module out to disk.

@heschik is working on a plan for this.

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 20, 2020

Change https://golang.org/cl/263937 mentions this issue: internal/memoize: add BindCloser, for binding values that must be closed

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 20, 2020

Change https://golang.org/cl/263938 mentions this issue: internal/lsp/cache: keep a cached workspace module dir

gopherbot pushed a commit to golang/tools that referenced this issue Oct 30, 2020
With its memoization and refcounting, the cache is well suited to the
sharing of other expensive resources, specifically those that interact
with the file system. However, it provides no means to clean up those
resources when they are no longer needed.

Add an additional argument to Bind to clean up any values produced by
the bound function when they are no longer referenced.

For golang/go#41836

Change-Id: Icb2b12949de06f2ec7daf868f12a9c699540fa5b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263937
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Robert Findley <rfindley@google.com>
gopherbot pushed a commit to golang/tools that referenced this issue Oct 30, 2020
Keep a workspace module dir around for running the go command against a
snapshot, bound to the contents of the workspace modfile.

This uses the cache's resource model to share the workspace module dir
across snapshots if it is not invalidated, and to delete it when it is
no longer in-use by a snapshot. Of course, the go command will still
only see files on the filesystem, but using this immutable model was
most consistent with the immutable workspace.

For golang/go#41836

Change-Id: Iaec544283b2f545071e5cab1d0ff2a66e6d24dff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/263938
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Nov 11, 2020

@findleyr: Can this be closed?

@findleyr
Copy link
Contributor

@findleyr findleyr commented Nov 11, 2020

@findleyr: Can this be closed?

No, I just added a handle for the snapshot workspace directory. The work of hooking up imports has not been done.

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Nov 11, 2020

Ok, thanks for clarifying. Are you planning to work on that or should @heschik do that final step?

@findleyr
Copy link
Contributor

@findleyr findleyr commented Nov 11, 2020

I thought Heschi was going to do it, but I might be misremembering. Happy to do it if needed.

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Nov 11, 2020

Ok sounds good--I'll leave it assigned to Heschi, just wasn't sure if you'd spoken offline.

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 17, 2020

Change https://golang.org/cl/270877 mentions this issue: internal/lsp: support unimported completions in multi-module mode

@oldCoderException
Copy link

@oldCoderException oldCoderException commented Mar 8, 2021

Hey there... I just thought I'd add that the lack of this feature is affecting me as well. Please see my "experience report" on the golang group at: https://groups.google.com/g/golang-nuts/c/2Xcfb4f7ans
Requiring that any module be at the root in VSCode has had a somewhat significant impact on my development workflow. I look forward to this being resolved. :) thanks guys!

@oldCoderException
Copy link

@oldCoderException oldCoderException commented Mar 8, 2021

I may have commented on the wrong issue? Should it have been #32394? Sorry, it's a bit confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants