From 3e30e21b53ede197f8cb89110874a4f884dfa720 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Tue, 1 Feb 2022 11:54:19 -0500 Subject: [PATCH] gopls: remove the workspace_metadata command Remove the workspace_metadata command, as VS Code no longer needs this to run workspace commands (it can use go.work instead). Updates golang/go#44696 Change-Id: Ife579a15e64969c4301e4508e18b7c8a8b633b9f Reviewed-on: https://go-review.googlesource.com/c/tools/+/382235 Trust: Robert Findley Run-TryBot: Robert Findley Reviewed-by: Hyang-Ah Hana Kim gopls-CI: kokoro TryBot-Result: Gopher Robot --- gopls/doc/commands.md | 17 ---- .../regtest/workspace/workspace_test.go | 77 ------------------- internal/lsp/cache/session.go | 14 +--- internal/lsp/cache/snapshot.go | 4 +- internal/lsp/cache/view.go | 61 +-------------- internal/lsp/command.go | 11 --- internal/lsp/command/command_gen.go | 16 ---- internal/lsp/command/interface.go | 5 -- internal/lsp/lsp_test.go | 2 +- internal/lsp/mod/mod_test.go | 2 +- internal/lsp/source/api_json.go | 6 -- internal/lsp/source/source_test.go | 2 +- internal/lsp/source/view.go | 6 +- internal/lsp/workspace.go | 19 +---- 14 files changed, 12 insertions(+), 230 deletions(-) diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md index 4c44f42559..97e18527f7 100644 --- a/gopls/doc/commands.md +++ b/gopls/doc/commands.md @@ -350,21 +350,4 @@ Args: } ``` -### **Query workspace metadata** -Identifier: `gopls.workspace_metadata` - -Query the server for information about active workspaces. - -Result: - -``` -{ - // All workspaces for this session. - "Workspaces": []{ - "Name": string, - "ModuleDir": string, - }, -} -``` - diff --git a/gopls/internal/regtest/workspace/workspace_test.go b/gopls/internal/regtest/workspace/workspace_test.go index d3275de906..a32a9644b4 100644 --- a/gopls/internal/regtest/workspace/workspace_test.go +++ b/gopls/internal/regtest/workspace/workspace_test.go @@ -5,9 +5,7 @@ package workspace import ( - "encoding/json" "fmt" - "io/ioutil" "path/filepath" "sort" "strings" @@ -17,7 +15,6 @@ import ( . "golang.org/x/tools/internal/lsp/regtest" "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/lsp/command" "golang.org/x/tools/internal/lsp/fake" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/testenv" @@ -852,80 +849,6 @@ func main() { }) } -func TestWorkspaceDirAccess(t *testing.T) { - const multiModule = ` --- moda/a/go.mod -- -module a.com - -go 1.15 --- moda/a/a.go -- -package main - -func main() { - fmt.Println("Hello") -} --- modb/go.mod -- -module b.com - -go 1.16 --- modb/b/b.go -- -package main - -func main() { - fmt.Println("World") -} -` - WithOptions( - Modes(Experimental), - SendPID(), - ).Run(t, multiModule, func(t *testing.T, env *Env) { - params := &protocol.ExecuteCommandParams{ - Command: command.WorkspaceMetadata.ID(), - Arguments: []json.RawMessage{json.RawMessage("{}")}, - } - var result command.WorkspaceMetadataResult - env.ExecuteCommand(params, &result) - - if n := len(result.Workspaces); n != 1 { - env.T.Fatalf("got %d workspaces, want 1", n) - } - // Don't factor this out of Server.addFolders. vscode-go expects this - // directory. - modPath := filepath.Join(result.Workspaces[0].ModuleDir, "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{"go 1.16", "a.com v1.9999999.0-goplsworkspace", "b.com v1.9999999.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 v1.9999999.0-goplsworkspace - ) - - replace a.com => %s/moda/a - `, workdir)) - env.Await(env.DoneWithChangeWatchedFiles()) - gotb, err = ioutil.ReadFile(modPath) - if err != nil { - t.Fatalf("reading expected workspace modfile: %v", err) - } - got = string(gotb) - want := "b.com v1.9999999.0-goplsworkspace" - if strings.Contains(got, want) { - t.Fatalf("workspace go.mod contains unexpected %q. got:\n%s", want, got) - } - }) -} - func TestDirectoryFiltersLoads(t *testing.T) { // exclude, and its error, should be excluded from the workspace. const files = ` diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index d38b6a8b59..84e3848d42 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -155,7 +155,7 @@ func (s *Session) Cache() interface{} { return s.cache } -func (s *Session) NewView(ctx context.Context, name string, folder, tempWorkspace span.URI, options *source.Options) (source.View, source.Snapshot, func(), error) { +func (s *Session) NewView(ctx context.Context, name string, folder span.URI, options *source.Options) (source.View, source.Snapshot, func(), error) { s.viewMu.Lock() defer s.viewMu.Unlock() for _, view := range s.views { @@ -163,7 +163,7 @@ func (s *Session) NewView(ctx context.Context, name string, folder, tempWorkspac return nil, nil, nil, source.ErrViewExists } } - view, snapshot, release, err := s.createView(ctx, name, folder, tempWorkspace, options, 0) + view, snapshot, release, err := s.createView(ctx, name, folder, options, 0) if err != nil { return nil, nil, func() {}, err } @@ -173,7 +173,7 @@ func (s *Session) NewView(ctx context.Context, name string, folder, tempWorkspac return view, snapshot, release, nil } -func (s *Session) createView(ctx context.Context, name string, folder, tempWorkspace span.URI, options *source.Options, snapshotID uint64) (*View, *snapshot, func(), error) { +func (s *Session) createView(ctx context.Context, name string, folder span.URI, options *source.Options, snapshotID uint64) (*View, *snapshot, func(), error) { index := atomic.AddInt64(&viewIndex, 1) if s.cache.options != nil { @@ -218,7 +218,6 @@ func (s *Session) createView(ctx context.Context, name string, folder, tempWorks filesByBase: map[string][]*fileBase{}, rootURI: root, workspaceInformation: *ws, - tempWorkspace: tempWorkspace, } v.importsState = &importsState{ ctx: backgroundCtx, @@ -257,11 +256,6 @@ func (s *Session) createView(ctx context.Context, name string, folder, tempWorks go func() { defer release() snapshot.initialize(initCtx, true) - // Ensure that the view workspace is written at least once following - // initialization. - if err := v.updateWorkspace(initCtx); err != nil { - event.Error(ctx, "copying workspace dir", err) - } }() return v, snapshot, snapshot.generation.Acquire(), nil } @@ -381,7 +375,7 @@ func (s *Session) updateView(ctx context.Context, view *View, options *source.Op return nil, err } - v, _, release, err := s.createView(ctx, view.name, view.folder, view.tempWorkspace, options, snapshotID) + v, _, release, err := s.createView(ctx, view.name, view.folder, options, snapshotID) release() if err != nil { diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 1dd3339e8e..e913c3e16f 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -1664,7 +1664,7 @@ func (ac *unappliedChanges) GetFile(ctx context.Context, uri span.URI) (source.F return ac.originalSnapshot.GetFile(ctx, uri) } -func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileChange, forceReloadMetadata bool) (*snapshot, bool) { +func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileChange, forceReloadMetadata bool) *snapshot { var vendorChanged bool newWorkspace, workspaceChanged, workspaceReload := s.workspace.invalidate(ctx, changes, &unappliedChanges{ originalSnapshot: s, @@ -2044,7 +2044,7 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC result.initializeOnce = &sync.Once{} } } - return result, workspaceChanged + return result } // guessPackageIDsForURI returns all packages related to uri. If we haven't diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index eb61d570d8..a6a02bfb4f 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -93,10 +93,6 @@ type View struct { // workspaceInformation tracks various details about this view's // environment variables, go version, and use of modules. workspaceInformation - - // tempWorkspace is a temporary directory dedicated to holding the latest - // version of the workspace go.mod file. (TODO: also go.sum file) - tempWorkspace span.URI } type workspaceInformation struct { @@ -231,10 +227,6 @@ func (v *View) Folder() span.URI { return v.folder } -func (v *View) TempWorkspace() span.URI { - return v.tempWorkspace -} - func (v *View) Options() *source.Options { v.optionsMu.Lock() defer v.optionsMu.Unlock() @@ -734,63 +726,12 @@ func (v *View) invalidateContent(ctx context.Context, changes map[span.URI]*file oldSnapshot := v.snapshot - var workspaceChanged bool - v.snapshot, workspaceChanged = oldSnapshot.clone(ctx, v.baseCtx, changes, forceReloadMetadata) - if workspaceChanged { - if err := v.updateWorkspaceLocked(ctx); err != nil { - event.Error(ctx, "copying workspace dir", err) - } - } + v.snapshot = oldSnapshot.clone(ctx, v.baseCtx, changes, forceReloadMetadata) go oldSnapshot.generation.Destroy("View.invalidateContent") return v.snapshot, v.snapshot.generation.Acquire() } -func (v *View) updateWorkspace(ctx context.Context) error { - if v.tempWorkspace == "" { - return nil - } - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() - return v.updateWorkspaceLocked(ctx) -} - -// updateWorkspaceLocked should only be called when v.snapshotMu is held. It -// guarantees that workspace module content will be copied to v.tempWorkace at -// some point in the future. We do not guarantee that the temp workspace sees -// all changes to the workspace module, only that it is eventually consistent -// with the workspace module of the latest snapshot. -func (v *View) updateWorkspaceLocked(ctx context.Context) error { - if v.snapshot == nil { - return errors.New("view is shutting down") - } - - release := v.snapshot.generation.Acquire() - defer release() - src, err := v.snapshot.getWorkspaceDir(ctx) - if err != nil { - return err - } - for _, name := range []string{"go.mod", "go.sum"} { - srcname := filepath.Join(src.Filename(), name) - srcf, err := os.Open(srcname) - if err != nil { - return errors.Errorf("opening snapshot %s: %w", name, err) - } - defer srcf.Close() - dstname := filepath.Join(v.tempWorkspace.Filename(), name) - dstf, err := os.Create(dstname) - if err != nil { - return errors.Errorf("truncating view %s: %w", name, err) - } - defer dstf.Close() - if _, err := io.Copy(dstf, srcf); err != nil { - return errors.Errorf("copying %s: %w", name, err) - } - } - return nil -} - func (s *Session) getWorkspaceInformation(ctx context.Context, folder span.URI, options *source.Options) (*workspaceInformation, error) { if err := checkPathCase(folder.Filename()); err != nil { return nil, errors.Errorf("invalid workspace folder path: %w; check that the casing of the configured workspace folder path agrees with the casing reported by the operating system", err) diff --git a/internal/lsp/command.go b/internal/lsp/command.go index 36c319f119..153af443b0 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -701,17 +701,6 @@ func (c *commandHandler) AddImport(ctx context.Context, args command.AddImportAr }) } -func (c *commandHandler) WorkspaceMetadata(ctx context.Context) (command.WorkspaceMetadataResult, error) { - var result command.WorkspaceMetadataResult - for _, view := range c.s.session.Views() { - result.Workspaces = append(result.Workspaces, command.Workspace{ - Name: view.Name(), - ModuleDir: view.TempWorkspace().Filename(), - }) - } - return result, nil -} - func (c *commandHandler) StartDebugging(ctx context.Context, args command.DebuggingArgs) (result command.DebuggingResult, _ error) { addr := args.Addr if addr == "" { diff --git a/internal/lsp/command/command_gen.go b/internal/lsp/command/command_gen.go index 09ebd2feec..f872c7fc69 100644 --- a/internal/lsp/command/command_gen.go +++ b/internal/lsp/command/command_gen.go @@ -38,7 +38,6 @@ const ( UpdateGoSum Command = "update_go_sum" UpgradeDependency Command = "upgrade_dependency" Vendor Command = "vendor" - WorkspaceMetadata Command = "workspace_metadata" ) var Commands = []Command{ @@ -61,7 +60,6 @@ var Commands = []Command{ UpdateGoSum, UpgradeDependency, Vendor, - WorkspaceMetadata, } func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Interface) (interface{}, error) { @@ -182,8 +180,6 @@ func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Inte return nil, err } return nil, s.Vendor(ctx, a0) - case "gopls.workspace_metadata": - return s.WorkspaceMetadata(ctx) } return nil, fmt.Errorf("unsupported command %q", params.Command) } @@ -415,15 +411,3 @@ func NewVendorCommand(title string, a0 URIArg) (protocol.Command, error) { Arguments: args, }, nil } - -func NewWorkspaceMetadataCommand(title string) (protocol.Command, error) { - args, err := MarshalArgs() - if err != nil { - return protocol.Command{}, err - } - return protocol.Command{ - Title: title, - Command: "gopls.workspace_metadata", - Arguments: args, - }, nil -} diff --git a/internal/lsp/command/interface.go b/internal/lsp/command/interface.go index 360dfc3492..d80e675b2a 100644 --- a/internal/lsp/command/interface.go +++ b/internal/lsp/command/interface.go @@ -127,11 +127,6 @@ type Interface interface { // themselves. AddImport(context.Context, AddImportArgs) error - // WorkspaceMetadata: Query workspace metadata - // - // Query the server for information about active workspaces. - WorkspaceMetadata(context.Context) (WorkspaceMetadataResult, error) - // StartDebugging: Start the gopls debug server // // Start the gopls debug server if it isn't running, and return the debug diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index ccfe2c9aa0..c3cd5461d3 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -53,7 +53,7 @@ func testLSP(t *testing.T, datum *tests.Data) { tests.DefaultOptions(options) session.SetOptions(options) options.SetEnvSlice(datum.Config.Env) - view, snapshot, release, err := session.NewView(ctx, datum.Config.Dir, span.URIFromPath(datum.Config.Dir), "", options) + view, snapshot, release, err := session.NewView(ctx, datum.Config.Dir, span.URIFromPath(datum.Config.Dir), options) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/mod/mod_test.go b/internal/lsp/mod/mod_test.go index 32989107fc..b2d257caee 100644 --- a/internal/lsp/mod/mod_test.go +++ b/internal/lsp/mod/mod_test.go @@ -45,7 +45,7 @@ func TestModfileRemainsUnchanged(t *testing.T) { if err != nil { t.Fatal(err) } - _, _, release, err := session.NewView(ctx, "diagnostics_test", span.URIFromPath(folder), "", options) + _, _, release, err := session.NewView(ctx, "diagnostics_test", span.URIFromPath(folder), options) release() if err != nil { t.Fatal(err) diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index 78a4af50b7..cf5f97888e 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -690,12 +690,6 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "Runs `go mod vendor` for a module.", ArgDoc: "{\n\t// The file URI.\n\t\"URI\": string,\n}", }, - { - Command: "gopls.workspace_metadata", - Title: "Query workspace metadata", - Doc: "Query the server for information about active workspaces.", - ResultDoc: "{\n\t// All workspaces for this session.\n\t\"Workspaces\": []{\n\t\t\"Name\": string,\n\t\t\"ModuleDir\": string,\n\t},\n}", - }, }, Lenses: []*LensJSON{ { diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 4a2c43fcf3..dc5fe53b58 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -52,7 +52,7 @@ func testSource(t *testing.T, datum *tests.Data) { options := source.DefaultOptions().Clone() tests.DefaultOptions(options) options.SetEnvSlice(datum.Config.Env) - view, _, release, err := session.NewView(ctx, "source_test", span.URIFromPath(datum.Config.Dir), "", options) + view, _, release, err := session.NewView(ctx, "source_test", span.URIFromPath(datum.Config.Dir), options) release() if err != nil { t.Fatal(err) diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index e7120ec5af..d7f4db1a71 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -232,10 +232,6 @@ type View interface { // Folder returns the folder with which this view was created. Folder() span.URI - // TempWorkspace returns the folder this view uses for its temporary - // workspace module. - TempWorkspace() span.URI - // Shutdown closes this view, and detaches it from its session. Shutdown(ctx context.Context) @@ -325,7 +321,7 @@ type Session interface { // non-empty tempWorkspace directory is provided, the View will record a copy // of its gopls workspace module in that directory, so that client tooling // can execute in the same main module. - NewView(ctx context.Context, name string, folder, tempWorkspace span.URI, options *Options) (View, Snapshot, func(), error) + NewView(ctx context.Context, name string, folder span.URI, options *Options) (View, Snapshot, func(), error) // Cache returns the cache that created this session, for debugging only. Cache() interface{} diff --git a/internal/lsp/workspace.go b/internal/lsp/workspace.go index c239942f29..74892ca6ef 100644 --- a/internal/lsp/workspace.go +++ b/internal/lsp/workspace.go @@ -6,12 +6,7 @@ package lsp import ( "context" - "fmt" - "os" - "path/filepath" - "sync/atomic" - "golang.org/x/tools/internal/event" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" @@ -44,19 +39,7 @@ func (s *Server) addView(ctx context.Context, name string, uri span.URI) (source if err := s.fetchConfig(ctx, name, uri, options); err != nil { return nil, func() {}, err } - // Try to assign a persistent temp directory for tracking this view's - // temporary workspace. - var tempWorkspace span.URI - if s.tempDir != "" { - index := atomic.AddInt64(&wsIndex, 1) - wsDir := filepath.Join(s.tempDir, fmt.Sprintf("workspace.%d", index)) - if err := os.Mkdir(wsDir, 0700); err == nil { - tempWorkspace = span.URIFromPath(wsDir) - } else { - event.Error(ctx, "making workspace dir", err) - } - } - _, snapshot, release, err := s.session.NewView(ctx, name, uri, tempWorkspace, options) + _, snapshot, release, err := s.session.NewView(ctx, name, uri, options) return snapshot, release, err }