internal/lsp/cache: assign a static temp workspace dir to the first view

Editors need a way to run commands in the same workspace that gopls
sees. Longer term, we need a good solution for this that supports
multiple workspace folders, but for now just write the first folder's
workspace to a deterministic location:
  $TMPDIR/gopls-<client PID>.workspace.

Using the client-provided PID allows this mechanism to work even for
multi-session daemons.

Along the way, simplify the snapshot reinitialization logic a bit.

Fixes golang/go#42126

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

Author: findleyr

gopls/internal/regtest/runner.go

@@ -125,6 +125,12 @@ func WithModes(modes Mode) RunOption {
 	})
 }
 
+func SendPID() RunOption {
+	return optionSetter(func(opts *runConfig) {
+		opts.editor.SendPID = true
+	})
+}
+
 // EditorConfig is a RunOption option that configured the regtest editor.
 type EditorConfig fake.EditorConfig
 

gopls/internal/regtest/workspace_test.go

@@ -6,6 +6,9 @@ package regtest
 
 import (
 	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -563,3 +566,65 @@ func main() {
 		)
 	})
 }
+
+func TestWorkspaceDirAccess(t *testing.T) {
+	const multiModule = `
+-- moda/a/go.mod --
+module a.com
+
+-- moda/a/a.go --
+package main
+
+func main() {
+	fmt.Println("Hello")
+}
+-- modb/go.mod --
+module b.com
+-- modb/b/b.go --
+package main
+
+func main() {
+	fmt.Println("World")
+}
+`
+	withOptions(
+		WithModes(Experimental),
+		SendPID(),
+	).run(t, multiModule, func(t *testing.T, env *Env) {
+		pid := os.Getpid()
+		// Don't factor this out of Server.addFolders. vscode-go expects this
+		// directory.
+		modPath := filepath.Join(os.TempDir(), fmt.Sprintf("gopls-%d.workspace", pid), "go.mod")
+		gotb, err := ioutil.ReadFile(modPath)
+		if err != nil {
+			t.Fatalf("reading expected workspace modfile: %v", err)
+		}
+		got := string(gotb)
+		for _, want := range []string{"a.com v0.0.0-goplsworkspace", "b.com v0.0.0-goplsworkspace"} {
+			if !strings.Contains(got, want) {
+				// want before got here, since the go.mod is multi-line
+				t.Fatalf("workspace go.mod missing %q. got:\n%s", want, got)
+			}
+		}
+		workdir := env.Sandbox.Workdir.RootURI().SpanURI().Filename()
+		env.WriteWorkspaceFile("gopls.mod", fmt.Sprintf(`
+				module gopls-workspace
+
+				require (
+					a.com v0.0.0-goplsworkspace
+				)
+
+				replace a.com => %s/moda/a
+				`, workdir))
+		env.Await(CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChangeWatchedFiles), 1))
+		gotb, err = ioutil.ReadFile(modPath)
+		if err != nil {
+			t.Fatalf("reading expected workspace modfile: %v", err)
+		}
+		got = string(gotb)
+		want := "b.com v0.0.0-goplsworkspace"
+		if strings.Contains(got, want) {
+			t.Fatalf("workspace go.mod contains unexpected %q. got:\n%s", want, got)
+		}
+	})
+}

internal/lsp/cache/session.go

