From beb32b387e38e7941b9505f1e3273b3a10fa0415 Mon Sep 17 00:00:00 2001 From: Andrew Grigorev Date: Tue, 19 Mar 2019 19:53:28 +0300 Subject: [PATCH] unprotect: copy should preceed the move It is better to make a file copy before moving it, since move is a nearly-atomic operation and copy is a long one. Also the biggest problem here is that if some application would access the file during the process of `copyfile` then it would get the part of file. --- dvc/repo/unprotect.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/dvc/repo/unprotect.py b/dvc/repo/unprotect.py index ddb69809d..3263abf33 100644 --- a/dvc/repo/unprotect.py +++ b/dvc/repo/unprotect.py @@ -13,13 +13,16 @@ from dvc.exceptions import DvcException def _unprotect_file(path): if System.is_symlink(path) or System.is_hardlink(path): logger.debug("Unprotecting '{}'".format(path)) - tmp = os.path.join(os.path.dirname(path), "." + str(uuid.uuid4())) - move(path, tmp) - copyfile(tmp, path) + # The operations order is important here - if some application would + # access the file during the process of copyfile then it would get + # only the part of file. So, at first, the file should be copied with + # the temporary name, and then original file should be replaced by new. + copyfile(path, tmp) + remove(path) + move(tmp, path) - remove(tmp) else: logger.debug( "Skipping copying for '{}', since it is not "