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: don't access the network for go mod tidy diagnostics #38462

Closed
stamblerre opened this issue Apr 15, 2020 · 6 comments
Closed

x/tools/gopls: don't access the network for go mod tidy diagnostics #38462

stamblerre opened this issue Apr 15, 2020 · 6 comments

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 15, 2020

A few reports of issues with gopls when the user has no internet (one on Slack, one on microsoft/vscode-go#3105).

I wonder if there is a way we can have a regression test this - maybe by configuring something in the environment? @findleyr

@findleyr
Copy link
Contributor

@findleyr findleyr commented Apr 15, 2020

Following https://golang.org/cl/228235, the regtests use a proxy within our control. If we have examples of how gopls breaks without a network connection, we should be able to reproduce.

@mingoal
Copy link

@mingoal mingoal commented Apr 15, 2020

@findleyr
suggest scenario:
in go file, include one package which has no local cache,

  1. without network connection, will gopls hang or not?
  2. with network connection which requires http proxy, will gopls hang or not?
@gopherbot
Copy link

@gopherbot gopherbot commented Nov 30, 2020

Change https://golang.org/cl/274120 mentions this issue: internal/lsp: disable network access for some go commands

gopherbot pushed a commit to golang/tools that referenced this issue Dec 3, 2020
For users with poor network connections, go command invocations that
access the network may hang up for long periods of time. Even for
users with good network connections, accessing proxy.golang.org or
github can take a few seconds, which is a long time to lock up an
editor.

Disable network access via GOPROXY=off when running most go commands.
There are two notable exceptions. First, the initial workspace load is
allowed, though subsequent loads are not. My reasoning is that if the
user is opening a project they've just downloaded, they probably expect
to fetch its dependencies. (Also, it's convenient to keep the regtests
going.) The second is the go mod tidy diagnostics, which I hope to
address in a later change. All the other commands that access the
network are at the user's request.

When the workspace load fails due to a dependency that hasn't been
downloaded, we present a quick fix on the go.mod line to do so. The only
way that happens is if there's a manual modification to the go.mod file,
since all of the other quick fixes will do the download. So I don't
think there's a need to present the fix anywhere else.

Updates golang/go#38462.

Change-Id: I470bc1ba790db7c1afee54d0b28fa0c6fd203fb9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/274120
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@stamblerre stamblerre moved this from Critical to In progress in vscode-go: gopls by default Dec 6, 2020
@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Dec 6, 2020

I believe the only remaining task here is to remove network access for the go mod tidy diagnostics.

@heschik, is there anything else I'm missing?

@heschi
Copy link
Contributor

@heschi heschi commented Dec 8, 2020

That's it as far as I know.

@stamblerre stamblerre changed the title x/tools/gopls: make sure gopls works without network connection x/tools/gopls: don't access the network for go mod tidy diagnostics Dec 8, 2020
@stamblerre stamblerre moved this from In progress to Non-critical in vscode-go: gopls by default Dec 8, 2020
@stamblerre stamblerre removed this from Non-critical in vscode-go: gopls by default Dec 16, 2020
marwan-at-work added a commit to marwan-at-work/tools that referenced this issue Dec 23, 2020
For users with poor network connections, go command invocations that
access the network may hang up for long periods of time. Even for
users with good network connections, accessing proxy.golang.org or
github can take a few seconds, which is a long time to lock up an
editor.

Disable network access via GOPROXY=off when running most go commands.
There are two notable exceptions. First, the initial workspace load is
allowed, though subsequent loads are not. My reasoning is that if the
user is opening a project they've just downloaded, they probably expect
to fetch its dependencies. (Also, it's convenient to keep the regtests
going.) The second is the go mod tidy diagnostics, which I hope to
address in a later change. All the other commands that access the
network are at the user's request.

When the workspace load fails due to a dependency that hasn't been
downloaded, we present a quick fix on the go.mod line to do so. The only
way that happens is if there's a manual modification to the go.mod file,
since all of the other quick fixes will do the download. So I don't
think there's a need to present the fix anywhere else.

Updates golang/go#38462.

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

@gopherbot gopherbot commented Feb 5, 2021

Change https://golang.org/cl/289309 mentions this issue: internal/lsp/cache: disable network for mod tidy diagnostics

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

Successfully merging a pull request may close this issue.

None yet
5 participants