From d406361bd33c303b967f8d7924c22ea9c7c3b707 Mon Sep 17 00:00:00 2001 From: "Mr. Outis" Date: Wed, 14 Nov 2018 13:39:02 -0600 Subject: [PATCH 1/4] checkout: Prompt when deleting files from working dir Close #1328 --- dvc/cli.py | 7 +++++++ dvc/command/checkout.py | 5 +++-- dvc/output/gs.py | 2 +- dvc/output/hdfs.py | 2 +- dvc/output/local.py | 6 ++++-- dvc/output/s3.py | 2 +- dvc/output/ssh.py | 2 +- dvc/project.py | 4 ++-- dvc/remote/base.py | 2 +- dvc/remote/local.py | 19 ++++++++++++++++--- dvc/stage.py | 4 ++-- tests/test_checkout.py | 35 +++++++++++++++++++++++++++-------- 12 files changed, 66 insertions(+), 24 deletions(-) diff --git a/dvc/cli.py b/dvc/cli.py index 838d2c0b5..ceb0cf023 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -192,6 +192,13 @@ def parse_args(argv=None): default=False, help='Checkout all dependencies of the ' 'specified target.') + checkout_parser.add_argument( + '-f', + '--force', + action='store_true', + default=False, + help='Do not prompt when removing ' + 'working directory files.') checkout_parser.add_argument( 'targets', nargs='*', diff --git a/dvc/command/checkout.py b/dvc/command/checkout.py index 097dfc7e2..55f6c53cc 100644 --- a/dvc/command/checkout.py +++ b/dvc/command/checkout.py @@ -4,9 +4,10 @@ from dvc.command.base import CmdBase class CmdCheckout(CmdBase): def run(self): if not self.args.targets: - self.project.checkout() + self.project.checkout(force=self.args.force) else: for target in self.args.targets: self.project.checkout(target=target, - with_deps=self.args.with_deps) + with_deps=self.args.with_deps, + force=self.args.force) return 0 diff --git a/dvc/output/gs.py b/dvc/output/gs.py index e85330999..405e5a8f8 100644 --- a/dvc/output/gs.py +++ b/dvc/output/gs.py @@ -35,7 +35,7 @@ class OutputGS(DependencyGS): return False - def checkout(self): + def checkout(self, force=False): if not self.use_cache: return diff --git a/dvc/output/hdfs.py b/dvc/output/hdfs.py index 19cca3585..85b435c4f 100644 --- a/dvc/output/hdfs.py +++ b/dvc/output/hdfs.py @@ -35,7 +35,7 @@ class OutputHDFS(DependencyHDFS): return False - def checkout(self): + def checkout(self, force=False): if not self.use_cache: return diff --git a/dvc/output/local.py b/dvc/output/local.py index 9bdf35c7e..ddf6973d3 100644 --- a/dvc/output/local.py +++ b/dvc/output/local.py @@ -60,11 +60,13 @@ class OutputLOCAL(DependencyLOCAL): return self.project.cache.local.changed(self.path_info, self.info) - def checkout(self): + def checkout(self, force=False): if not self.use_cache: return - self.project.cache.local.checkout(self.path_info, self.info) + self.project.cache.local.checkout(self.path_info, + self.info, + force=force) def _verify_metric(self): if not self.metric: diff --git a/dvc/output/s3.py b/dvc/output/s3.py index e3e2babb2..1bbba03bd 100644 --- a/dvc/output/s3.py +++ b/dvc/output/s3.py @@ -35,7 +35,7 @@ class OutputS3(DependencyS3): return False - def checkout(self): + def checkout(self, force=False): if not self.use_cache: return diff --git a/dvc/output/ssh.py b/dvc/output/ssh.py index 4b4f6c298..4238c1f3d 100644 --- a/dvc/output/ssh.py +++ b/dvc/output/ssh.py @@ -35,7 +35,7 @@ class OutputSSH(DependencySSH): return False - def checkout(self): + def checkout(self, force=False): if not self.use_cache: return diff --git a/dvc/project.py b/dvc/project.py index 67b3da604..ac0a86873 100644 --- a/dvc/project.py +++ b/dvc/project.py @@ -466,7 +466,7 @@ class Project(object): used.append(out.path) self.state.remove_unused_links(used) - def checkout(self, target=None, with_deps=False): + def checkout(self, target=None, with_deps=False, force=False): all_stages = self.active_stages() stages = all_stages @@ -482,7 +482,7 @@ class Project(object): 'not going to be checked out.' self.logger.warn(msg.format(stage.relpath)) - stage.checkout() + stage.checkout(force=force) def _get_pipeline(self, node): pipelines = list(filter(lambda g: node in g.nodes(), diff --git a/dvc/remote/base.py b/dvc/remote/base.py index 6e246d39e..39844aefa 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -92,7 +92,7 @@ class RemoteBase(object): def save(self, path_info): raise NotImplementedError - def checkout(self, path_info, checksum_info): + def checkout(self, path_info, checksum_info, force=False): raise NotImplementedError def download(self, from_infos, to_infos, no_progress_bar=False, name=None): diff --git a/dvc/remote/local.py b/dvc/remote/local.py index 8816b1307..cbfe51e59 100644 --- a/dvc/remote/local.py +++ b/dvc/remote/local.py @@ -268,7 +268,7 @@ class RemoteLOCAL(RemoteBase): def is_dir_cache(cls, cache): return cache.endswith(cls.MD5_DIR_SUFFIX) - def checkout(self, path_info, checksum_info): + def checkout(self, path_info, checksum_info, force=False): path = path_info['path'] md5 = checksum_info.get(self.PARAM_MD5) cache = self.get(md5) @@ -337,14 +337,14 @@ class RemoteLOCAL(RemoteBase): if bar: progress.update_target(dir_relpath, processed, dir_size) - self._discard_working_directory_changes(path, dir_info) + self._discard_working_directory_changes(path, dir_info, force=force) self.state.update_link(path) if bar: progress.finish_target(dir_relpath) - def _discard_working_directory_changes(self, path, dir_info): + def _discard_working_directory_changes(self, path, dir_info, force=False): working_dir_files = set( os.path.join(root, file) for root, _, files in os.walk(path) @@ -359,6 +359,19 @@ class RemoteLOCAL(RemoteBase): delta = working_dir_files - cached_files for file in delta: + msg = ( + 'File "{}" is going to be removed. ' + 'Are you sure you want to proceed?' + .format(file) + ) + + confirmed = force or self.project.prompt.prompt(msg, False) + + if not confirmed: + raise DvcException('Unable to remove {} without a confirmation' + " from the user. Use '-f' to force." + .format(file)) + remove(file) def _move(self, inp, outp): diff --git a/dvc/stage.py b/dvc/stage.py index da723731a..b9bc676aa 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -478,9 +478,9 @@ class Stage(object): if paths: raise MissingDataSource(paths) - def checkout(self): + def checkout(self, force=False): for out in self.outs: - out.checkout() + out.checkout(force=force) def _status(self, entries, name): ret = {} diff --git a/tests/test_checkout.py b/tests/test_checkout.py index fd242fe19..127a68a44 100644 --- a/tests/test_checkout.py +++ b/tests/test_checkout.py @@ -13,7 +13,9 @@ from tests.basic_env import TestDvc from tests.test_repro import TestRepro from dvc.stage import Stage from dvc.remote.local import RemoteLOCAL +from dvc.exceptions import DvcException +from mock import patch class TestCheckout(TestRepro): def setUp(self): @@ -162,22 +164,39 @@ class TestRemoveFilesWhenCheckout(CheckoutBase): class TestCheckoutCleanWorkingDir(CheckoutBase): def test(self): from tests.test_data_cloud import sleep - + stages = self.dvc.add(self.DATA_DIR) 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]) + + ret = main(['checkout', '--force', stage.relpath]) self.assertEqual(ret, 0) self.assertFalse(os.path.exists(working_dir_change)) + @patch('dvc.prompt.Prompt.prompt') + def test_force(self, mock_prompt): + mock_prompt.return_value = False + + stages = self.dvc.add(self.DATA_DIR) + stage = stages[0] + + working_dir_change = os.path.join(self.DATA_DIR, 'not_cached.txt') + with open(working_dir_change, 'w') as f: + f.write('not_cached') + + ret = main(['checkout', stage.relpath]) + + mock_prompt.assert_called() + self.assertNotEqual(ret, 0) + self.assertRaises(DvcException) + class TestCheckoutSelectiveRemove(CheckoutBase): def test(self): @@ -193,16 +212,16 @@ class TestCheckoutSelectiveRemove(CheckoutBase): staged_files = self.outs_info(stage) sleep() - + os.remove(staged_files[0].path) - + sleep() - + ret = main(['checkout', stage.relpath]) self.assertEqual(ret, 0) sleep() - + checkedout_files = self.outs_info(stage) self.assertEqual(len(staged_files), len(checkedout_files)) From e57480eb461335e251e5f4eae7497157c44ee5d2 Mon Sep 17 00:00:00 2001 From: "Mr. Outis" Date: Fri, 16 Nov 2018 18:30:47 -0600 Subject: [PATCH 2/4] Always perfom a safe remove on checkout --- dvc/remote/local.py | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/dvc/remote/local.py b/dvc/remote/local.py index cbfe51e59..b969ee992 100644 --- a/dvc/remote/local.py +++ b/dvc/remote/local.py @@ -295,9 +295,7 @@ class RemoteLOCAL(RemoteBase): if not self.is_dir_cache(cache): if os.path.exists(path): - msg = u'File \'{}\' exists. Removing before checkout.' - Logger.warn(msg.format(os.path.relpath(path))) - remove(path) + remove(path) if force else self._safe_remove(path) self.link(cache, path) self.state.update_link(path) @@ -331,7 +329,7 @@ class RemoteLOCAL(RemoteBase): } if self.changed(entry_info, entry_checksum_info): - remove(p) + remove(p) if force else self._safe_remove(p) self.link(c, p) if bar: @@ -359,20 +357,21 @@ class RemoteLOCAL(RemoteBase): delta = working_dir_files - cached_files for file in delta: - msg = ( - 'File "{}" is going to be removed. ' - 'Are you sure you want to proceed?' - .format(file) - ) + remove(file) if force else self._safe_remove(file) - confirmed = force or self.project.prompt.prompt(msg, False) + def _safe_remove(self, file): + msg = ( + 'File "{}" is going to be removed. ' + 'Are you sure you want to proceed?' + .format(file) + ) - if not confirmed: - raise DvcException('Unable to remove {} without a confirmation' - " from the user. Use '-f' to force." - .format(file)) + confirmed = self.project.prompt.prompt(msg, False) - remove(file) + if not confirmed: + raise DvcException('Unable to remove {} without a confirmation' + " from the user. Use '-f' to force." + .format(file)) def _move(self, inp, outp): # moving in two stages to make last the move atomic in From 2906a31b66bebd536e15ac3db14d40c8bf470c3e Mon Sep 17 00:00:00 2001 From: "Mr. Outis" Date: Fri, 16 Nov 2018 19:05:53 -0600 Subject: [PATCH 3/4] tests: dvc checkout --force --- dvc/project.py | 2 +- tests/test_checkout.py | 32 ++++++++++++++++---------------- tests/test_metrics.py | 2 +- tests/test_repro.py | 2 +- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/dvc/project.py b/dvc/project.py index ac0a86873..69050f029 100644 --- a/dvc/project.py +++ b/dvc/project.py @@ -957,7 +957,7 @@ class Project(object): for fname, t, x in entries: if stage: - self.checkout(stage) + self.checkout(stage, force=True) rel = os.path.relpath(fname) metric = self._read_metric(fname, diff --git a/tests/test_checkout.py b/tests/test_checkout.py index 127a68a44..c42add4a8 100644 --- a/tests/test_checkout.py +++ b/tests/test_checkout.py @@ -36,7 +36,7 @@ class TestCheckout(TestRepro): shutil.rmtree(self.DATA_DIR) def test(self): - self.dvc.checkout() + self.dvc.checkout(force=True) self._test_checkout() def _test_checkout(self): @@ -46,10 +46,10 @@ class TestCheckout(TestRepro): class TestCheckoutSingleStage(TestCheckout): def test(self): - ret = main(['checkout', self.foo_stage.path]) + ret = main(['checkout', '--force', self.foo_stage.path]) self.assertEqual(ret, 0) - ret = main(['checkout', self.data_dir_stage.path]) + ret = main(['checkout', '--force', self.data_dir_stage.path]) self.assertEqual(ret, 0) self._test_checkout() @@ -64,7 +64,7 @@ class TestCheckoutCorruptedCacheFile(TestRepro): with open(cache, 'a') as fd: fd.write('1') - self.dvc.checkout() + self.dvc.checkout(force=True) self.assertFalse(os.path.isfile(self.FOO)) self.assertFalse(os.path.isfile(cache)) @@ -93,14 +93,14 @@ class TestCheckoutCorruptedCacheDir(TestDvc): with open(cache, 'w+') as fobj: fobj.write('1') - self.dvc.checkout() + self.dvc.checkout(force=True) self.assertFalse(os.path.exists(cache)) class TestCmdCheckout(TestCheckout): def test(self): - ret = main(['checkout']) + ret = main(['checkout', '--force']) self.assertEqual(ret, 0) self._test_checkout() @@ -146,7 +146,7 @@ class TestRemoveFilesWhenCheckout(CheckoutBase): # add the file into a separate branch self.dvc.scm.checkout(branch_1, True) - ret = main(['checkout']) + ret = main(['checkout', '--force']) self.assertEqual(ret, 0) self.commit_data_file(fname) @@ -156,7 +156,7 @@ class TestRemoveFilesWhenCheckout(CheckoutBase): # Make sure `dvc checkout` removes the file # self.dvc.checkout() - ret = main(['checkout']) + ret = main(['checkout', '--force']) self.assertEqual(ret, 0) self.assertFalse(os.path.exists(fname)) @@ -217,7 +217,7 @@ class TestCheckoutSelectiveRemove(CheckoutBase): sleep() - ret = main(['checkout', stage.relpath]) + ret = main(['checkout', '--force', stage.relpath]) self.assertEqual(ret, 0) sleep() @@ -269,12 +269,12 @@ class TestGitIgnoreWhenCheckout(CheckoutBase): self.commit_data_file(fname_master) self.dvc.scm.checkout(branch_1, True) - ret = main(['checkout']) + ret = main(['checkout', '--force']) self.assertEqual(ret, 0) self.commit_data_file(fname_branch) self.dvc.scm.checkout(branch_master) - ret = main(['checkout']) + ret = main(['checkout', '--force']) self.assertEqual(ret, 0) ignored = self.read_ignored() @@ -283,7 +283,7 @@ class TestGitIgnoreWhenCheckout(CheckoutBase): self.assertIn('/' + fname_master, ignored) self.dvc.scm.checkout(branch_1) - ret = main(['checkout']) + ret = main(['checkout', '--force']) self.assertEqual(ret, 0) ignored = self.read_ignored() self.assertIn('/' + fname_branch, ignored) @@ -300,7 +300,7 @@ class TestCheckoutMissingMd5InStageFile(TestRepro): with open(self.file1_stage, 'w') as fd: yaml.dump(d, fd) - self.dvc.checkout() + self.dvc.checkout(force=True) class TestCheckoutEmptyDir(TestDvc): @@ -317,7 +317,7 @@ class TestCheckoutEmptyDir(TestDvc): stage.outs[0].remove() self.assertFalse(os.path.exists(dname)) - self.dvc.checkout() + self.dvc.checkout(force=True) self.assertTrue(os.path.isdir(dname)) self.assertEqual(len(os.listdir(dname)), 0) @@ -332,7 +332,7 @@ class TestCheckoutNotCachedFile(TestDvc): deps=[self.FOO, self.CODE], outs_no_cache=['out']) - self.dvc.checkout() + self.dvc.checkout(force=True) class TestCheckoutWithDeps(TestRepro): @@ -343,7 +343,7 @@ class TestCheckoutWithDeps(TestRepro): self.assertFalse(os.path.exists(self.FOO)) self.assertFalse(os.path.exists(self.file1)) - ret = main(['checkout', self.file1_stage, '--with-deps']) + ret = main(['checkout', '--force', self.file1_stage, '--with-deps']) self.assertEqual(ret, 0) self.assertTrue(os.path.exists(self.FOO)) diff --git a/tests/test_metrics.py b/tests/test_metrics.py index 3aa814221..02a7475cf 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -214,7 +214,7 @@ class TestNoMetrics(TestDvc): class TestCachedMetrics(TestDvc): def _do_write(self, branch): self.dvc.scm.checkout(branch) - self.dvc.checkout() + self.dvc.checkout(force=True) with open('metrics.json', 'w+') as fd: json.dump({'metrics': branch}, fd) diff --git a/tests/test_repro.py b/tests/test_repro.py index 531379726..a1976c08d 100644 --- a/tests/test_repro.py +++ b/tests/test_repro.py @@ -585,7 +585,7 @@ class TestReproExternalBase(TestDvc): self.dvc.gc() self.dvc.remove(cmd_stage.path, outs_only=True) - self.dvc.checkout(cmd_stage.path) + self.dvc.checkout(cmd_stage.path, force=True) class TestReproExternalS3(TestReproExternalBase): From f26c1fffef2bc0e2777b4497dbf7314373ccb53b Mon Sep 17 00:00:00 2001 From: "Mr. Outis" Date: Sun, 18 Nov 2018 09:27:42 -0600 Subject: [PATCH 4/4] pull: Add --force flag that permeates down to checkout --- dvc/cli.py | 5 +++++ dvc/command/data_sync.py | 3 ++- dvc/project.py | 5 +++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/dvc/cli.py b/dvc/cli.py index ceb0cf023..9653df092 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -325,6 +325,11 @@ def parse_args(argv=None): default=False, help='Fetch cache for all dependencies of the ' 'specified target.') + pull_parser.add_argument( + '-f', + '--force', + help='Do not prompt when removing ' + 'working directory files.') pull_parser.set_defaults(func=CmdDataPull) # Push diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index 839da86f4..62586f90a 100644 --- a/dvc/command/data_sync.py +++ b/dvc/command/data_sync.py @@ -25,7 +25,8 @@ class CmdDataPull(CmdDataBase): show_checksums=self.args.show_checksums, all_branches=self.args.all_branches, all_tags=self.args.all_tags, - with_deps=self.args.with_deps) + with_deps=self.args.with_deps, + force=self.args.force) except Exception as exc: self.project.logger.error('Failed to pull data from the cloud', exc) diff --git a/dvc/project.py b/dvc/project.py index 69050f029..5763bc2c7 100644 --- a/dvc/project.py +++ b/dvc/project.py @@ -762,7 +762,8 @@ class Project(object): all_branches=False, show_checksums=False, with_deps=False, - all_tags=False): + all_tags=False, + force=False): self.fetch(target, jobs, remote=remote, @@ -770,7 +771,7 @@ class Project(object): all_tags=all_tags, show_checksums=show_checksums, with_deps=with_deps) - self.checkout(target=target, with_deps=with_deps) + self.checkout(target=target, with_deps=with_deps, force=force) def _local_status(self, target=None): status = {}