diff --git a/dvc/remote/base.py b/dvc/remote/base.py index ea620eeed..541f6c6ed 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -372,7 +372,6 @@ class RemoteBASE(object): self.state.save_link(path_info) self.state.save(cache_info, checksum) self.state.save(path_info, checksum) - return dir_info def is_empty(self, path_info): return False @@ -543,10 +542,11 @@ class RemoteBASE(object): return False for entry in self.get_dir_cache(checksum): - checksum = entry[self.PARAM_CHECKSUM] - if self.changed_cache_file(checksum): + entry_checksum = entry[self.PARAM_CHECKSUM] + if self.changed_cache_file(entry_checksum): return True + self._update_unpacked_dir(checksum) return False def changed_cache(self, checksum): @@ -639,7 +639,6 @@ class RemoteBASE(object): self.state.save_link(path_info) self.state.save(path_info, checksum) - return dir_info def _remove_redundant_files(self, path_info, dir_info, force): existing_files = set( @@ -714,3 +713,6 @@ class RemoteBASE(object): def _changed_unpacked_dir(self, checksum): return True + + def _update_unpacked_dir(self, checksum): + pass diff --git a/dvc/remote/local/__init__.py b/dvc/remote/local/__init__.py index 038863adc..c65a5987c 100644 --- a/dvc/remote/local/__init__.py +++ b/dvc/remote/local/__init__.py @@ -237,7 +237,8 @@ class RemoteLOCAL(RemoteBASE): if path_info.scheme != "local": raise NotImplementedError - remove(path_info.path) + if self.exists(path_info): + remove(path_info.path) def move(self, from_info, to_info): if from_info.scheme != "local" or to_info.scheme != "local": @@ -605,24 +606,28 @@ class RemoteLOCAL(RemoteBASE): return unpacked_dir_info - def _update_unpacked_dir(self, checksum, dir_info): + def _path_info_changed(self, path_info): + if self.exists(path_info) and self.state.get(path_info): + return False + return True + + def _update_unpacked_dir(self, checksum): unpacked_dir_info = self._get_unpacked_dir_path_info(checksum) - if self.exists(unpacked_dir_info): - if self.state.get(unpacked_dir_info): - return + if not self._path_info_changed(unpacked_dir_info): + return - shutil.rmtree(unpacked_dir_info.path) + self.remove(unpacked_dir_info) try: + dir_info = self.get_dir_cache(checksum) self._create_unpacked_dir(checksum, dir_info, unpacked_dir_info) except Exception: logger.warning( "Could not create '{}'".format(unpacked_dir_info.path) ) - if self.exists(unpacked_dir_info): - self.remove(unpacked_dir_info) + self.remove(unpacked_dir_info) def _create_unpacked_dir(self, checksum, dir_info, unpacked_dir_info): self.makedirs(unpacked_dir_info) @@ -645,18 +650,16 @@ class RemoteLOCAL(RemoteBASE): return not self.state.get(status_unpacked_dir_info) def _save_dir(self, path_info, checksum): - dir_info = super(RemoteLOCAL, self)._save_dir(path_info, checksum) - self._update_unpacked_dir(checksum, dir_info) - return dir_info + super(RemoteLOCAL, self)._save_dir(path_info, checksum) + self._update_unpacked_dir(checksum) def _checkout_dir( self, path_info, checksum, force, progress_callback=None ): - dir_info = super(RemoteLOCAL, self)._checkout_dir( + super(RemoteLOCAL, self)._checkout_dir( path_info, checksum, force, progress_callback ) - self._update_unpacked_dir(checksum, dir_info) - return dir_info + self._update_unpacked_dir(checksum) def extract_used_local_checksums(self, cinfos): used = super(RemoteLOCAL, self).extract_used_local_checksums(cinfos) diff --git a/tests/func/remote/test_local.py b/tests/func/remote/test_local.py index 2cbbbadd0..488abcb72 100644 --- a/tests/func/remote/test_local.py +++ b/tests/func/remote/test_local.py @@ -15,8 +15,8 @@ from tests.utils import trees_equal def test_should_create_cache_status_check_dir_on_add(dvc_repo, repo_dir): stages = dvc_repo.add(repo_dir.DATA_DIR) - unpacked_dir = ( - stages[0].outs[0].cache_path + RemoteLOCAL.UNPACKED_DIR_SUFFIX + unpacked_dir = RemoteLOCAL._append_unpacked_suffix( + stages[0].outs[0].cache_path ) assert os.path.exists(unpacked_dir) @@ -42,8 +42,9 @@ def test_should_create_cache_status_check_dir_on_checkout(dvc_repo, repo_dir): dvc_repo = Repo(dvc_repo.dvc_dir) stages = dvc_repo.add(repo_dir.DATA_DIR) - unpacked_dir = ( - stages[0].outs[0].cache_path + RemoteLOCAL.UNPACKED_DIR_SUFFIX + + unpacked_dir = RemoteLOCAL._append_unpacked_suffix( + stages[0].outs[0].cache_path ) assert dvc_repo.push() == 1 @@ -69,3 +70,19 @@ def test_should_not_fail_add_on_unpacked_dir_creation_exception( ): ret = main(["add", repo_dir.DATA_DIR]) assert ret == 0 + + +def test_should_create_unpacked_dir_on_status_check(dvc_repo, repo_dir): + stages = dvc_repo.add(repo_dir.DATA_DIR) + assert len(stages) == 1 + + unpacked_dir = RemoteLOCAL._append_unpacked_suffix( + stages[0].outs[0].cache_path + ) + + assert os.path.exists(unpacked_dir) + shutil.rmtree(unpacked_dir) + assert not os.path.exists(unpacked_dir) + + assert dvc_repo.status("{}.dvc".format(repo_dir.DATA_DIR)) == {} + assert os.path.exists(unpacked_dir)