diff --git a/dvc/command/gc.py b/dvc/command/gc.py index 0f160b544..2209f1eac 100644 --- a/dvc/command/gc.py +++ b/dvc/command/gc.py @@ -26,12 +26,10 @@ class CmdGC(Traverse): Logger.warn(u'[Cmd-GC] Data item {} is not a valid symbolic link'.format(target)) data_item = self.settings.path_factory.data_item(target) - print('DATA ITEM {} {}'.format(data_item.data.relative, data_item.cache.relative)) - for cache_data_item in data_item.get_all_caches(): - if cache_data_item.cache.relative != data_item.cache.relative: + if cache_data_item.cache.relative != data_item.resolved_cache.relative: os.remove(cache_data_item.cache.relative) - Logger.info(u'GC cache file {}'.format(cache_data_item.cache.relative)) + Logger.info(u'GC cache file {} was removed'.format(cache_data_item.cache.relative)) self._remove_cloud_cache(cache_data_item) Logger.debug(u'[Cmd-GC] GC data item {}. Success.'.format(data_item.data.relative)) @@ -46,6 +44,7 @@ class CmdGC(Traverse): # Renaming def gc_dir(self, target): + Logger.debug(u'[Cmd-GC] GC dir {}.'.format(target)) return self._traverse_dir(target) def gc_file(self, target): diff --git a/dvc/path/data_item.py b/dvc/path/data_item.py index 80e630825..4d1429a97 100644 --- a/dvc/path/data_item.py +++ b/dvc/path/data_item.py @@ -72,6 +72,7 @@ class DataItem(object): def copy(self, cache_file=None): if not cache_file: cache_file = self._cache_file + return DataItem(self._data.abs, self._git, self._config, cache_file) def __hash__(self): @@ -99,7 +100,7 @@ class DataItem(object): cache_dir = os.path.join(self._git.git_dir_abs, self._config.cache_dir) if self._cache_file: - file_name = os.path.basename(self._cache_file) + file_name = os.path.relpath(os.path.realpath(self._cache_file), cache_dir) else: file_name = self.data_dvc_short + self.CACHE_FILE_SEP + self._git.curr_commit @@ -110,12 +111,12 @@ class DataItem(object): result = [] suffix_len = self._git.COMMIT_LEN + len(self.CACHE_FILE_SEP) - cache_prefix = os.path.basename(self.cache.relative[:-suffix_len]) + cache_prefix = os.path.basename(self.resolved_cache.relative[:-suffix_len]) - print('DATA={} DIRNAME={}'.format(self._data.relative, self.cache.dirname)) - for cache_file in os.listdir(self.cache.dirname): + for cache_file in os.listdir(self.resolved_cache.dirname): if cache_file[:-suffix_len] == cache_prefix: - result.append(self.copy(cache_file)) + data_item = self.copy(os.path.join(self.resolved_cache.dirname, cache_file)) + result.append(data_item) return result diff --git a/dvc/path/factory.py b/dvc/path/factory.py index 2570a1c94..23f20e0d2 100644 --- a/dvc/path/factory.py +++ b/dvc/path/factory.py @@ -15,8 +15,8 @@ class PathFactory(object): def path(self, relative_raw): return Path(relative_raw, self._git) - def data_item(self, data_file): - return DataItem(data_file, self._git, self._config) + def data_item(self, data_file, cache_file=None): + return DataItem(data_file, self._git, self._config, cache_file) def stated_data_item(self, state, data_file): return StatedDataItem(state, data_file, self._git, self._config) diff --git a/tests/test_cmd_data_remove.py b/tests/test_cmd_data_remove.py index eee2fd233..0d228fb86 100644 --- a/tests/test_cmd_data_remove.py +++ b/tests/test_cmd_data_remove.py @@ -1,7 +1,6 @@ import os -from dvc.command.remove import CmdDataRemove, DataRemoveError -from dvc.path.data_item import DataItemError +from dvc.command.remove import CmdDataRemove from tests.basic_env import DirHierarchyEnvironment @@ -16,7 +15,7 @@ class TestCmdDataRemove(DirHierarchyEnvironment): dir1_dvc_name = os.path.join('data', self.dir1) self.assertTrue(os.path.exists(dir1_dvc_name)) - cmd.remove_dir_file_by_file(dir1_dvc_name) + cmd.remove_dir(dir1_dvc_name) self.assertFalse(os.path.exists(dir1_dvc_name)) def test_remove_deep_dir(self): @@ -38,8 +37,7 @@ class TestCmdDataRemove(DirHierarchyEnvironment): dir1_dvc_name = os.path.join('data', self.dir1) self.assertTrue(os.path.exists(dir1_dvc_name)) - with self.assertRaises(DataRemoveError): - cmd.remove_dir(dir1_dvc_name) + cmd.remove_dir(dir1_dvc_name) pass def test_data_dir_removal(self): @@ -50,8 +48,7 @@ class TestCmdDataRemove(DirHierarchyEnvironment): data_dir = 'data' self.assertTrue(os.path.exists(data_dir)) - with self.assertRaises(DataRemoveError): - cmd.remove_dir(data_dir) + cmd.remove_dir(data_dir) pass @@ -95,9 +92,8 @@ class TestRemoveDataItem(DirHierarchyEnvironment): self.assertTrue(os.path.exists(self.state6)) self.assertTrue(os.path.isfile(self.state6)) - with self.assertRaises(DataItemError): - cmd.remove_file(self.file6) - self.assertTrue(os.path.exists(self.file6)) + cmd.remove_file(self.file6) + self.assertFalse(os.path.exists(self.file6)) class TestRemoveEndToEnd(DirHierarchyEnvironment): @@ -148,7 +144,7 @@ class TestRemoveEndToEnd(DirHierarchyEnvironment): self.assertTrue(os.path.exists(dir2_full)) self.assertTrue(os.path.exists(self.file1)) - self.assertTrue(cmd.remove_all_targets()) + self.assertTrue(cmd.remove_all()) self.assertFalse(os.path.exists(dir11_full)) self.assertFalse(os.path.exists(self.file5)) @@ -202,7 +198,7 @@ class TestRemoveEndToEndWithAnError(DirHierarchyEnvironment): self.assertTrue(os.path.exists(self.file1)) self.assertTrue(os.path.exists(self.file6)) - self.assertTrue(cmd.remove_all_targets()) + self.assertTrue(cmd.remove_all()) self.assertFalse(os.path.exists(dir11_full)) self.assertFalse(os.path.exists(self.file5)) diff --git a/tests/test_cmd_gc.py b/tests/test_cmd_gc.py new file mode 100644 index 000000000..15506d586 --- /dev/null +++ b/tests/test_cmd_gc.py @@ -0,0 +1,139 @@ +import glob, os + +from dvc.command.gc import CmdGC +from dvc.config import ConfigI +from dvc.git_wrapper import GitWrapperI +from dvc.path.data_item import DataItem +from dvc.path.factory import PathFactory +from dvc.settings import Settings +from dvc.system import System +from tests.basic_env import BasicEnvironment + + +class TestCmdDataRemove(BasicEnvironment): + def setUp(self): + self._test_dir = os.path.join(os.path.sep, 'tmp', 'dvc_unit_test') + curr_dir = None + commit = 'abc1234' + + BasicEnvironment.init_environment(self, self._test_dir, curr_dir) + + self._commit = commit + + self._git = GitWrapperI(git_dir=self._test_git_dir, commit=self._commit) + self._config = ConfigI('data', 'cache', 'state') + self.path_factory = PathFactory(self._git, self._config) + self.settings = Settings([], self._git, self._config) + + self.dir1 = 'dir1' + + self.create_dirs(self.dir1) + + def test_single_file(self): + os.chdir(self._test_git_dir) + + self.crate_data_item_with_five_caches('', 'file1.txt') + self.assertEqual(5, self.cache_file_nums('file1*')) + + cmd = CmdGC(self.settings) + os.chdir(self._test_git_dir) + + cmd.gc_file(os.path.join('data', 'file1.txt')) + self.assertEqual(1, self.cache_file_nums('file1*')) + + def test_all(self): + os.chdir(self._test_git_dir) + + self.crate_data_item_with_five_caches('', 'file1.txt') + self.crate_data_item_with_five_caches('', 'file2.txt') + self.crate_data_item_with_five_caches(self.dir1, 'file3.txt') + + self.assertEqual(5, self.cache_file_nums('file1*')) + self.assertEqual(5, self.cache_file_nums('file2*')) + self.assertEqual(5, self.cache_file_nums(os.path.join(self.dir1, 'file3*'))) + + cmd = CmdGC(self.settings) + os.chdir(self._test_git_dir) + + cmd.parsed_args.no_git_actions = True + cmd.parsed_args.target = ['data'] + cmd.gc_all() + + self.assertEqual(1, self.cache_file_nums('file1*')) + self.assertEqual(1, self.cache_file_nums('file2*')) + self.assertEqual(1, self.cache_file_nums(os.path.join(self.dir1, 'file3*'))) + + def test_empty(self): + os.chdir(self._test_git_dir) + + self.crate_data_item_with_five_caches('', 'file1.txt') + self.crate_data_item_with_five_caches('', 'file2.txt') + self.assertEqual(5, self.cache_file_nums('file1*')) + self.assertEqual(5, self.cache_file_nums('file2*')) + + cmd = CmdGC(self.settings) + os.chdir(self._test_git_dir) + + cmd.parsed_args.no_git_actions = True + cmd.gc_all() + + self.assertEqual(5, self.cache_file_nums('file1*')) + self.assertEqual(5, self.cache_file_nums('file2*')) + + def create_dirs(self, dir): + os.mkdir(os.path.join(self._test_git_dir, 'data', dir)) + os.mkdir(os.path.join(self._test_git_dir, 'cache', dir)) + os.mkdir(os.path.join(self._test_git_dir, 'state', dir)) + + def cache_file_nums(self, pattern): + os.chdir(os.path.join(self._test_git_dir, 'cache')) + files = [] + for file in glob.glob(pattern): + files.append(file) + return len(files) + + def crate_data_item_with_five_caches(self, dir, data_file, content='random text'): + os.chdir(self._test_git_dir) + + file_result = os.path.join('data', dir, data_file) + + state_result = os.path.join('state', dir, data_file + DataItem.STATE_FILE_SUFFIX) + self.create_content_file(state_result, 'state content') + + file_prefix = data_file + DataItem.CACHE_FILE_SEP + cache_result = os.path.join('cache', dir, file_prefix + self._commit) + self.create_content_file(cache_result, content) + relevant_dir = self.relevant_dir(os.path.join(dir, data_file)) + cache_file = os.path.join(relevant_dir, cache_result) + + d = os.path.join(self._test_git_dir, 'data', dir) + os.chdir(d) + print('---- CREATE SL {}: {} --> {}'.format(d, data_file, cache_file)) + System.symlink(cache_file, data_file) + os.chdir(self._test_git_dir) + + data_item = self.settings.path_factory.existing_data_item(file_result) + print('*DATA ITEM {}: {} {}'.format(file_result, data_item.data.relative, data_item.cache.relative)) + + print('----> {} {} {} {}'.format(dir, data_file, cache_file, file_result)) + + # Additional cache files + self.create_content_file(os.path.join('cache', dir, file_prefix + 'aaaaaaa')) + self.create_content_file(os.path.join('cache', dir, file_prefix + '1111111')) + self.create_content_file(os.path.join('cache', dir, file_prefix + '5555555')) + self.create_content_file(os.path.join('cache', dir, file_prefix + '123abff')) + return + + @staticmethod + def create_content_file(file, content='some test'): + fd = open(file, 'w+') + fd.write(content) + fd.close() + + @staticmethod + def relevant_dir(data_file): + deepth = data_file.count(os.path.sep) + relevant_path = '..' + for i in range(deepth): + relevant_path = os.path.join(relevant_path, '..') + return relevant_path diff --git a/tests/test_data_file_obj.py b/tests/test_data_file_obj.py index 8397fb4f5..c78faa9d7 100644 --- a/tests/test_data_file_obj.py +++ b/tests/test_data_file_obj.py @@ -36,6 +36,45 @@ class TestDataFileObjBasic(BasicEnvironment): pass +class TestDataItemWithGivenCache(BasicEnvironment): + def setUp(self): + BasicEnvironment.init_environment(self, test_dir=os.path.join(os.path.sep, 'tmp', 'ntx_unit_test')) + + self._git = GitWrapperI(git_dir=self._test_git_dir, commit='ad45ba8') + self._config = ConfigI('data', 'cache', 'state') + + self._file = os.path.join('data', 'file.txt') + self._cache = os.path.join('cache', 'file.txt_abcd') + + self._data_item = DataItem(self._file, self._git, self._config, self._cache) + pass + + def test_basic(self): + self.assertEqual(self._data_item.data.relative, self._file) + self.assertEqual(self._data_item.cache.relative, self._cache) + + def test_resolve_unexist_symlink(self): + self.assertEqual(self._data_item.resolved_cache.relative, self._file) + + +class TestDeepDataItemWithGivenCache(BasicEnvironment): + def setUp(self): + BasicEnvironment.init_environment(self, test_dir=os.path.join(os.path.sep, 'tmp', 'ntx_unit_test')) + + self._git = GitWrapperI(git_dir=self._test_git_dir, commit='ad45ba8') + self._config = ConfigI('data', 'cache', 'state') + + self._file = os.path.join('data', 'dir1', 'file.txt') + self._cache = os.path.join('cache', 'dir1', 'file.txt_abcd') + + self._data_item = DataItem(self._file, self._git, self._config, self._cache) + pass + + def test_basic(self): + self.assertEqual(self._data_item.data.relative, self._file) + self.assertEqual(self._data_item.cache.relative, self._cache) + + class TestPathFactory(BasicEnvironment): def setUp(self): BasicEnvironment.init_environment(self, test_dir=os.path.join(os.path.sep, 'tmp', 'ntx_unit_test'))