local: Don't prompt at removal if file is already cached (#1367)

* local: Don't prompt at removal if file is already cached

Fix #1366

* [fix] Do `remove` at `save_remove`

* Use `state.update` to compute checksum instead of `save_info`

* PEP8 compilant

dvc/remote/local.py

@@ -293,9 +293,11 @@ def checkout(self, path_info, checksum_info, force=False):
         Logger.info(msg.format(os.path.relpath(path), md5))
 
         if not self.is_dir_cache(cache):
-
             if os.path.exists(path):
-                remove(path) if force else self._safe_remove(path)
+                if force or self._already_cached(path):
+                    remove(path)
+                else:
+                    self._safe_remove(path)
 
             self.link(cache, path)
             self.state.update_link(path)
@@ -319,17 +321,16 @@ def checkout(self, path_info, checksum_info, force=False):
             p = os.path.join(path, relpath)
             c = self.get(m)
 
-            entry_info = {
-                'scheme': path_info['scheme'],
-                self.PARAM_PATH: p,
-            }
+            entry_info = {'scheme': path_info['scheme'], self.PARAM_PATH: p}
 
-            entry_checksum_info = {
-                self.PARAM_MD5: m,
-            }
+            entry_checksum_info = {self.PARAM_MD5: m}
 
             if self.changed(entry_info, entry_checksum_info):
-                remove(p) if force else self._safe_remove(p)
+                if force or self._already_cached(p):
+                    remove(p)
+                else:
+                    self._safe_remove(p)
+
                 self.link(c, p)
 
             if bar:
@@ -342,6 +343,11 @@ def checkout(self, path_info, checksum_info, force=False):
         if bar:
             progress.finish_target(dir_relpath)
 
+    def _already_cached(self, path):
+        current_md5 = self.state.update(path)
+
+        return not self.changed_cache(current_md5)
+
     def _discard_working_directory_changes(self, path, dir_info, force=False):
         working_dir_files = set(
             os.path.join(root, file)
@@ -357,7 +363,10 @@ def _discard_working_directory_changes(self, path, dir_info, force=False):
         delta = working_dir_files - cached_files
 
         for file in delta:
-            remove(file) if force else self._safe_remove(file)
+            if force or self._already_cached(file):
+                remove(file)
+            else:
+                self._safe_remove(file)
 
     def _safe_remove(self, file):
         msg = (
@@ -373,6 +382,8 @@ def _safe_remove(self, file):
                                " from the user. Use '-f' to force."
                                .format(file))
 
+        remove(file)
+
     def _move(self, inp, outp):
         # moving in two stages to make last the move atomic in
         # case inp and outp are in different filesystems

tests/test_checkout.py

@@ -162,9 +162,12 @@ def test(self):
 
 
 class TestCheckoutCleanWorkingDir(CheckoutBase):
-    def test(self):
+    @patch('dvc.prompt.Prompt.prompt')
+    def test(self, mock_prompt):
         from tests.test_data_cloud import sleep
 
+        mock_prompt.return_value = True
+
         stages = self.dvc.add(self.DATA_DIR)
         stage = stages[0]
 
@@ -176,7 +179,7 @@ def test(self):
 
         sleep()
 
-        ret = main(['checkout', '--force', stage.relpath])
+        ret = main(['checkout', stage.relpath])
         self.assertEqual(ret, 0)
         self.assertFalse(os.path.exists(working_dir_change))
 
@@ -189,15 +192,15 @@ def test_force(self, mock_prompt):
         stages = self.dvc.add(self.DATA_DIR)
         self.assertEqual(len(stages), 1)
         stage = stages[0]
-        
+
         sleep()
-        
+
         working_dir_change = os.path.join(self.DATA_DIR, 'not_cached.txt')
         with open(working_dir_change, 'w') as f:
             f.write('not_cached')
 
         sleep()
-            
+
         ret = main(['checkout', stage.relpath])
         self.assertNotEqual(ret, 0)