Skip to content

Commit aee3994

Browse files
Bryan C. Millsgopherbot
Bryan C. Mills
authored andcommitted
gopls/internal/lsp/fake: in (*Workdir).RenameFile, fall back to read + write
os.Rename cannot portably rename files across directories; notably, that operation fails with "invalid argument" on some Plan 9 filesystems. Fixes golang/go#57111. Change-Id: Ifd108bb58fa968fc2c8a21b99211a904d6d3d4e6 Reviewed-on: https://go-review.googlesource.com/c/tools/+/455515 Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com>
1 parent fe60148 commit aee3994

File tree

1 file changed

+78
-3
lines changed

1 file changed

+78
-3
lines changed

gopls/internal/lsp/fake/workdir.go

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -330,12 +330,87 @@ func (w *Workdir) fileEvent(path string, changeType protocol.FileChangeType) Fil
330330

331331
// RenameFile performs an on disk-renaming of the workdir-relative oldPath to
332332
// workdir-relative newPath.
333+
//
334+
// oldPath must either be a regular file or in the same directory as newPath.
333335
func (w *Workdir) RenameFile(ctx context.Context, oldPath, newPath string) error {
334336
oldAbs := w.AbsPath(oldPath)
335337
newAbs := w.AbsPath(newPath)
336338

337-
if err := robustio.Rename(oldAbs, newAbs); err != nil {
338-
return err
339+
w.fileMu.Lock()
340+
defer w.fileMu.Unlock()
341+
342+
// For os.Rename, “OS-specific restrictions may apply when oldpath and newpath
343+
// are in different directories.” If that applies here, we may fall back to
344+
// ReadFile, WriteFile, and RemoveFile to perform the rename non-atomically.
345+
//
346+
// However, the fallback path only works for regular files: renaming a
347+
// directory would be much more complex and isn't needed for our tests.
348+
fallbackOk := false
349+
if filepath.Dir(oldAbs) != filepath.Dir(newAbs) {
350+
fi, err := os.Stat(oldAbs)
351+
if err == nil && !fi.Mode().IsRegular() {
352+
return &os.PathError{
353+
Op: "RenameFile",
354+
Path: oldPath,
355+
Err: fmt.Errorf("%w: file is not regular and not in the same directory as %s", os.ErrInvalid, newPath),
356+
}
357+
}
358+
fallbackOk = true
359+
}
360+
361+
var renameErr error
362+
const debugFallback = false
363+
if fallbackOk && debugFallback {
364+
renameErr = fmt.Errorf("%w: debugging fallback path", os.ErrInvalid)
365+
} else {
366+
renameErr = robustio.Rename(oldAbs, newAbs)
367+
}
368+
if renameErr != nil {
369+
if !fallbackOk {
370+
return renameErr // The OS-specific Rename restrictions do not apply.
371+
}
372+
373+
content, err := w.ReadFile(oldPath)
374+
if err != nil {
375+
// If we can't even read the file, the error from Rename may be accurate.
376+
return renameErr
377+
}
378+
fi, err := os.Stat(newAbs)
379+
if err == nil {
380+
if fi.IsDir() {
381+
// “If newpath already exists and is not a directory, Rename replaces it.”
382+
// But if it is a directory, maybe not?
383+
return renameErr
384+
}
385+
// On most platforms, Rename replaces the named file with a new file,
386+
// rather than overwriting the existing file it in place. Mimic that
387+
// behavior here.
388+
if err := robustio.RemoveAll(newAbs); err != nil {
389+
// Maybe we don't have permission to replace newPath?
390+
return renameErr
391+
}
392+
} else if !os.IsNotExist(err) {
393+
// If the destination path already exists or there is some problem with it,
394+
// the error from Rename may be accurate.
395+
return renameErr
396+
}
397+
if writeErr := WriteFileData(newPath, []byte(content), w.RelativeTo); writeErr != nil {
398+
// At this point we have tried to actually write the file.
399+
// If it still doesn't exist, assume that the error from Rename was accurate:
400+
// for example, maybe we don't have permission to create the new path.
401+
// Otherwise, return the error from the write, which may indicate some
402+
// other problem (such as a full disk).
403+
if _, statErr := os.Stat(newAbs); !os.IsNotExist(statErr) {
404+
return writeErr
405+
}
406+
return renameErr
407+
}
408+
if err := robustio.RemoveAll(oldAbs); err != nil {
409+
// If we failed to remove the old file, that may explain the Rename error too.
410+
// Make a best effort to back out the write to the new path.
411+
robustio.RemoveAll(newAbs)
412+
return renameErr
413+
}
339414
}
340415

341416
// Send synthetic file events for the renaming. Renamed files are handled as
@@ -346,7 +421,7 @@ func (w *Workdir) RenameFile(ctx context.Context, oldPath, newPath string) error
346421
w.fileEvent(newPath, protocol.Created),
347422
}
348423
w.sendEvents(ctx, events)
349-
424+
delete(w.files, oldPath)
350425
return nil
351426
}
352427

0 commit comments

Comments
 (0)