@@ -7,6 +7,7 @@ package cache
 import (
 	"context"
 	"fmt"
+	"os"
 	"strconv"
 	"strings"
 	"sync"
@@ -143,10 +144,10 @@ func (s *Session) Cache() interface{} {
 	return s.cache
 }
 
-func (s *Session) NewView(ctx context.Context, name string, folder span.URI, options *source.Options) (source.View, source.Snapshot, func(), error) {
+func (s *Session) NewView(ctx context.Context, name string, folder, tempWorkspace span.URI, options *source.Options) (source.View, source.Snapshot, func(), error) {
 	s.viewMu.Lock()
 	defer s.viewMu.Unlock()
-	view, snapshot, release, err := s.createView(ctx, name, folder, options, 0)
+	view, snapshot, release, err := s.createView(ctx, name, folder, tempWorkspace, options, 0)
 	if err != nil {
 		return nil, nil, func() {}, err
 	}
@@ -156,7 +157,7 @@ func (s *Session) NewView(ctx context.Context, name string, folder span.URI, opt
 	return view, snapshot, release, nil
 }
 
-func (s *Session) createView(ctx context.Context, name string, folder span.URI, options *source.Options, snapshotID uint64) (*View, *snapshot, func(), error) {
+func (s *Session) createView(ctx context.Context, name string, folder, tempWorkspace span.URI, options *source.Options, snapshotID uint64) (*View, *snapshot, func(), error) {
 	index := atomic.AddInt64(&viewIndex, 1)
 
 	if s.cache.options != nil {
@@ -182,6 +183,8 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
 
 	v := &View{
 		session:              s,
+		initialWorkspaceLoad: make(chan struct{}),
+		initializationSema:   make(chan struct{}, 1),
 		id:                   strconv.FormatInt(index, 10),
 		options:              options,
 		baseCtx:              baseCtx,
@@ -192,6 +195,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
 		filesByURI:           make(map[span.URI]*fileBase),
 		filesByBase:          make(map[string][]*fileBase),
 		workspaceInformation: *ws,
+		tempWorkspace:        tempWorkspace,
 	}
 	v.importsState = &importsState{
 		ctx: backgroundCtx,
@@ -202,26 +206,24 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
 		},
 	}
 	v.snapshot = &snapshot{
-		id:                 snapshotID,
-		view:               v,
-		initialized:        make(chan struct{}),
-		initializationSema: make(chan struct{}, 1),
-		initializeOnce:     &sync.Once{},
-		generation:         s.cache.store.Generation(generationName(v, 0)),
-		packages:           make(map[packageKey]*packageHandle),
-		ids:                make(map[span.URI][]packageID),
-		metadata:           make(map[packageID]*metadata),
-		files:              make(map[span.URI]source.VersionedFileHandle),
-		goFiles:            make(map[parseKey]*parseGoHandle),
-		importedBy:         make(map[packageID][]packageID),
-		actions:            make(map[actionKey]*actionHandle),
-		workspacePackages:  make(map[packageID]packagePath),
-		unloadableFiles:    make(map[span.URI]struct{}),
-		parseModHandles:    make(map[span.URI]*parseModHandle),
-		modTidyHandles:     make(map[span.URI]*modTidyHandle),
-		modUpgradeHandles:  make(map[span.URI]*modUpgradeHandle),
-		modWhyHandles:      make(map[span.URI]*modWhyHandle),
-		workspace:          workspace,
+		id:                snapshotID,
+		view:              v,
+		initializeOnce:    &sync.Once{},
+		generation:        s.cache.store.Generation(generationName(v, 0)),
+		packages:          make(map[packageKey]*packageHandle),
+		ids:               make(map[span.URI][]packageID),
+		metadata:          make(map[packageID]*metadata),
+		files:             make(map[span.URI]source.VersionedFileHandle),
+		goFiles:           make(map[parseKey]*parseGoHandle),
+		importedBy:        make(map[packageID][]packageID),
+		actions:           make(map[actionKey]*actionHandle),
+		workspacePackages: make(map[packageID]packagePath),
+		unloadableFiles:   make(map[span.URI]struct{}),
+		parseModHandles:   make(map[span.URI]*parseModHandle),
+		modTidyHandles:    make(map[span.URI]*modTidyHandle),
+		modUpgradeHandles: make(map[span.URI]*modUpgradeHandle),
+		modWhyHandles:     make(map[span.URI]*modWhyHandle),
+		workspace:         workspace,
 	}
 
 	// Initialize the view without blocking.
@@ -231,6 +233,19 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
 	release := snapshot.generation.Acquire(initCtx)
 	go func() {
 		snapshot.initialize(initCtx, true)
+		if v.tempWorkspace != "" {
+			var err error
+			if err = os.Mkdir(v.tempWorkspace.Filename(), 0700); err == nil {
+				var wsdir span.URI
+				wsdir, err = snapshot.getWorkspaceDir(initCtx)
+				if err == nil {
+					err = copyWorkspace(v.tempWorkspace, wsdir)
+				}
+			}
+			if err != nil {
+				event.Error(initCtx, "creating workspace dir", err)
+			}
+		}
 		release()
 	}()
 	return v, snapshot, snapshot.generation.Acquire(ctx), nil
@@ -349,7 +364,7 @@ func (s *Session) updateView(ctx context.Context, view *View, options *source.Op
 	view.snapshotMu.Lock()
 	snapshotID := view.snapshot.id
 	view.snapshotMu.Unlock()
-	v, _, release, err := s.createView(ctx, view.name, view.folder, options, snapshotID)
+	v, _, release, err := s.createView(ctx, view.name, view.folder, view.tempWorkspace, options, snapshotID)
 	release()
 	if err != nil {
 		// we have dropped the old view, but could not create the new one

internal/lsp/cache/snapshot.go

@@ -47,31 +47,15 @@ type snapshot struct {
 	builtin *builtinPackageHandle
 
 	// The snapshot's initialization state is controlled by the fields below.
-	// These fields are propagated across snapshots to avoid multiple
-	// concurrent initializations. They may be invalidated during cloning.
 	//
-	// initialized is closed when the snapshot has been fully initialized. On
-	// initialization, the snapshot's workspace packages are loaded. All of the
-	// fields below are set as part of initialization. If we failed to load, we
-	// only retry if the go.mod file changes, to avoid too many go/packages
-	// calls.
-	//
-	// When the view is created, its snapshot's initializeOnce is non-nil,
-	// initialized is open. Once initialization completes, initializedErr may
-	// be set and initializeOnce becomes nil. If initializedErr is non-nil,
-	// initialization may be retried (depending on how files are changed). To
-	// indicate that initialization should be retried, initializeOnce will be
-	// set. The next time a caller requests workspace packages, the
-	// initialization will retry.
-	initialized chan struct{}
-
-	// initializationSema is used as a mutex to guard initializeOnce and
-	// initializedErr, which will be updated after each attempt to initialize
-	// the snapshot. We use a channel instead of a mutex to avoid blocking when
-	// a context is canceled.
-	initializationSema chan struct{}
-	initializeOnce     *sync.Once
-	initializedErr     error
+	// initializeOnce guards snapshot initialization. Each snapshot is
+	// initialized at most once: reinitialization is triggered on later snapshots
+	// by invalidating this field.
+	initializeOnce *sync.Once
+	// initializedErr holds the last error resulting from initialization. If
+	// initialization fails, we only retry when the the workspace modules change,
+	// to avoid too many go/packages calls.
+	initializedErr error
 
 	// mu guards all of the maps in the snapshot.
 	mu sync.Mutex
@@ -884,7 +868,7 @@ func (s *snapshot) AwaitInitialized(ctx context.Context) {
 	select {
 	case <-ctx.Done():
 		return
-	case <-s.initialized:
+	case <-s.view.initialWorkspaceLoad:
 	}
 	// We typically prefer to run something as intensive as the IWL without
 	// blocking. I'm not sure if there is a way to do that here.
@@ -1020,36 +1004,39 @@ func generationName(v *View, snapshotID uint64) string {
 	return fmt.Sprintf("v%v/%v", v.id, snapshotID)
 }
 
-func (s *snapshot) clone(ctx context.Context, changes map[span.URI]*fileChange, forceReloadMetadata bool) *snapshot {
+func (s *snapshot) clone(ctx context.Context, changes map[span.URI]*fileChange, forceReloadMetadata bool) (*snapshot, bool) {
+	// Track some important types of changes.
+	var (
+		vendorChanged  bool
+		modulesChanged bool
+	)
 	newWorkspace, workspaceChanged := s.workspace.invalidate(ctx, changes)
 
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
 	newGen := s.view.session.cache.store.Generation(generationName(s.view, s.id+1))
 	result := &snapshot{
-		id:                 s.id + 1,
-		generation:         newGen,
-		view:               s.view,
-		builtin:            s.builtin,
-		initialized:        s.initialized,
-		initializationSema: s.initializationSema,
-		initializeOnce:     s.initializeOnce,
-		initializedErr:     s.initializedErr,
-		ids:                make(map[span.URI][]packageID),
-		importedBy:         make(map[packageID][]packageID),
-		metadata:           make(map[packageID]*metadata),
-		packages:           make(map[packageKey]*packageHandle),
-		actions:            make(map[actionKey]*actionHandle),
-		files:              make(map[span.URI]source.VersionedFileHandle),
-		goFiles:            make(map[parseKey]*parseGoHandle),
-		workspacePackages:  make(map[packageID]packagePath),
-		unloadableFiles:    make(map[span.URI]struct{}),
-		parseModHandles:    make(map[span.URI]*parseModHandle),
-		modTidyHandles:     make(map[span.URI]*modTidyHandle),
-		modUpgradeHandles:  make(map[span.URI]*modUpgradeHandle),
-		modWhyHandles:      make(map[span.URI]*modWhyHandle),
-		workspace:          newWorkspace,
+		id:                s.id + 1,
+		generation:        newGen,
+		view:              s.view,
+		builtin:           s.builtin,
+		initializeOnce:    s.initializeOnce,
+		initializedErr:    s.initializedErr,
+		ids:               make(map[span.URI][]packageID),
+		importedBy:        make(map[packageID][]packageID),
+		metadata:          make(map[packageID]*metadata),
+		packages:          make(map[packageKey]*packageHandle),
+		actions:           make(map[actionKey]*actionHandle),
+		files:             make(map[span.URI]source.VersionedFileHandle),
+		goFiles:           make(map[parseKey]*parseGoHandle),
+		workspacePackages: make(map[packageID]packagePath),
+		unloadableFiles:   make(map[span.URI]struct{}),
+		parseModHandles:   make(map[span.URI]*parseModHandle),
+		modTidyHandles:    make(map[span.URI]*modTidyHandle),
+		modUpgradeHandles: make(map[span.URI]*modUpgradeHandle),
+		modWhyHandles:     make(map[span.URI]*modWhyHandle),
+		workspace:         newWorkspace,
 	}
 
 	if !workspaceChanged && s.workspaceDirHandle != nil {
@@ -1105,14 +1092,11 @@ func (s *snapshot) clone(ctx context.Context, changes map[span.URI]*fileChange,
 		result.modWhyHandles[k] = v
 	}
 
-	var reinitialize reinitializeView
-
 	// directIDs keeps track of package IDs that have directly changed.
 	// It maps id->invalidateMetadata.
 	directIDs := map[packageID]bool{}
 	// Invalidate all package metadata if the workspace module has changed.
 	if workspaceChanged {
-		reinitialize = definitelyReinit
 		for k := range s.metadata {
 			directIDs[k] = true
 		}
@@ -1122,7 +1106,7 @@ func (s *snapshot) clone(ctx context.Context, changes map[span.URI]*fileChange,
 		// Maybe reinitialize the view if we see a change in the vendor
 		// directory.
 		if inVendor(uri) {
-			reinitialize = maybeReinit
+			vendorChanged = true
 		}
 
 		// The original FileHandle for this URI is cached on the snapshot.
@@ -1158,8 +1142,8 @@ func (s *snapshot) clone(ctx context.Context, changes map[span.URI]*fileChange,
 			// If the view's go.mod file's contents have changed, invalidate
 			// the metadata for every known package in the snapshot.
 			delete(result.parseModHandles, uri)
-			if _, ok := result.workspace.activeModFiles()[uri]; ok && reinitialize < maybeReinit {
-				reinitialize = maybeReinit
+			if _, ok := result.workspace.activeModFiles()[uri]; ok {
+				modulesChanged = true
 			}
 		}
 		// Handle the invalidated file; it may have new contents or not exist.
@@ -1285,20 +1269,14 @@ copyIDs:
 	}
 
 	// The snapshot may need to be reinitialized.
-	if reinitialize != doNotReinit {
-		result.reinitialize(reinitialize == definitelyReinit)
+	if modulesChanged || workspaceChanged || vendorChanged {
+		if workspaceChanged || result.initializedErr != nil {
+			result.initializeOnce = &sync.Once{}
+		}
 	}
-	return result
+	return result, workspaceChanged
 }
 
-type reinitializeView int
-
-const (
-	doNotReinit = reinitializeView(iota)
-	maybeReinit
-	definitelyReinit
-)
-
 // guessPackagesForURI returns all packages related to uri. If we haven't seen this
 // URI before, we guess based on files in the same directory. This is of course
 // incorrect in build systems where packages are not organized by directory.