From 7505ea79a0082601f4ab903e04c4f98b2bbd2a9c Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Sat, 4 Apr 2020 14:17:16 +0200 Subject: [PATCH] lib/osutil: Don't remove before rename on Windows (ref #6493) (#6495) This was needed in ancient times but not currently. --- lib/osutil/atomic.go | 11 ----------- lib/osutil/atomic_test.go | 11 ++++++++++- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/osutil/atomic.go b/lib/osutil/atomic.go index 9618b724c..939fefdd1 100644 --- a/lib/osutil/atomic.go +++ b/lib/osutil/atomic.go @@ -9,7 +9,6 @@ package osutil import ( "errors" "path/filepath" - "runtime" "github.com/syncthing/syncthing/lib/fs" ) @@ -93,16 +92,6 @@ func (w *AtomicWriter) Close() error { return err } - // Remove the destination file, on Windows only. If it fails, and not due - // to the file not existing, we won't be able to complete the rename - // either. Return this error because it may be more informative. On non- - // Windows we want the atomic rename behavior so we don't attempt remove. - if runtime.GOOS == "windows" { - if err := w.fs.Remove(w.path); err != nil && !fs.IsNotExist(err) { - return err - } - } - if err := w.fs.Rename(w.next.Name(), w.path); err != nil { w.err = err return err diff --git a/lib/osutil/atomic_test.go b/lib/osutil/atomic_test.go index 0fe3415a3..9ee30064c 100644 --- a/lib/osutil/atomic_test.go +++ b/lib/osutil/atomic_test.go @@ -52,6 +52,15 @@ func TestCreateAtomicCreate(t *testing.T) { } func TestCreateAtomicReplace(t *testing.T) { + testCreateAtomicReplace(t, 0644) +} +func TestCreateAtomicReplaceReadOnly(t *testing.T) { + testCreateAtomicReplace(t, 0400) +} + +func testCreateAtomicReplace(t *testing.T, oldPerms os.FileMode) { + t.Helper() + os.RemoveAll("testdata") defer os.RemoveAll("testdata") @@ -59,7 +68,7 @@ func TestCreateAtomicReplace(t *testing.T) { t.Fatal(err) } - if err := ioutil.WriteFile("testdata/file", []byte("some old data"), 0644); err != nil { + if err := ioutil.WriteFile("testdata/file", []byte("some old data"), oldPerms); err != nil { t.Fatal(err) }