remote: create unpacked dir after status check if needed

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

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)

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)