This repository has been archived by the owner on Aug 13, 2019. It is now read-only.
fileutil.Replace - remove destination only when a directory. #636
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 <[email protected]>
bwplotka
reviewed
Jun 19, 2019
yes probably broken for windows, but will wait for some Windows user to contribute the windows changes. |
Signed-off-by: Krasi Georgiev <[email protected]>
Signed-off-by: Krasi Georgiev <[email protected]>
Signed-off-by: Krasi Georgiev <[email protected]>
bwplotka
approved these changes
Jun 24, 2019
There was a problem hiding this comment.
yes probably broken for windows, but will wait for some Windows user to contribute the windows changes
Not sure if that's right approach, we could ask someone to test it / add windows machines at some point to CI. However since os.Rename has this specified in method comment, I think it is likely it works (: so LGTM
// If newpath already exists and is not a directory, Rename replaces it.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In cases where a rename fails the
fileutil.Replacewould delete thesource files/folder.
There is no easy way to make directory renaming atomic, but for files
os.Renameis atomic and replaced the destination file so there is noneed to remove the destination file explicitly.
this was caught while working on the FS errors injections on rename in #583 with
block.Delete. Before this change a failure during a rename would cause deleting the source meta and tombstone files.