From 5d27fc48a8884dcda2b59e669cc523770081c6e0 Mon Sep 17 00:00:00 2001 From: Krasi Georgiev <8903888+krasi-georgiev@users.noreply.github.com> Date: Wed, 19 Jun 2019 12:10:51 +0300 Subject: [PATCH 1/3] fileutil.Replace - remove destination only when a directory. In cases where a rename fails the fileutil.Replace would delete the source files/folder. There is no easy way to make directory renaming atomic, but for files os.Rename is atomic and replaced the destination file so there is no need to remove the destination file explicitly. Signed-off-by: Krasi Georgiev <8903888+krasi-georgiev@users.noreply.github.com> --- fileutil/fileutil.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/fileutil/fileutil.go b/fileutil/fileutil.go index c55a2b81d..25de316f8 100644 --- a/fileutil/fileutil.go +++ b/fileutil/fileutil.go @@ -128,9 +128,20 @@ func Rename(from, to string) error { // Replace moves a file or directory to a new location and deletes any previous data. // It is not atomic. func Replace(from, to string) error { - if err := os.RemoveAll(to); err != nil { - return err + // Remove destionation only if it is a dir. + // Otherwise os.Rename replaces the destination file and is atomic. + { + f, err := os.Stat(to) + if err != nil { + return err + } + if f.IsDir() { + if err := os.RemoveAll(to); err != nil { + return err + } + } } + if err := os.Rename(from, to); err != nil { return err } From ef9b3461c5e562ac5e3954487990bbff34accd3a Mon Sep 17 00:00:00 2001 From: Krasi Georgiev <8903888+krasi-georgiev@users.noreply.github.com> Date: Wed, 19 Jun 2019 14:27:19 +0300 Subject: [PATCH 2/3] fixed the err handling and better comment Signed-off-by: Krasi Georgiev <8903888+krasi-georgiev@users.noreply.github.com> --- fileutil/fileutil.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/fileutil/fileutil.go b/fileutil/fileutil.go index 25de316f8..214269401 100644 --- a/fileutil/fileutil.go +++ b/fileutil/fileutil.go @@ -128,16 +128,15 @@ func Rename(from, to string) error { // Replace moves a file or directory to a new location and deletes any previous data. // It is not atomic. func Replace(from, to string) error { - // Remove destionation only if it is a dir. - // Otherwise os.Rename replaces the destination file and is atomic. + // Remove destination only if it is a dir otherwise leave it to os.Rename + // as it replaces the destination when it is file and is atomic. { f, err := os.Stat(to) - if err != nil { - return err - } - if f.IsDir() { - if err := os.RemoveAll(to); err != nil { - return err + if !os.IsNotExist(err) { + if err == nil && f.IsDir() { + if err := os.RemoveAll(to); err != nil { + return err + } } } } From 251c3a4d645d9740416fb4300101ca15b2360685 Mon Sep 17 00:00:00 2001 From: Krasi Georgiev <8903888+krasi-georgiev@users.noreply.github.com> Date: Mon, 24 Jun 2019 11:51:49 +0300 Subject: [PATCH 3/3] comment Signed-off-by: Krasi Georgiev <8903888+krasi-georgiev@users.noreply.github.com> --- fileutil/fileutil.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fileutil/fileutil.go b/fileutil/fileutil.go index 214269401..4088f522a 100644 --- a/fileutil/fileutil.go +++ b/fileutil/fileutil.go @@ -129,7 +129,7 @@ func Rename(from, to string) error { // It is not atomic. func Replace(from, to string) error { // Remove destination only if it is a dir otherwise leave it to os.Rename - // as it replaces the destination when it is file and is atomic. + // as it replaces the destination file and is atomic. { f, err := os.Stat(to) if !os.IsNotExist(err) {