diff --git a/dvc/remote/base.py b/dvc/remote/base.py index 7f985b3f0..02c0773e1 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -758,45 +758,20 @@ class RemoteBASE(object): self.remove(path_info) def _checkout_file( - self, - path_info, - checksum, - force, - progress_callback=None, - save_link=True, + self, path_info, checksum, force, progress_callback=None ): - # NOTE: In case if path_info is already cached and path_info's - # link type matches cache link type, we would like to avoid - # relinking. - if self.changed( - path_info, {self.PARAM_CHECKSUM: checksum} - ) or not self._link_matches(path_info): + cache_info = self.checksum_to_path_info(checksum) + if self.exists(path_info): + msg = "data '{}' exists. Removing before checkout." + logger.warning(msg.format(str(path_info))) self.safe_remove(path_info, force=force) - cache_info = self.checksum_to_path_info(checksum) - self.link(cache_info, path_info) - - if save_link: - self.state.save_link(path_info) - - self.state.save(path_info, checksum) - else: - # NOTE: performing (un)protection costs us +/- the same as checking - # if path_info is protected. Instead of implementing logic, - # just (un)protect according to self.protected. - if self.protected: - self.protect(path_info) - else: - # NOTE dont allow copy, because we checked before that link - # type matches cache, and we don't want data duplication - self.unprotect(path_info, allow_copy=False) - + self.link(cache_info, path_info) + self.state.save_link(path_info) + self.state.save(path_info, checksum) if progress_callback: progress_callback(str(path_info)) - def _link_matches(self, path_info): - return True - def makedirs(self, path_info): """Optional: Implement only if the remote needs to create directories before copying/linking/moving data @@ -818,14 +793,17 @@ class RemoteBASE(object): for entry in dir_info: relative_path = entry[self.PARAM_RELPATH] entry_checksum = entry[self.PARAM_CHECKSUM] + entry_cache_info = self.checksum_to_path_info(entry_checksum) entry_info = path_info / relative_path - self._checkout_file( - entry_info, - entry_checksum, - force, - progress_callback, - save_link=False, - ) + + entry_checksum_info = {self.PARAM_CHECKSUM: entry_checksum} + if self.changed(entry_info, entry_checksum_info): + if self.exists(entry_info): + self.safe_remove(entry_info, force=force) + self.link(entry_cache_info, entry_info) + self.state.save(entry_info, entry_checksum) + if progress_callback: + progress_callback(str(entry_info)) self._remove_redundant_files(path_info, dir_info, force) @@ -904,7 +882,7 @@ class RemoteBASE(object): return 1 @staticmethod - def unprotect(path_info, allow_copy=True): + def unprotect(path_info): pass def _get_unpacked_dir_names(self, checksums): diff --git a/dvc/remote/local.py b/dvc/remote/local.py index ff88dfdcc..876eae11f 100644 --- a/dvc/remote/local.py +++ b/dvc/remote/local.py @@ -394,22 +394,19 @@ class RemoteLOCAL(RemoteBASE): logger.warning(msg) @staticmethod - def _unprotect_file(path, allow_copy=True): + def _unprotect_file(path): if System.is_symlink(path) or System.is_hardlink(path): - if allow_copy: - logger.debug("Unprotecting '{}'".format(path)) - tmp = os.path.join(os.path.dirname(path), "." + str(uuid())) + logger.debug("Unprotecting '{}'".format(path)) + tmp = os.path.join(os.path.dirname(path), "." + str(uuid())) - # 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, name="Unprotecting '{}'".format(relpath(path)) - ) - remove(path) - os.rename(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, name="Unprotecting '{}'".format(relpath(path))) + remove(path) + os.rename(tmp, path) else: logger.debug( @@ -419,11 +416,11 @@ class RemoteLOCAL(RemoteBASE): os.chmod(path, os.stat(path).st_mode | stat.S_IWRITE) - def _unprotect_dir(self, path, allow_copy=True): + def _unprotect_dir(self, path): for fname in walk_files(path, self.repo.dvcignore): - self._unprotect_file(fname, allow_copy) + RemoteLOCAL._unprotect_file(fname) - def unprotect(self, path_info, allow_copy=True): + def unprotect(self, path_info): path = path_info.fspath if not os.path.exists(path): raise DvcException( @@ -431,9 +428,9 @@ class RemoteLOCAL(RemoteBASE): ) if os.path.isdir(path): - self._unprotect_dir(path, allow_copy) + self._unprotect_dir(path) else: - self._unprotect_file(path, allow_copy) + RemoteLOCAL._unprotect_file(path) @staticmethod def protect(path_info): @@ -525,18 +522,3 @@ class RemoteLOCAL(RemoteBASE): self.cache_type_confirmed = True return self.cache_types[0] - - def _link_matches(self, path_info): - is_hardlink = System.is_hardlink(path_info) - is_symlink = System.is_symlink(path_info) - is_copy_or_reflink = not is_hardlink and not is_symlink - - cache_type = self._get_cache_type(path_info) - - if cache_type == "symlink": - return is_symlink - - if cache_type == "hardlink": - return is_hardlink - - return is_copy_or_reflink