internal/lsp: run go mod vendor exclusively to avoid file contention
Running go mod vendor can result in vendor/modules.txt being transiently
deleted while it is being updated. On Windows this introduces potential
problems with file locking, if modules.txt is being read by another go
process, such as an ongoing package load.
Change the command to use RunGoCommandPiped, which executes serially
within the gopls process.
For golang/go#49646
Change-Id: If2d1fe5431122ba05014051a0e9c303cf7ee9cb2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/399714
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/gopls/internal/regtest/misc/vendor_test.go b/gopls/internal/regtest/misc/vendor_test.go
index 0e615f2..324a800 100644
--- a/gopls/internal/regtest/misc/vendor_test.go
+++ b/gopls/internal/regtest/misc/vendor_test.go
@@ -5,7 +5,6 @@
package misc
import (
- "runtime"
"testing"
. "golang.org/x/tools/internal/lsp/regtest"
@@ -27,9 +26,16 @@
func TestInconsistentVendoring(t *testing.T) {
testenv.NeedsGo1Point(t, 14)
- if runtime.GOOS == "windows" {
- t.Skipf("skipping test due to flakiness on Windows: https://golang.org/issue/49646")
- }
+
+ // TODO(golang/go#49646): delete this comment once this test is stable.
+ //
+ // In golang/go#49646, this test is reported as flaky on Windows. We believe
+ // this is due to file contention from go mod vendor that should be resolved.
+ // If this test proves to still be flaky, skip it.
+ //
+ // if runtime.GOOS == "windows" {
+ // t.Skipf("skipping test due to flakiness on Windows: https://golang.org/issue/49646")
+ // }
const pkgThatUsesVendoring = `
-- go.mod --
diff --git a/internal/lsp/command.go b/internal/lsp/command.go
index ae19a7d..072ee08 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -256,11 +256,19 @@
progress: "Running go mod vendor",
forURI: args.URI,
}, func(ctx context.Context, deps commandDeps) error {
- _, err := deps.snapshot.RunGoCommandDirect(ctx, source.Normal|source.AllowNetwork, &gocommand.Invocation{
+ // Use RunGoCommandPiped here so that we don't compete with any other go
+ // command invocations. go mod vendor deletes modules.txt before recreating
+ // it, and therefore can run into file locking issues on Windows if that
+ // file is in use by another process, such as go list.
+ //
+ // If golang/go#44119 is resolved, go mod vendor will instead modify
+ // modules.txt in-place. In that case we could theoretically allow this
+ // command to run concurrently.
+ err := deps.snapshot.RunGoCommandPiped(ctx, source.Normal|source.AllowNetwork, &gocommand.Invocation{
Verb: "mod",
Args: []string{"vendor"},
WorkingDir: filepath.Dir(args.URI.SpanURI().Filename()),
- })
+ }, &bytes.Buffer{}, &bytes.Buffer{})
return err
})
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index d263c1a..f698a07 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -98,6 +98,10 @@
// RunGoCommandPiped runs the given `go` command, writing its output
// to stdout and stderr. Verb, Args, and WorkingDir must be specified.
+ //
+ // RunGoCommandPiped runs the command serially using gocommand.RunPiped,
+ // enforcing that this command executes exclusively to other commands on the
+ // server.
RunGoCommandPiped(ctx context.Context, mode InvocationFlags, inv *gocommand.Invocation, stdout, stderr io.Writer) error
// RunGoCommandDirect runs the given `go` command. Verb, Args, and
