stage: check if local path contains symlink and, if so, treat added files as relpath

tests/test_add.py

@@ -7,7 +7,9 @@ import yaml
 import time
 import shutil
 import filecmp
+import posixpath
 
+from dvc.system import System
 from mock import patch
 
 from dvc.main import main
@@ -331,3 +333,76 @@ class TestShouldCollectDirCacheOnlyOnce(TestDvc):
             ret = main(["status"])
             self.assertEqual(0, ret)
         self.assertEqual(1, collect_dir_counter.mock.call_count)
+
+
+class SymlinkAddTestBase(TestDvc):
+    def _get_data_dir(self):
+        raise NotImplementedError
+
+    def _prepare_external_data(self):
+        data_dir = self._get_data_dir()
+
+        self.data_file_name = "data_file"
+        external_data_path = os.path.join(data_dir, self.data_file_name)
+        with open(external_data_path, "w+") as f:
+            f.write("data")
+
+        self.link_name = "data_link"
+        System.symlink(data_dir, self.link_name)
+
+    def _test(self):
+        self._prepare_external_data()
+
+        ret = main(["add", os.path.join(self.link_name, self.data_file_name)])
+        self.assertEqual(0, ret)
+
+        stage_file = self.data_file_name + Stage.STAGE_FILE_SUFFIX
+        self.assertTrue(os.path.exists(stage_file))
+
+        with open(stage_file, "r") as fobj:
+            d = yaml.safe_load(fobj)
+        relative_data_path = posixpath.join(
+            self.link_name, self.data_file_name
+        )
+        self.assertEqual(relative_data_path, d["outs"][0]["path"])
+
+
+class TestShouldAddDataFromExternalSymlink(SymlinkAddTestBase):
+    def _get_data_dir(self):
+        return self.mkdtemp()
+
+    def test(self):
+        self._test()
+
+
+class TestShouldAddDataFromInternalSymlink(SymlinkAddTestBase):
+    def _get_data_dir(self):
+        return self.DATA_DIR
+
+    def test(self):
+        self._test()
+
+
+class TestShouldPlaceStageInDataDirIfRepositoryBelowSymlink(TestDvc):
+    def test(self):
+        def is_symlink_true_below_dvc_root(path):
+            if path == os.path.dirname(self.dvc.root_dir):
+                return True
+            return False
+
+        with patch.object(
+            System, "is_symlink", side_effect=is_symlink_true_below_dvc_root
+        ):
+
+            ret = main(["add", self.DATA])
+            self.assertEqual(0, ret)
+
+            stage_file_path_on_data_below_symlink = (
+                os.path.basename(self.DATA) + Stage.STAGE_FILE_SUFFIX
+            )
+            self.assertFalse(
+                os.path.exists(stage_file_path_on_data_below_symlink)
+            )
+
+            stage_file_path = self.DATA + Stage.STAGE_FILE_SUFFIX
+            self.assertTrue(os.path.exists(stage_file_path))

tests/unit/fs.py

@@ -1,26 +0,0 @@
-import os
-
-from dvc.utils.compat import str
-from dvc.utils.fs import get_mtime_and_size
-from tests.basic_env import TestDir
-
-
-class TestMtimeAndSize(TestDir):
-    def test(self):
-        file_time, file_size = get_mtime_and_size(self.DATA)
-        dir_time, dir_size = get_mtime_and_size(self.DATA_DIR)
-
-        actual_file_size = os.path.getsize(self.DATA)
-        actual_dir_size = (
-            os.path.getsize(self.DATA_DIR)
-            + os.path.getsize(self.DATA)
-            + os.path.getsize(self.DATA_SUB_DIR)
-            + os.path.getsize(self.DATA_SUB)
-        )
-
-        self.assertIs(type(file_time), str)
-        self.assertIs(type(file_size), str)
-        self.assertEqual(file_size, str(actual_file_size))
-        self.assertIs(type(dir_time), str)
-        self.assertIs(type(dir_size), str)
-        self.assertEqual(dir_size, str(actual_dir_size))

tests/unit/utils/fs.py

@@ -0,0 +1,87 @@
+import os
+from unittest import TestCase
+
+import dvc
+from dvc.system import System
+from dvc.utils.compat import str
+from dvc.utils.fs import (
+    get_mtime_and_size,
+    contains_symlink_up_to,
+    BasePathNotInCheckedPathException,
+)
+from mock import patch
+from tests.basic_env import TestDir
+from tests.utils import spy
+
+
+class TestMtimeAndSize(TestDir):
+    def test(self):
+        file_time, file_size = get_mtime_and_size(self.DATA)
+        dir_time, dir_size = get_mtime_and_size(self.DATA_DIR)
+
+        actual_file_size = os.path.getsize(self.DATA)
+        actual_dir_size = (
+            os.path.getsize(self.DATA_DIR)
+            + os.path.getsize(self.DATA)
+            + os.path.getsize(self.DATA_SUB_DIR)
+            + os.path.getsize(self.DATA_SUB)
+        )
+
+        self.assertIs(type(file_time), str)
+        self.assertIs(type(file_size), str)
+        self.assertEqual(file_size, str(actual_file_size))
+        self.assertIs(type(dir_time), str)
+        self.assertIs(type(dir_size), str)
+        self.assertEqual(dir_size, str(actual_dir_size))
+
+
+class TestContainsLink(TestCase):
+    def test_should_raise_exception_on_base_path_not_in_path(self):
+        with self.assertRaises(BasePathNotInCheckedPathException):
+            contains_symlink_up_to(os.path.join("foo", "path"), "bar")
+
+    @patch.object(System, "is_symlink", return_value=True)
+    def test_should_return_true_on_symlink_in_path(self, _):
+        base_path = "foo"
+        path = os.path.join(base_path, "bar")
+        self.assertTrue(contains_symlink_up_to(path, base_path))
+
+    @patch.object(System, "is_symlink", return_value=False)
+    def test_should_return_false_on_path_eq_to_base_path(self, _):
+        path = "path"
+        self.assertFalse(contains_symlink_up_to(path, path))
+
+    @patch.object(System, "is_symlink", return_value=False)
+    @patch.object(os.path, "dirname", side_effect=lambda arg: arg)
+    def test_should_return_false_on_no_more_dirs_below_path(
+        self, dirname_patch, _
+    ):
+        self.assertFalse(contains_symlink_up_to("path"))
+        dirname_patch.assert_called_once()
+
+    @patch.object(System, "is_symlink", return_value=False)
+    @patch.object(os.path, "dirname", return_value="")
+    def test_should_call_recursive_on_no_condition_matched(self, _, __):
+        contains_symlink_spy = spy(contains_symlink_up_to)
+        with patch.object(
+            dvc.utils.fs, "contains_symlink_up_to", contains_symlink_spy
+        ):
+
+            # call from full path to match contains_symlink_spy patch path
+            self.assertFalse(dvc.utils.fs.contains_symlink_up_to("path"))
+            self.assertEqual(2, contains_symlink_spy.mock.call_count)
+
+    @patch.object(System, "is_symlink", return_value=True)
+    def test_should_return_false_when_base_path_is_symlink(self, _):
+        base_path = "foo"
+        target_path = os.path.join(base_path, "bar")
+
+        def base_path_is_symlink(path):
+            if path == base_path:
+                return True
+            return False
+
+        with patch.object(
+            System, "is_symlink", side_effect=base_path_is_symlink
+        ):
+            self.assertFalse(contains_symlink_up_to(target_path, base_path))