Merge pull request #1223 from efiop/master

Misc fixes

dvc/remote/local.py

@@ -81,14 +81,35 @@ class RemoteLOCAL(RemoteBase):
         relpath = os.path.relpath(path, self.cache_dir)
         return os.path.dirname(relpath) + os.path.basename(relpath)
 
-    def changed_cache(self, md5):
+    def changed_cache_file(self, md5):
         cache = self.get(md5)
         if self.state.changed(cache, md5=md5):
             if os.path.exists(cache):
-                msg = 'Corrupted cache file {}'
+                msg = 'Corrupted cache file {}.'
                 Logger.warn(msg.format(os.path.relpath(cache)))
                 remove(cache)
             return True
+        return False
+
+    def changed_cache(self, md5):
+        cache = self.get(md5)
+        clist = [(cache, md5)]
+
+        while True:
+            if len(clist) == 0:
+                break
+
+            cache, md5 = clist.pop()
+            if self.changed_cache_file(md5):
+                return True
+
+            if not self.is_dir_cache(cache):
+                continue
+
+            for entry in self.load_dir_cache(md5):
+                md5 = entry[self.PARAM_MD5]
+                cache = self.get(md5)
+                clist.append((cache, md5))
 
         return False
 
@@ -512,7 +533,7 @@ class RemoteLOCAL(RemoteBase):
 
         progress.update_target(title, 90, 100)
 
-        local_exists = [not self.changed_cache(md5) for md5 in md5s]
+        local_exists = [not self.changed_cache_file(md5) for md5 in md5s]
 
         progress.finish_target(title)
 
@@ -539,7 +560,7 @@ class RemoteLOCAL(RemoteBase):
         names = []
         # NOTE: filter files that are not corrupted
         for md5, name in grouped:
-            if self.changed_cache(md5):
+            if self.changed_cache_file(md5):
                 md5s.append(md5)
                 names.append(name)
 
@@ -617,7 +638,7 @@ class RemoteLOCAL(RemoteBase):
 
         # NOTE: verifying that our cache is not corrupted
         def func(info):
-            return not self.changed_cache(info[self.PARAM_MD5])
+            return not self.changed_cache_file(info[self.PARAM_MD5])
         checksum_infos = list(filter(func, checksum_infos))
 
         progress.update_target(title, 20, 100)

dvc/state.py

@@ -86,33 +86,34 @@ class State(object):
         Logger.debug(cmd)
         return self.c.execute(cmd)
 
-    def _load(self):
+    def _load(self, empty=False):
         from dvc import VERSION
 
-        cmd = "PRAGMA user_version;"
-        self._execute(cmd)
-        ret = self.c.fetchall()
-        assert len(ret) == 1
-        assert len(ret[0]) == 1
-        assert isinstance(ret[0][0], int)
-        self.version = ret[0][0]
+        if not empty:
+            cmd = "PRAGMA user_version;"
+            self._execute(cmd)
+            ret = self.c.fetchall()
+            assert len(ret) == 1
+            assert len(ret[0]) == 1
+            assert isinstance(ret[0][0], int)
+            version = ret[0][0]
 
-        if self.version > self.VERSION:
-            msg = "You are using an old version '{}' of dvc that is using " \
-                  "state file version '{}' which is not compatible with the " \
-                  "state file version '{}' that is used in this projet. " \
-                  "Please upgrade right now!"
-            raise DvcException(msg.format(VERSION,
-                                          self.VERSION,
-                                          self.version))
-        elif self.version < self.VERSION:
-            msg = "State file version '{}' is too old. " \
-                  "Reformatting to the current version '{}'."
-            self.project.logger.warn(msg.format(self.version, self.VERSION))
-            cmd = "DROP TABLE IF EXISTS {};"
-            self._execute(cmd.format(self.STATE_TABLE))
-            self._execute(cmd.format(self.STATE_INFO_TABLE))
-            self._execute(cmd.format(self.LINK_STATE_TABLE))
+            if version > self.VERSION:
+                msg = "You are using an old version '{}' of dvc that is " \
+                      "using state file version '{}' which is not " \
+                      "compatible with the state file version '{}' that " \
+                      "is used in this projet. Please upgrade right now!"
+                raise DvcException(msg.format(VERSION,
+                                              self.VERSION,
+                                              version))
+            elif version < self.VERSION:
+                msg = "State file version '{}' is too old. " \
+                      "Reformatting to the current version '{}'."
+                self.project.logger.warn(msg.format(version, self.VERSION))
+                cmd = "DROP TABLE IF EXISTS {};"
+                self._execute(cmd.format(self.STATE_TABLE))
+                self._execute(cmd.format(self.STATE_INFO_TABLE))
+                self._execute(cmd.format(self.LINK_STATE_TABLE))
 
         # Check that the state file is indeed a database
         cmd = "CREATE TABLE IF NOT EXISTS {} ({})"
@@ -137,13 +138,14 @@ class State(object):
             assert self.db is None
             assert self.c is None
             assert self.inserts == 0
+            empty = not os.path.exists(self.state_file)
             self.db = sqlite3.connect(self.state_file)
             self.c = self.db.cursor()
 
             # Try loading once to check that the file is indeed a database
             # and reformat it if it is not.
             try:
-                self._load()
+                self._load(empty=empty)
                 return
             except sqlite3.DatabaseError:
                 self.c.close()

tests/basic_env.py

