Skip to content

Commit 7646fae

Browse files
committed
internal/lsp/fake: use hash rather than mtime to identify workdir files
On builders with low resolution clocks (e.g. WSL), it's possible for us to miss file events that occur within a single 'tick'. Fix this by instead tracking full file identity. Since we should have only a few relatively small files in tests, the additional overhead should be small. For golang/go#43554 Change-Id: I05a48567f83007ff2278145638547c6ebb2523fd Reviewed-on: https://go-review.googlesource.com/c/tools/+/283052 Run-TryBot: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org> Trust: Robert Findley <rfindley@google.com>
1 parent 45115c1 commit 7646fae

File tree

3 files changed

+32
-17
lines changed

3 files changed

+32
-17
lines changed

internal/lsp/fake/sandbox.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,11 @@ func Tempdir(txt string) (string, error) {
146146
if err != nil {
147147
return "", err
148148
}
149-
if err := writeTxtar(txt, RelativeTo(dir)); err != nil {
150-
return "", err
149+
files := unpackTxt(txt)
150+
for name, data := range files {
151+
if err := WriteFileData(name, data, RelativeTo(dir)); err != nil {
152+
return "", errors.Errorf("writing to tempdir: %w", err)
153+
}
151154
}
152155
return dir, nil
153156
}

internal/lsp/fake/workdir.go

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,13 @@ package fake
77
import (
88
"bytes"
99
"context"
10+
"crypto/sha256"
11+
"fmt"
1012
"io/ioutil"
1113
"os"
1214
"path/filepath"
1315
"strings"
1416
"sync"
15-
"time"
1617

1718
"golang.org/x/tools/internal/lsp/protocol"
1819
"golang.org/x/tools/internal/span"
@@ -82,7 +83,7 @@ type Workdir struct {
8283
watchers []func(context.Context, []FileEvent)
8384

8485
fileMu sync.Mutex
85-
files map[string]time.Time
86+
files map[string]string
8687
}
8788

8889
// NewWorkdir writes the txtar-encoded file data in txt to dir, and returns a
@@ -91,11 +92,18 @@ func NewWorkdir(dir string) *Workdir {
9192
return &Workdir{RelativeTo: RelativeTo(dir)}
9293
}
9394

95+
func hashFile(data []byte) string {
96+
return fmt.Sprintf("%x", sha256.Sum256(data))
97+
}
98+
9499
func (w *Workdir) writeInitialFiles(txt string) error {
95-
writeTxtar(txt, w.RelativeTo)
96-
// Poll to capture the current file state.
97-
if _, err := w.pollFiles(); err != nil {
98-
return errors.Errorf("polling files: %w", err)
100+
files := unpackTxt(txt)
101+
w.files = map[string]string{}
102+
for name, data := range files {
103+
w.files[name] = hashFile(data)
104+
if err := WriteFileData(name, data, w.RelativeTo); err != nil {
105+
return errors.Errorf("writing to workdir: %w", err)
106+
}
99107
}
100108
return nil
101109
}
@@ -256,10 +264,10 @@ func (w *Workdir) writeFile(ctx context.Context, path, content string) (FileEven
256264
}, nil
257265
}
258266

259-
// ListFiles lists files in the given directory, returning a map of relative
267+
// listFiles lists files in the given directory, returning a map of relative
260268
// path to modification time.
261-
func (w *Workdir) ListFiles(dir string) (map[string]time.Time, error) {
262-
files := make(map[string]time.Time)
269+
func (w *Workdir) listFiles(dir string) (map[string]string, error) {
270+
files := make(map[string]string)
263271
absDir := w.AbsPath(dir)
264272
if err := filepath.Walk(absDir, func(fp string, info os.FileInfo, err error) error {
265273
if err != nil {
@@ -269,7 +277,11 @@ func (w *Workdir) ListFiles(dir string) (map[string]time.Time, error) {
269277
return nil
270278
}
271279
path := w.RelPath(fp)
272-
files[path] = info.ModTime()
280+
data, err := ioutil.ReadFile(fp)
281+
if err != nil {
282+
return err
283+
}
284+
files[path] = hashFile(data)
273285
return nil
274286
}); err != nil {
275287
return nil, err
@@ -294,20 +306,20 @@ func (w *Workdir) pollFiles() ([]FileEvent, error) {
294306
w.fileMu.Lock()
295307
defer w.fileMu.Unlock()
296308

297-
files, err := w.ListFiles(".")
309+
files, err := w.listFiles(".")
298310
if err != nil {
299311
return nil, err
300312
}
301313
var evts []FileEvent
302314
// Check which files have been added or modified.
303-
for path, mtime := range files {
304-
oldmtime, ok := w.files[path]
315+
for path, hash := range files {
316+
oldhash, ok := w.files[path]
305317
delete(w.files, path)
306318
var typ protocol.FileChangeType
307319
switch {
308320
case !ok:
309321
typ = protocol.Created
310-
case oldmtime != mtime:
322+
case oldhash != hash:
311323
typ = protocol.Changed
312324
default:
313325
continue

internal/lsp/fake/workdir_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func TestWorkdir_ListFiles(t *testing.T) {
104104
defer cleanup()
105105

106106
checkFiles := func(dir string, want []string) {
107-
files, err := wd.ListFiles(dir)
107+
files, err := wd.listFiles(dir)
108108
if err != nil {
109109
t.Fatal(err)
110110
}

0 commit comments

Comments
 (0)