From 6d54a114ae7899d15fa868345419bb528a92af56 Mon Sep 17 00:00:00 2001 From: "Mr. Outis" Date: Mon, 26 Nov 2018 13:25:54 -0600 Subject: [PATCH] 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 | 33 ++++++++++++++++++++++----------- tests/test_checkout.py | 13 ++++++++----- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/dvc/remote/local.py b/dvc/remote/local.py index 40e54372d..b59db25f3 100644 --- a/dvc/remote/local.py +++ b/dvc/remote/local.py @@ -293,9 +293,11 @@ class RemoteLOCAL(RemoteBase): 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 @@ class RemoteLOCAL(RemoteBase): 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 @@ class RemoteLOCAL(RemoteBase): 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 @@ class RemoteLOCAL(RemoteBase): 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 @@ class RemoteLOCAL(RemoteBase): " 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 diff --git a/tests/test_checkout.py b/tests/test_checkout.py index 441f9f45c..72473b40b 100644 --- a/tests/test_checkout.py +++ b/tests/test_checkout.py @@ -162,9 +162,12 @@ class TestRemoveFilesWhenCheckout(CheckoutBase): 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 @@ class TestCheckoutCleanWorkingDir(CheckoutBase): 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 @@ class TestCheckoutCleanWorkingDir(CheckoutBase): 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)