From 0cd0e6700f6fc30bbc8acc42361048ec4880ae60 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Wed, 7 Mar 2018 12:02:09 +0200 Subject: [PATCH] cache: check of cache corruption Signed-off-by: Ruslan Kuprieiev --- dvc/cache.py | 4 +++ dvc/output.py | 51 ++++++++++---------------------- dvc/state.py | 67 +++++++++++++++++++++++++++--------------- tests/test_checkout.py | 15 ++++++++++ tests/test_state.py | 43 +++++++++++++++------------ 5 files changed, 102 insertions(+), 78 deletions(-) diff --git a/dvc/cache.py b/dvc/cache.py index be66b7a23..7f0c9fdda 100644 --- a/dvc/cache.py +++ b/dvc/cache.py @@ -33,6 +33,10 @@ class Cache(object): def get(self, md5): return os.path.join(self.cache_dir, md5[0:2], md5[2:]) + def path_to_md5(self, path): + relpath = os.path.relpath(path, self.cache_dir) + return os.path.dirname(relpath) + os.path.basename(relpath) + def find_cache(self, files): file_set = set(files) cached = {} diff --git a/dvc/output.py b/dvc/output.py index 2b1bd17f5..e43ce00c4 100644 --- a/dvc/output.py +++ b/dvc/output.py @@ -70,13 +70,7 @@ class Dependency(object): if not os.path.exists(self.path): return True - state = self.project.state.get(self.path) - if state and state.mtime == self.mtime() and state.inode == self.inode(): - md5 = state.md5 - else: - md5 = self.compute_md5() - - return self.md5 != md5 + return self.project.state.changed(self.path, self.md5) def changed(self): return self._changed_md5() @@ -85,18 +79,6 @@ class Dependency(object): def is_dir_cache(cache): return cache.endswith(Output.MD5_DIR_SUFFIX) - def compute_md5(self): - if os.path.isdir(self.path): - return dirhash(self.path, hashfunc='md5') + self.MD5_DIR_SUFFIX - else: - return file_md5(self.path)[0] - - def mtime(self): - return os.path.getmtime(self.path) - - def inode(self): - return System.inode(self.path) - def save(self): if not os.path.exists(self.path): raise CmdOutputDoesNotExistError(self.rel_path) @@ -104,15 +86,7 @@ class Dependency(object): if not os.path.isfile(self.path) and not os.path.isdir(self.path): raise CmdOutputIsNotFileOrDirError(self.rel_path) - state = self.project.state.get(self.path) - if state and state.mtime == self.mtime() and state.inode == self.inode(): - md5 = state.md5 - msg = u'\'{}\' using md5 {} from state file' - self.project.logger.debug(msg.format(self.dvc_path, md5)) - self.md5 = md5 - else: - self.md5 = self.compute_md5() - self.project.state.update(self.path, self.md5, self.mtime(), self.inode()) + self.md5 = self.project.state.update(self.path).md5 def dumpd(self, cwd): return { @@ -182,11 +156,21 @@ class Output(Dependency): def loads_from(cls, project, s_list, use_cache=False, cwd=os.curdir): return [cls.loads(project, x, use_cache=use_cache, cwd=cwd) for x in s_list] + def _changed_cache(self, cache): + state = self.project.state.update(cache) + if state.md5 != self.project.cache.path_to_md5(cache): + self.project.logger.debug('Corrupted cache file {}'.format(cache)) + self.project._remove_cache_file(cache) + return True + + return False + def _changed_file(self, path, cache): if os.path.isfile(path) and \ os.path.isfile(cache) and \ System.samefile(path, cache) and \ - os.stat(cache).st_mode & stat.S_IREAD: + os.stat(cache).st_mode & stat.S_IREAD and \ + not self._changed_cache(cache): return False return True @@ -319,13 +303,8 @@ class Output(Dependency): path = os.path.join(root, fname) relpath = os.path.relpath(path, self.path) - state = self.project.state.get(path) - if state and state.mtime == mtime and state.inode == inode: - md5 = state.md5 - else: - md5 = file_md5(path)[0] - - dir_info.append({self.PARAM_RELPATH: relpath, self.PARAM_MD5: md5}) + state = self.project.state.update(path) + dir_info.append({self.PARAM_RELPATH: relpath, self.PARAM_MD5: state.md5}) return dir_info diff --git a/dvc/state.py b/dvc/state.py index f4ae082df..1e1babf4a 100644 --- a/dvc/state.py +++ b/dvc/state.py @@ -1,22 +1,22 @@ import os import tinydb +from checksumdir import dirhash +from dvc.system import System +from dvc.output import Output +from dvc.utils import file_md5 from dvc.exceptions import DvcException class StateEntry(object): - PARAM_PATH = 'path' PARAM_MTIME = 'mtime' PARAM_MD5 = 'md5' PARAM_INODE = 'inode' - def __init__(self, root_dir, path, md5, mtime, inode): - self.root_dir = root_dir - self.path = path + def __init__(self, md5, mtime, inode): self.mtime = mtime self.md5 = md5 self.inode = inode - self.dvc_path = os.path.relpath(self.path, self.root_dir) def update(self, md5, mtime, inode): self.mtime = mtime @@ -24,16 +24,14 @@ class StateEntry(object): self.inode = inode @staticmethod - def loadd(root_dir, d): - path = os.path.join(root_dir, d[StateEntry.PARAM_PATH]) + def loadd(d): mtime = d[StateEntry.PARAM_MTIME] md5 = d[StateEntry.PARAM_MD5] inode = d[StateEntry.PARAM_INODE] - return StateEntry(root_dir, path, md5, mtime, inode) + return StateEntry(md5, mtime, inode) def dumpd(self): return { - self.PARAM_PATH: self.dvc_path, self.PARAM_MD5: self.md5, self.PARAM_MTIME: self.mtime, self.PARAM_INODE: self.inode @@ -58,27 +56,50 @@ class State(object): def init(root_dir, dvc_dir): return State(root_dir, dvc_dir) - def update(self, path, md5, mtime, inode): - existing = self.get(path) - if not existing: - return self.add(path, md5, mtime, inode) + def compute_md5(self, path): + if os.path.isdir(path): + return dirhash(path, hashfunc='md5') + Output.MD5_DIR_SUFFIX + else: + return file_md5(path)[0] - state = StateEntry(self.root_dir, path, md5, mtime, inode) - self._db.update(state.dumpd(), self._q.path == state.dvc_path) + def changed(self, path, md5): + state = self.update(path) + return state.md5 != md5 + + def update(self, path): + mtime = os.path.getmtime(path) + inode = System.inode(path) + + state = self._get(inode, mtime) + if state: + return state + + md5 = self.compute_md5(path) + state = StateEntry(md5, mtime, inode) + d = state.dumpd() + if self._db.contains(self._q.inode == inode): + self._db.update(d, self._q.inode == inode) + else: + self._db.insert(d) return state - def add(self, path, md5, mtime, inode): - entry = StateEntry(self.root_dir, path, md5, mtime, inode) - self._db.insert(entry.dumpd()) - return entry - - def get(self, path): - d_list = self._db.search(self._q.path == os.path.relpath(path, self.root_dir)) + def _get(self, inode, mtime): + d_list = self._db.search(self._q.inode == inode) if not len(d_list): return None if len(d_list) > 1: raise StateDuplicateError() - return StateEntry.loadd(self.root_dir, d_list[0]) + state = StateEntry.loadd(d_list[0]) + if mtime == state.mtime and inode == state.inode: + return state + + return None + + def get(self, path): + mtime = os.path.getmtime(path) + inode = System.inode(path) + + return self._get(inode, mtime) diff --git a/tests/test_checkout.py b/tests/test_checkout.py index 8528dd7b4..7a248b1a2 100644 --- a/tests/test_checkout.py +++ b/tests/test_checkout.py @@ -26,6 +26,21 @@ class TestCheckout(TestRepro): self.assertTrue(filecmp.cmp(self.FOO, self.orig)) +class TestCheckoutCorruptedCacheFile(TestRepro): + def test(self): + cache = self.foo_stage.outs[0].cache + + os.chmod(cache, stat.S_IWRITE) + with open(cache, 'a') as fd: + fd.write('1') + os.chmod(cache, stat.S_IREAD) + + self.dvc.checkout() + + self.assertFalse(os.path.isfile(self.FOO)) + self.assertFalse(os.path.isfile(cache)) + + class TestCmdCheckout(TestCheckout): def test(self): ret = main(['checkout']) diff --git a/tests/test_state.py b/tests/test_state.py index af80eaa3a..f29f49585 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -1,35 +1,40 @@ import os +import stat +from dvc.system import System from dvc.state import State, StateEntry +from dvc.utils import file_md5 from tests.basic_env import TestDvc class TestState(TestDvc): def test_update(self): - path = os.path.join(self.dvc.root_dir, '1') - md5_1 = '1' - md5_2 = '2' - mtime_1 = 1 - mtime_2 = 2 - inode_1 = 1 - inode_2 = 2 + path = os.path.join(self.dvc.root_dir, self.FOO) + md5 = file_md5(path)[0] + mtime = os.path.getmtime(path) + inode = System.inode(path) state = State(self.dvc.root_dir, self.dvc.dvc_dir) - self.assertIsNone(state.get(path)) - state.add(path, md5_1, mtime_1, inode_1) + state.update(path) entry = state.get(path) self.assertIsInstance(entry, StateEntry) - self.assertEqual(entry.path, path) - self.assertEqual(entry.md5, md5_1) - self.assertEqual(entry.mtime, mtime_1) - self.assertEqual(entry.inode, inode_1) + self.assertEqual(entry.md5, md5) + self.assertEqual(entry.mtime, mtime) + self.assertEqual(entry.inode, inode) - state.update(path, md5_2, mtime_2, inode_2) - entry = state.get(path) + os.chmod(path, stat.S_IWRITE) + os.unlink(path) + with open(path, 'w+') as fd: + fd.write('1') + + md5 = file_md5(path)[0] + mtime = os.path.getmtime(path) + inode = System.inode(path) + + entry = state.update(path) self.assertIsInstance(entry, StateEntry) - self.assertEqual(entry.path, path) - self.assertEqual(entry.md5, md5_2) - self.assertEqual(entry.mtime, mtime_2) - self.assertEqual(entry.inode, inode_2) + self.assertEqual(entry.md5, md5) + self.assertEqual(entry.mtime, mtime) + self.assertEqual(entry.inode, inode)