Merge pull request #1841 from pared/1257

cache: maker reflink, copy, default links

dvc/remote/local.py

@@ -1,5 +1,7 @@
 from __future__ import unicode_literals
 
+from copy import copy
+
 from dvc.utils.compat import str, makedirs
 
 import os
@@ -49,7 +51,7 @@ class RemoteLOCAL(RemoteBase):
     PARAM_RELPATH = "relpath"
     MD5_DIR_SUFFIX = ".dir"
 
-    CACHE_TYPES = ["reflink", "hardlink", "symlink", "copy"]
+    DEFAULT_CACHE_TYPES = ["reflink", "copy"]
     CACHE_TYPE_MAP = {
         "copy": shutil.copyfile,
         "symlink": System.symlink,
@@ -74,7 +76,7 @@ class RemoteLOCAL(RemoteBase):
                 types = [t.strip() for t in types.split(",")]
             self.cache_types = types
         else:
-            self.cache_types = self.CACHE_TYPES
+            self.cache_types = copy(self.DEFAULT_CACHE_TYPES)
 
         if self.cache_dir is not None and not os.path.exists(self.cache_dir):
             os.mkdir(self.cache_dir)

tests/test_cache.py

@@ -168,3 +168,8 @@ class TestCmdCacheDir(TestDvc):
         self.assertEqual(len(subdirs), 1)
         files = os.listdir(os.path.join(tmpdir, subdirs[0]))
         self.assertEqual(len(files), 1)
+
+
+class TestShouldCacheBeReflinkOrCopyByDefault(TestDvc):
+    def test(self):
+        self.assertEqual(self.dvc.cache.local.cache_types, ["reflink", "copy"])

tests/test_checkout.py

@@ -12,7 +12,7 @@ from dvc.main import main
 from dvc import progress
 from dvc.repo import Repo as DvcRepo
 from dvc.system import System
-from dvc.utils import walk_files
+from dvc.utils import walk_files, load_stage_file
 from tests.basic_env import TestDvc
 from tests.test_repro import TestRepro
 from dvc.stage import Stage, StageFileBadNameError, StageFileDoesNotExistError
@@ -134,7 +134,9 @@ class CheckoutBase(TestDvc):
         FileInfo = collections.namedtuple("FileInfo", "path inode")
 
         paths = [
-            path for output in stage.outs for path in walk_files(output.path)
+            path
+            for output in stage["outs"]
+            for path in walk_files(output["path"])
         ]
 
         return [
@@ -210,14 +212,19 @@ class TestCheckoutSelectiveRemove(CheckoutBase):
         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]
+        ret = main(["add", self.DATA_DIR])
+        self.assertEqual(0, ret)
+
+        stage_path = self.DATA_DIR + Stage.STAGE_FILE_SUFFIX
+        stage = load_stage_file(stage_path)
         staged_files = self.outs_info(stage)
 
-        os.remove(staged_files[0].path)
+        # move instead of remove, to lock inode assigned to stage_files[0].path
+        # if we were to use remove, we might end up with same inode assigned to
+        # newly checked out file
+        shutil.move(staged_files[0].path, "random_name")
 
-        ret = main(["checkout", "--force", stage.relpath])
+        ret = main(["checkout", "--force", stage_path])
         self.assertEqual(ret, 0)
 
         checkedout_files = self.outs_info(stage)

tests/test_repro.py

@@ -854,30 +854,6 @@ class TestReproExternalBase(TestDvc):
                         mock_download.assert_not_called()
                         mock_checkout.assert_called_once()
 
-    def corrupted_cache(self):
-        os.unlink("bar.dvc")
-
-        stage = self.dvc.run(
-            deps=[self.FOO], outs=[self.BAR], cmd="echo bar > bar"
-        )
-
-        with open(self.BAR, "w") as fd:
-            fd.write("corrupting the cache")
-
-        patch_checkout = patch.object(
-            stage.outs[0], "checkout", wraps=stage.outs[0].checkout
-        )
-
-        patch_run = patch.object(stage, "_run", wraps=stage._run)
-
-        with self.dvc.state:
-            with patch_checkout as mock_checkout:
-                with patch_run as mock_run:
-                    stage.run()
-
-                    mock_run.assert_called_once()
-                    mock_checkout.assert_not_called()
-
     @patch("dvc.prompt.confirm", return_value=True)
     def test(self, mock_prompt):
         if not self.should_test():
@@ -977,8 +953,6 @@ class TestReproExternalBase(TestDvc):
         self.dvc.checkout(cmd_stage.path, force=True)
         self.assertEqual(self.dvc.status(cmd_stage.path), {})
 
-        self.corrupted_cache()
-
 
 class TestReproExternalS3(TestReproExternalBase):
     def should_test(self):

tests/test_run.py

@@ -9,6 +9,7 @@ import yaml
 
 from dvc.main import main
 from dvc.output import OutputBase
+from dvc.repo import Repo as DvcRepo
 from dvc.utils import file_md5, load_stage_file
 from dvc.system import System
 from dvc.stage import Stage, StagePathNotFoundError, StagePathNotDirectoryError
@@ -920,3 +921,33 @@ class TestNewRunShouldNotRemoveOutsOnPersist(TestRerunWithSameOutputs):
     @property
     def _outs_command(self):
         return "--outs-persist"
+
+
+class TestShouldNotCheckoutUponCorruptedLocalHardlinkCache(TestDvc):
+    def setUp(self):
+        super(
+            TestShouldNotCheckoutUponCorruptedLocalHardlinkCache, self
+        ).setUp()
+        ret = main(["config", "cache.type", "hardlink"])
+        self.assertEqual(ret, 0)
+        self.dvc = DvcRepo(".")
+
+    def test(self):
+        cmd = "cp {} {}".format(self.FOO, self.BAR)
+        stage = self.dvc.run(deps=[self.FOO], outs=[self.BAR], cmd=cmd)
+
+        with open(self.BAR, "w") as fd:
+            fd.write("corrupting the output cache")
+
+        patch_checkout = mock.patch.object(
+            stage.outs[0], "checkout", wraps=stage.outs[0].checkout
+        )
+        patch_run = mock.patch.object(stage, "_run", wraps=stage._run)
+
+        with self.dvc.state:
+            with patch_checkout as mock_checkout:
+                with patch_run as mock_run:
+                    stage.run()
+
+                    mock_run.assert_called_once()
+                    mock_checkout.assert_not_called()