Skip to content

Commit 224a61b

Browse files
adonovanadonovan
committed
gopls/internal/lsp/source: delete Snapshot.WriteEnv method
This method is called once immediately after View construction. It used to run 'go version', but with minor tweaks, the View already has this information via wsInfo. This change removes it from the Snapshot interface and eliminates the unnecessary 'go version' subprocess. Change-Id: I86e8dd37a7a237949c05820ca3e4fdb5035f90f7 Reviewed-on: https://go-review.googlesource.com/c/tools/+/459782 gopls-CI: kokoro <noreply+kokoro@google.com> Run-TryBot: Alan Donovan <adonovan@google.com> Reviewed-by: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
1 parent 81e741e commit 224a61b

6 files changed

Lines changed: 44 additions & 54 deletions

File tree

‎gopls/internal/lsp/cache/session.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,9 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
286286
// Save one reference in the view.
287287
v.releaseSnapshot = v.snapshot.Acquire()
288288

289+
// Record the environment of the newly created view in the log.
290+
event.Log(ctx, viewEnv(v))
291+
289292
// Initialize the view without blocking.
290293
initCtx, initCancel := context.WithCancel(xcontext.Detach(ctx))
291294
v.initCancelFirstAttempt = initCancel

‎gopls/internal/lsp/cache/view.go‎

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
package cache
77

