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>

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 @@ var Goodbye error
 
 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 --

internal/lsp/command.go

@@ -256,11 +256,19 @@ func (c *commandHandler) Vendor(ctx context.Context, args command.URIArg) error
 		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
 	})
 }

internal/lsp/source/view.go

@@ -98,6 +98,10 @@ type Snapshot interface {
 
 	// 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