Repository: iterative/dvc

Issue:
dvc: add sqlite temp files to gitignore
E.g. .dvc/state-journal. See https://www.sqlite.org/tempfiles.html for other temporary files

Discussion:

Patch:
diff --git a/.dvc/.gitignore b/.dvc/.gitignore
index 0e30536ff..a53edd2f3 100644
--- a/.dvc/.gitignore
+++ b/.dvc/.gitignore
@@ -5,4 +5,6 @@ config.local
 updater
 cache
 state
-state.lock
\ No newline at end of file
+state.lock
+state-journal
+state-wal
diff --git a/dvc/cache.py b/dvc/cache.py
index 821249689..c492e32cc 100644
--- a/dvc/cache.py
+++ b/dvc/cache.py
@@ -12,24 +12,25 @@ class Cache(object):
         self.project = project
 
         config = project.config._config[Config.SECTION_CACHE]
+        local = config.get(Config.SECTION_CACHE_LOCAL)
 
-        local = config.get(Config.SECTION_CACHE_LOCAL, None)
         if local:
             name = Config.SECTION_REMOTE_FMT.format(local)
             sect = project.config._config[name]
         else:
-            sect = {}
             cache_dir = config.get(Config.SECTION_CACHE_DIR, self.CACHE_DIR)
+            cache_type = config.get(Config.SECTION_CACHE_TYPE)
+            protected = config.get(Config.SECTION_CACHE_PROTECTED)
+
             if not os.path.isabs(cache_dir):
                 cache_dir = os.path.join(project.dvc_dir, cache_dir)
                 cache_dir = os.path.abspath(os.path.realpath(cache_dir))
-            sect[Config.SECTION_REMOTE_URL] = cache_dir
-            t = config.get(Config.SECTION_CACHE_TYPE, None)
-            if t:
-                sect[Config.SECTION_CACHE_TYPE] = t
-            protected = config.get(Config.SECTION_CACHE_PROTECTED, None)
-            if protected is not None:
-                sect[Config.SECTION_CACHE_PROTECTED] = protected
+
+            sect = {
+                Config.SECTION_REMOTE_URL: cache_dir,
+                Config.SECTION_CACHE_TYPE: cache_type,
+                Config.SECTION_CACHE_PROTECTED: protected
+            }
 
         self.local = Remote(project, sect)
 
diff --git a/dvc/project.py b/dvc/project.py
index 1c62a3a81..797060264 100644
--- a/dvc/project.py
+++ b/dvc/project.py
@@ -131,10 +131,12 @@ class Project(object):
         shutil.rmtree(self.dvc_dir)
 
     def _ignore(self):
-        flist = [self.state.state_file,
-                 self.lock.lock_file,
-                 self.config.config_local_file,
-                 self.updater.updater_file]
+        flist = [
+            self.state.state_file,
+            self.lock.lock_file,
+            self.config.config_local_file,
+            self.updater.updater_file,
+        ] + self.state.temp_files
 
         if self.cache.local.cache_dir.startswith(self.root_dir):
             flist += [self.cache.local.cache_dir]
@@ -465,12 +467,10 @@ class Project(object):
 
     def checkout(self, target=None, with_deps=False):
         all_stages = self.active_stages()
+        stages = all_stages
 
         if target:
-            stages = self._collect(target,
-                                   with_deps=with_deps)
-        else:
-            stages = all_stages
+            stages = self._collect(target, with_deps=with_deps)
 
         with self.state:
             self._cleanup_unused_links(all_stages)
diff --git a/dvc/remote/__init__.py b/dvc/remote/__init__.py
index 7fec8f094..6b71f1d8a 100644
--- a/dvc/remote/__init__.py
+++ b/dvc/remote/__init__.py
@@ -14,14 +14,11 @@ REMOTES = [RemoteHDFS, RemoteSSH, RemoteS3, RemoteGS, RemoteAzure, RemoteLOCAL]
 
 def supported_url(url):
     config = {Config.SECTION_REMOTE_URL: url}
-    for r in REMOTES:
-        if r.supported(config):
-            return True
-    return False
+    return any(remote.supported(config) for remote in REMOTES)
 
 
 def Remote(project, config):
-    for r in REMOTES:
-        if r.supported(config):
-            return r(project, config)
+    for remote in REMOTES:
+        if remote.supported(config):
+            return remote(project, config)
     raise UnsupportedRemoteError(str(config))
