Skip to content

Commit

Permalink
internal/lsp: print the go version only once the view is created
Browse files Browse the repository at this point in the history
Printing the Go version without the session's go command runner means
that we may not find the right Go version. Also, panicking when we
cannot find a go command is not useful to the user--show the error as a
view initialization error instead.

Fixes golang/go#41701

Change-Id: I0e0753da9795b1c78331db1faecd27c2bfcee9b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258312
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
  • Loading branch information
stamblerre committed Sep 30, 2020
1 parent c8c0a1c commit 66e72d0
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 28 deletions.
22 changes: 20 additions & 2 deletions internal/lsp/cache/view.go
Expand Up @@ -358,8 +358,26 @@ func (s *snapshot) WriteEnv(ctx context.Context, w io.Writer) error {
fullEnv[s[0]] = s[1]
}
}
fmt.Fprintf(w, "go env for %v\n(root %s)\n(valid build configuration = %v)\n(build flags: %v)\n",
s.view.folder.Filename(), s.view.rootURI.Filename(), s.view.hasValidBuildConfiguration, buildFlags)
goVersion, err := s.view.session.gocmdRunner.Run(ctx, gocommand.Invocation{
Verb: "version",
BuildFlags: buildFlags,
Env: env,
WorkingDir: s.view.rootURI.Filename(),
})
if err != nil {
return err
}
fmt.Fprintf(w, `go env for %v
(root %s)
(go version %s)
(valid build configuration = %v)
(build flags: %v)
`,
s.view.folder.Filename(),
s.view.rootURI.Filename(),
goVersion.String(),
s.view.hasValidBuildConfiguration,
buildFlags)
for k, v := range fullEnv {
fmt.Fprintf(w, "%s=%s\n", k, v)
}
Expand Down
13 changes: 0 additions & 13 deletions internal/lsp/debug/info.go
Expand Up @@ -10,8 +10,6 @@ import (
"fmt"
"io"
"strings"

"golang.org/x/tools/internal/gocommand"
)

type PrintMode int
Expand Down Expand Up @@ -48,17 +46,6 @@ func PrintVersionInfo(ctx context.Context, w io.Writer, verbose bool, mode Print
section(w, mode, "Build info", func() {
printBuildInfo(w, true, mode)
})
fmt.Fprint(w, "\n")
section(w, mode, "Go info", func() {
gocmdRunner := &gocommand.Runner{}
version, err := gocmdRunner.Run(ctx, gocommand.Invocation{
Verb: "version",
})
if err != nil {
panic(err)
}
fmt.Fprintln(w, version.String())
})
}

func section(w io.Writer, mode PrintMode, title string, body func()) {
Expand Down
11 changes: 2 additions & 9 deletions internal/lsp/general.go
Expand Up @@ -18,7 +18,6 @@ import (
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/jsonrpc2"
"golang.org/x/tools/internal/lsp/debug"
"golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/span"
Expand Down Expand Up @@ -165,12 +164,6 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa
options := s.session.Options()
defer func() { s.session.SetOptions(options) }()

// TODO: this event logging may be unnecessary.
// The version info is included in the initialize response.
buf := &bytes.Buffer{}
debug.PrintVersionInfo(ctx, buf, true, debug.PlainText)
event.Log(ctx, buf.String())

if err := s.addFolders(ctx, s.pendingFolders); err != nil {
return err
}
Expand Down Expand Up @@ -217,7 +210,7 @@ func (s *Server) addFolders(ctx context.Context, folders []protocol.WorkspaceFol
continue
}
work := s.progress.start(ctx, "Setting up workspace", "Loading packages...", nil, nil)
view, snapshot, release, err := s.addView(ctx, folder.Name, uri)
snapshot, release, err := s.addView(ctx, folder.Name, uri)
if err != nil {
viewErrors[uri] = err
work.end(fmt.Sprintf("Error loading packages: %s", err))
Expand All @@ -238,7 +231,7 @@ func (s *Server) addFolders(ctx context.Context, folders []protocol.WorkspaceFol
// Print each view's environment.
buf := &bytes.Buffer{}
if err := snapshot.WriteEnv(ctx, buf); err != nil {
event.Error(ctx, "failed to write environment", err, tag.Directory.Of(view.Folder().Filename()))
viewErrors[uri] = err
continue
}
event.Log(ctx, buf.String())
Expand Down
9 changes: 5 additions & 4 deletions internal/lsp/workspace.go
Expand Up @@ -26,18 +26,19 @@ func (s *Server) didChangeWorkspaceFolders(ctx context.Context, params *protocol
return s.addFolders(ctx, event.Added)
}

func (s *Server) addView(ctx context.Context, name string, uri span.URI) (source.View, source.Snapshot, func(), error) {
func (s *Server) addView(ctx context.Context, name string, uri span.URI) (source.Snapshot, func(), error) {
s.stateMu.Lock()
state := s.state
s.stateMu.Unlock()
if state < serverInitialized {
return nil, nil, func() {}, errors.Errorf("addView called before server initialized")
return nil, func() {}, errors.Errorf("addView called before server initialized")
}
options := s.session.Options().Clone()
if err := s.fetchConfig(ctx, name, uri, options); err != nil {
return nil, nil, func() {}, err
return nil, func() {}, err
}
return s.session.NewView(ctx, name, uri, options)
_, snapshot, release, err := s.session.NewView(ctx, name, uri, options)
return snapshot, release, err
}

func (s *Server) didChangeConfiguration(ctx context.Context, changed interface{}) error {
Expand Down

0 comments on commit 66e72d0

Please sign in to comment.