@@ -1,4 +1,5 @@
 import os
+import uuid
 import shutil
 import tempfile
 from git import Repo
@@ -38,8 +39,15 @@ class TestDir(TestCase):
         with open(name, 'a') as f:
             f.write(contents)
 
+    @staticmethod
+    def mkdtemp():
+        prefix = 'dvc-test.{}.'.format(os.getpid())
+        suffix = '.{}'.format(uuid.uuid4())
+        return tempfile.mkdtemp(prefix=prefix, suffix=suffix)
+
     def setUp(self):
-        self._root_dir = tempfile.mkdtemp()
+        self._root_dir = TestDir.mkdtemp()
+
         self._pushd(self._root_dir)
         self.create(self.FOO, self.FOO_CONTENTS)
         self.create(self.BAR, self.BAR_CONTENTS)

tests/test_add.py

@@ -2,7 +2,6 @@ import os
 import stat
 import shutil
 import filecmp
-import tempfile
 
 from dvc.main import main
 from dvc.utils import file_md5
@@ -132,7 +131,7 @@ class TestAddFileInDir(TestDvc):
 
 class TestAddExternalLocalFile(TestDvc):
     def test(self):
-        dname = tempfile.mkdtemp()
+        dname = TestDvc.mkdtemp()
         fname = os.path.join(dname, 'foo')
         shutil.copyfile(self.FOO, fname)
 

tests/test_cache.py

@@ -1,6 +1,5 @@
 import os
 import shutil
-import tempfile
 
 from dvc.cache import Cache
 from dvc.system import System
@@ -52,7 +51,7 @@ class TestCacheLoadBadDirCache(TestDvc):
 
 class TestExternalCacheDir(TestDvc):
     def test(self):
-        cache_dir = tempfile.mkdtemp()
+        cache_dir = TestDvc.mkdtemp()
 
         ret = main(['config', 'cache.dir', cache_dir])
         self.assertEqual(ret, 0)

tests/test_checkout.py

@@ -6,6 +6,7 @@ import shutil
 import filecmp
 
 from dvc.main import main
+from dvc.project import Project
 from tests.basic_env import TestDvc
 from tests.test_repro import TestRepro
 from dvc.stage import Stage
@@ -65,6 +66,32 @@ class TestCheckoutCorruptedCacheFile(TestRepro):
         self.assertFalse(os.path.isfile(cache))
 
 
+class TestCheckoutCorruptedCacheDir(TestDvc):
+    def test(self):
+        # NOTE: using 'copy' so that cache and link don't have same inode
+        ret = main(['config', 'cache.type', 'copy'])
+        self.assertEqual(ret, 0)
+
+        self.dvc = Project('.')
+        stages = self.dvc.add(self.DATA_DIR)
+        self.assertEqual(len(stages), 1)
+        self.assertEqual(len(stages[0].outs), 1)
+        out = stages[0].outs[0]
+
+        # NOTE: modifying cache file for one of the files inside the directory
+        # to check if dvc will detect that the cache is corrupted.
+        entry = self.dvc.cache.local.load_dir_cache(out.md5)[0]
+        md5 = entry[self.dvc.cache.local.PARAM_MD5]
+        cache = self.dvc.cache.local.get(md5)
+
+        with open(cache, 'w+') as fobj:
+            fobj.write('1')
+
+        self.dvc.checkout()
+
+        self.assertFalse(os.path.exists(cache))
+
+
 class TestCmdCheckout(TestCheckout):
     def test(self):
         ret = main(['checkout'])

tests/test_data_cloud.py

@@ -6,7 +6,6 @@ import time
 import uuid
 import shutil
 import getpass
-import tempfile
 import platform
 
 from dvc.main import main
@@ -105,7 +104,7 @@ def _should_test_hdfs():
 
 
 def get_local_storagepath():
-    return tempfile.mkdtemp()
+    return TestDvc.mkdtemp()
 
 
 def get_local_url():
@@ -164,7 +163,7 @@ class TestDataCloud(TestDvc):
         clist = [('s3://', RemoteS3),
                  ('gs://', RemoteGS),
                  ('ssh://user@localhost:/', RemoteSSH),
-                 (tempfile.mkdtemp(), RemoteLOCAL)]
+                 (TestDvc.mkdtemp(), RemoteLOCAL)]
 
         if _should_test_azure():
             clist.append(('azure://ContainerName=', RemoteAzure))

tests/test_repro.py

@@ -4,7 +4,6 @@ import stat
 import shutil
 import filecmp
 import getpass
-import tempfile
 import posixpath
 from subprocess import Popen, PIPE
 
@@ -655,7 +654,7 @@ class TestReproExternalSSH(TestReproExternalBase):
     @property
     def bucket(self):
         if not self._dir:
-            self._dir = tempfile.mkdtemp()
+            self._dir = TestDvc.mkdtemp()
         return '{}@127.0.0.1:{}'.format(getpass.getuser(), self._dir)
 
     def cmd(self, i, o):
@@ -702,7 +701,7 @@ class TestReproExternalSSH(TestReproExternalBase):
 class TestReproExternalLOCAL(TestReproExternalBase):
     def setUp(self):
         super(TestReproExternalLOCAL, self).setUp()
-        self.tmpdir = tempfile.mkdtemp()
+        self.tmpdir = TestDvc.mkdtemp()
 
     def should_test(self):
         return True