88
import (
9+
"bytes"
910
"context"
1011
"encoding/json"
1112
"fmt"
12-
"io"
1313
"io/ioutil"
1414
"os"
1515
"path"
@@ -123,8 +123,10 @@ type workspaceInformation struct {
123123
// The Go version in use: X in Go 1.X.
124124
goversion int
125125

126-
// The Go version reported by go version command. (e.g. go1.19.1, go1.20-rc.1, go1.21-abcdef01)
127-
goversionString string
126+
// The complete output of the go version command.
127+
// (Call gocommand.ParseGoVersionOutput to extract a version
128+
// substring such as go1.19.1 or go1.20-rc.1, go1.21-abcdef01.)
129+
goversionOutput string
128130

129131
// hasGopackagesDriver is true if the user has a value set for the
130132
// GOPACKAGESDRIVER environment variable or a gopackagesdriver binary on
@@ -346,14 +348,28 @@ func (s *Session) SetViewOptions(ctx context.Context, v *View, options *source.O
346348
return newView, err
347349
}
348350

349-
func (s *snapshot) WriteEnv(ctx context.Context, w io.Writer) error {
350-
s.view.optionsMu.Lock()
351-
env := s.view.options.EnvSlice()
352-
buildFlags := append([]string{}, s.view.options.BuildFlags...)
353-
s.view.optionsMu.Unlock()
351+
// viewEnv returns a string describing the environment of a newly created view.
352+
func viewEnv(v *View) string {
353+
v.optionsMu.Lock()
354+
env := v.options.EnvSlice()
355+
buildFlags := append([]string{}, v.options.BuildFlags...)
356+
v.optionsMu.Unlock()
357+
358+
var buf bytes.Buffer
359+
fmt.Fprintf(&buf, `go env for %v
360+
(root %s)
361+
(go version %s)
362+
(valid build configuration = %v)
363+
(build flags: %v)
364+
`,
365+
v.folder.Filename(),
366+
v.rootURI.Filename(),
367+
strings.TrimRight(v.workspaceInformation.goversionOutput, "\n"),
368+
v.snapshot.ValidBuildConfiguration(),
369+
buildFlags)
354370

355371
fullEnv := make(map[string]string)
356-
for k, v := range s.view.goEnv {
372+
for k, v := range v.goEnv {
357373
fullEnv[k] = v
358374
}
359375
for _, v := range env {
@@ -365,29 +381,11 @@ func (s *snapshot) WriteEnv(ctx context.Context, w io.Writer) error {
365381
fullEnv[s[0]] = s[1]
366382
}
367383
}
368-
goVersion, err := s.view.gocmdRunner.Run(ctx, gocommand.Invocation{
369-
Verb: "version",
370-
Env: env,
371-
WorkingDir: s.view.rootURI.Filename(),
372-
})
373-
if err != nil {
374-
return err
375-
}
376-
fmt.Fprintf(w, `go env for %v
377-
(root %s)
378-
(go version %s)
379-
(valid build configuration = %v)
380-
(build flags: %v)
381-
`,
382-
s.view.folder.Filename(),
383-
s.view.rootURI.Filename(),
384-
strings.TrimRight(goVersion.String(), "\n"),
385-
s.ValidBuildConfiguration(),
386-
buildFlags)
387384
for k, v := range fullEnv {
388-
fmt.Fprintf(w, "%s=%s\n", k, v)
385+
fmt.Fprintf(&buf, "%s=%s\n", k, v)
389386
}
390-
return nil
387+
388+
return buf.String()
391389
}
392390

393391
func (s *snapshot) RunProcessEnvFunc(ctx context.Context, fn func(*imports.Options) error) error {
@@ -816,7 +814,7 @@ func (s *Session) getWorkspaceInformation(ctx context.Context, folder span.URI,
816814
if err != nil {
817815
return nil, err
818816
}
819-
goversionString, err := gocommand.GoVersionString(ctx, inv, s.gocmdRunner)
817+
goversionOutput, err := gocommand.GoVersionOutput(ctx, inv, s.gocmdRunner)
820818
if err != nil {
821819
return nil, err
822820
}
@@ -847,7 +845,7 @@ func (s *Session) getWorkspaceInformation(ctx context.Context, folder span.URI,
847845
return &workspaceInformation{
848846
hasGopackagesDriver: hasGopackagesDriver,
849847
goversion: goversion,
850-
goversionString: goversionString,
848+
goversionOutput: goversionOutput,
851849
environmentVariables: envVars,
852850
goEnv: env,
853851
}, nil
@@ -1072,7 +1070,7 @@ func (v *View) GoVersion() int {
10721070
}
10731071

10741072
func (v *View) GoVersionString() string {
1075-
return v.workspaceInformation.goversionString
1073+
return gocommand.ParseGoVersionOutput(v.workspaceInformation.goversionOutput)
10761074
}
10771075

10781076
// Copied from

‎gopls/internal/lsp/general.go‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
package lsp
66

77
import (
8-
"bytes"
98
"context"
109
"encoding/json"
1110
"fmt"
@@ -339,15 +338,6 @@ func (s *Server) addFolders(ctx context.Context, folders []protocol.WorkspaceFol
339338
}
340339
// Inv: release() must be called once.
341340

342-
// Print each view's environment.
343-
var buf bytes.Buffer
344-
if err := snapshot.WriteEnv(ctx, &buf); err != nil {
345-
viewErrors[uri] = err
346-
release()
347-
continue
348-
}
349-
event.Log(ctx, buf.String())
350-
351341
// Initialize snapshot asynchronously.
352342
initialized := make(chan struct{})
353343
nsnapshots.Add(1)

‎gopls/internal/lsp/source/view.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,6 @@ type Snapshot interface {
7474
// and their GOPATH.
7575
ValidBuildConfiguration() bool
7676

77-
// WriteEnv writes the view-specific environment to the io.Writer.
78-
WriteEnv(ctx context.Context, w io.Writer) error
79-
8077
// FindFile returns the FileHandle for the given URI, if it is already
8178
// in the given snapshot.
8279
FindFile(uri span.URI) VersionedFileHandle

‎internal/gocommand/version.go‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,22 +58,24 @@ func GoVersion(ctx context.Context, inv Invocation, r *Runner) (int, error) {
5858
return 0, fmt.Errorf("no parseable ReleaseTags in %v", tags)
5959
}
6060

61-
// GoVersionString reports the go version string as shown in `go version` command output.
62-
// When `go version` outputs in non-standard form, this returns an empty string.
63-
func GoVersionString(ctx context.Context, inv Invocation, r *Runner) (string, error) {
61+
// GoVersionOutput returns the complete output of the go version command.
62+
func GoVersionOutput(ctx context.Context, inv Invocation, r *Runner) (string, error) {
6463
inv.Verb = "version"
6564
goVersion, err := r.Run(ctx, inv)
6665
if err != nil {
6766
return "", err
6867
}
69-
return parseGoVersionOutput(goVersion.Bytes()), nil
68+
return goVersion.String(), nil
7069
}
7170

72-
func parseGoVersionOutput(data []byte) string {
71+
// ParseGoVersionOutput extracts the Go version string
72+
// from the output of the "go version" command.
73+
// Given an unrecognized form, it returns an empty string.
74+
func ParseGoVersionOutput(data string) string {
7375
re := regexp.MustCompile(`^go version (go\S+|devel \S+)`)
74-
m := re.FindSubmatch(data)
76+
m := re.FindStringSubmatch(data)
7577
if len(m) != 2 {
7678
return "" // unrecognized version
7779
}
78-
return string(m[1])
80+
return m[1]
7981
}

‎internal/gocommand/version_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func TestParseGoVersionOutput(t *testing.T) {
2323
}
2424
for i, tt := range tests {
2525
t.Run(strconv.Itoa(i), func(t *testing.T) {
26-
if got := parseGoVersionOutput([]byte(tt.args)); got != tt.want {
26+
if got := ParseGoVersionOutput(tt.args); got != tt.want {
2727
t.Errorf("parseGoVersionOutput() = %v, want %v", got, tt.want)
2828
}
2929
})

0 commit comments

Comments
 (0)