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

Jump to bottom

go/packages: switch to native overlays for 1.16 #41598

Closed
stamblerre opened this issue Sep 24, 2020 · 5 comments
Closed

go/packages: switch to native overlays for 1.16 #41598

stamblerre opened this issue Sep 24, 2020 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
gopls/v0.5.3

Comments

@stamblerre
Copy link
Contributor

stamblerre commented Sep 24, 2020 •
edited

#39958 will be part of the 1.16 release. As soon as https://golang.org/cl/253747 is merged, we can start adding a flag to use native Go command overlays in go/packages and begin testing with gopls.

/cc @matloob @heschik @findleyr
@stamblerre stamblerre added this to the gopls/v1.0.0 milestone Sep 24, 2020
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 24, 2020
This was referenced Sep 25, 2020
Closed
Closed
@gopherbot
Copy link

Change https://golang.org/cl/263985 mentions this issue: internal/gocommand: add a GoVersion function to avoid duplicating logic

@stamblerre
Copy link
Contributor Author

It's looking like we will need to get the Go version for every call to packages.Load.

@heschik: Would it be reasonable to cache the go version in the go command runner, and consequently, enforce a requirement that a given runner can only be used with a single go binary?

@stamblerre stamblerre assigned stamblerre and unassigned pjweinb Oct 21, 2020
@heschi
Copy link
Contributor

heschi commented Oct 21, 2020

There is currently no clear rule about what the lifetime/scope of a Runner is, and it's currently one per gopls session, so that would definitely not be correct given differing environments.

We already do multiple go command invocations per Load call (size loading, in particular) and I don't think this is the time to try to reduce them. That said, I think the approach I took in imports.(*ProcessEnv).init is a reasonable template, but the Go version and size information are somewhat different since they are not actually directly user-overridable.

gopherbot pushed a commit to golang/tools that referenced this issue Oct 21, 2020
@stamblerre
internal/gocommand: add a GoVersion function to avoid duplicating logic
6652d1f 
It seems easier to have this as a shared internal function.

Updates golang/go#41598

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

Ok, thanks--I wasn't sure if it was OK that we would be adding another go list call to go/packages. I will file a separate issue as a reminder to look into this.

@gopherbot
Copy link

Change https://golang.org/cl/263984 mentions this issue: go/packages: use native overlay support for 1.16

@stamblerre stamblerre mentioned this issue Oct 23, 2020
Closed
@golang golang locked and limited conversation to collaborators Oct 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
gopls/v0.5.3
Development

No branches or pull requests
5 participants
@cagedmantis @pjweinb @stamblerre @gopherbot @heschi