diff --git a/dvc/remote/local.py b/dvc/remote/local.py
index 5db5e9cbd..317c47dbf 100644
--- a/dvc/remote/local.py
+++ b/dvc/remote/local.py
@@ -270,7 +270,7 @@ class RemoteLOCAL(RemoteBase):
 
     def checkout(self, path_info, checksum_info):
         path = path_info['path']
-        md5 = checksum_info.get(self.PARAM_MD5, None)
+        md5 = checksum_info.get(self.PARAM_MD5)
         cache = self.get(md5)
 
         if not cache:
@@ -289,15 +289,16 @@ class RemoteLOCAL(RemoteBase):
             remove(path)
             return
 
-        if os.path.exists(path):
-            msg = u'Data \'{}\' exists. Removing before checkout.'
-            Logger.warn(msg.format(os.path.relpath(path)))
-            remove(path)
-
         msg = u'Checking out \'{}\' with cache \'{}\'.'
         Logger.info(msg.format(os.path.relpath(path), md5))
 
         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)
+
             self.link(cache, path)
             self.state.update_link(path)
             return
@@ -319,7 +320,19 @@ class RemoteLOCAL(RemoteBase):
             m = entry[self.PARAM_MD5]
             p = os.path.join(path, relpath)
             c = self.get(m)
-            self.link(c, p)
+
+            entry_info = {
+                'scheme': path_info['scheme'],
+                self.PARAM_PATH: p,
+            }
+
+            entry_checksum_info = {
+                self.PARAM_MD5: m,
+            }
+
+            if self.changed(entry_info, entry_checksum_info):
+                remove(p)
+                self.link(c, p)
 
             if bar:
                 progress.update_target(dir_relpath, processed, dir_size)
diff --git a/dvc/state.py b/dvc/state.py
index dcb95c34b..cdf62d527 100644
--- a/dvc/state.py
+++ b/dvc/state.py
@@ -57,6 +57,13 @@ class State(object):
             return
 
         self.state_file = os.path.join(self.dvc_dir, self.STATE_FILE)
+
+        # https://www.sqlite.org/tempfiles.html
+        self.temp_files = [
+            self.state_file + '-journal',
+            self.state_file + '-wal',
+        ]
+
         self.db = None
         self.c = None
         self.inserts = 0


Test patch:
diff --git a/tests/test_checkout.py b/tests/test_checkout.py
index b073c0a64..4e46fcde0 100644
--- a/tests/test_checkout.py
+++ b/tests/test_checkout.py
@@ -4,9 +4,11 @@ import time
 import stat
 import shutil
 import filecmp
+import collections
 
 from dvc.main import main
 from dvc.project import Project
+from dvc.system import System
 from tests.basic_env import TestDvc
 from tests.test_repro import TestRepro
 from dvc.stage import Stage
@@ -116,6 +118,20 @@ class CheckoutBase(TestDvc):
     def read_ignored(self):
         return list(map(lambda s: s.strip('\n'), open(self.GIT_IGNORE).readlines()))
 
+    def outs_info(self, stage):
+        FileInfo = collections.namedtuple('FileInfo', 'path inode')
+
+        paths = [
+            os.path.join(root, file)
+            for output in stage.outs
+            for root, _, files in os.walk(output.path)
+            for file in files
+        ]
+
+        return [
+            FileInfo(path=path, inode=System.inode(path))
+            for path in paths
+        ]
 
 class TestRemoveFilesWhenCheckout(CheckoutBase):
     def test(self):
@@ -143,6 +159,29 @@ class TestRemoveFilesWhenCheckout(CheckoutBase):
         self.assertFalse(os.path.exists(fname))
 
 
+class TestCheckoutSelectiveRemove(CheckoutBase):
+    def test(self):
+        # Use copy to test for changes in the inodes
+        ret = main(['config', 'cache.type', 'copy'])
+        self.assertEqual(ret, 0)
+
+        stages = self.dvc.add(self.DATA_DIR)
+        self.assertEqual(len(stages), 1)
+        stage = stages[0]
+        staged_files = self.outs_info(stage)
+
+        os.remove(staged_files[0].path)
+        ret = main(['checkout', stage.relpath])
+        self.assertEqual(ret, 0)
+
+        checkedout_files = self.outs_info(stage)
+
+        self.assertEqual(len(staged_files), len(checkedout_files))
+        self.assertEqual(staged_files[0].path, checkedout_files[0].path)
+        self.assertNotEqual(staged_files[0].inode, checkedout_files[0].inode)
+        self.assertEqual(staged_files[1].inode, checkedout_files[1].inode)
+
+
 class TestGitIgnoreBasic(CheckoutBase):
     def test(self):
         fname1 = 'file_1'