Merge pull request #2798 from skshetry/fix-str-rel-repo

Display path relative to temporary repo when get/import-ing files

dvc/output/local.py

@@ -7,9 +7,11 @@
 from dvc.istextfile import istextfile
 from dvc.output.base import OutputBase
 from dvc.remote.local import RemoteLOCAL
+from dvc.utils import relpath
 from dvc.utils.compat import fspath_py35
 from dvc.utils.compat import str
 from dvc.utils.compat import urlparse
+from dvc.utils.fs import path_isin
 
 
 logger = logging.getLogger(__name__)
@@ -38,7 +40,14 @@ def _parse_path(self, remote, path):
         return self.REMOTE.path_cls(abs_p)
 
     def __str__(self):
-        return str(self.path_info)
+        if not self.is_in_repo:
+            return str(self.def_path)
+
+        cur_dir = os.getcwd()
+        if path_isin(cur_dir, self.repo.root_dir):
+            return relpath(self.path_info, cur_dir)
+
+        return relpath(self.path_info, self.repo.root_dir)
 
     @property
     def fspath(self):

dvc/utils/fs.py

@@ -134,3 +134,14 @@ def remove(path):
     except OSError as exc:
         if exc.errno != errno.ENOENT:
             raise
+
+
+def path_isin(child, parent):
+    """Check if given `child` path is inside `parent`."""
+
+    def normalize_path(path):
+        return os.path.normpath(fspath_py35(path))
+
+    parent = os.path.join(normalize_path(parent), "")
+    child = normalize_path(child)
+    return child != parent and child.startswith(parent)

tests/unit/output/test_local.py

@@ -1,8 +1,10 @@
-from mock import patch
+import os
 
+from mock import patch
 from dvc.output import OutputLOCAL
 from dvc.remote.local import RemoteLOCAL
 from dvc.stage import Stage
+from dvc.utils import relpath
 from tests.basic_env import TestDvc
 
 
@@ -21,6 +23,34 @@ def test_save_missing(self):
                 o.save()
 
 
+def test_str_workdir_outside_repo(erepo):
+    stage = Stage(erepo.dvc)
+    output = OutputLOCAL(stage, "path", cache=False)
+
+    assert relpath("path", erepo.dvc.root_dir) == str(output)
+
+
+def test_str_workdir_inside_repo(dvc_repo):
+    stage = Stage(dvc_repo)
+    output = OutputLOCAL(stage, "path", cache=False)
+
+    assert "path" == str(output)
+
+    stage = Stage(dvc_repo, wdir="some_folder")
+    output = OutputLOCAL(stage, "path", cache=False)
+
+    assert os.path.join("some_folder", "path") == str(output)
+
+
+def test_str_on_absolute_path(dvc_repo):
+    stage = Stage(dvc_repo)
+
+    path = os.path.abspath(os.path.join("path", "to", "file"))
+    output = OutputLOCAL(stage, path, cache=False)
+
+    assert path == str(output)
+
+
 class TestGetFilesNumber(TestDvc):
     def _get_output(self):
         stage = Stage(self.dvc)

tests/unit/utils/test_fs.py

@@ -16,7 +16,7 @@
 from dvc.utils.fs import get_inode
 from dvc.utils.fs import get_mtime_and_size
 from dvc.utils.fs import move
-from dvc.utils.fs import remove
+from dvc.utils.fs import path_isin, remove
 from tests.basic_env import TestDir
 from tests.utils import spy
 
@@ -164,3 +164,44 @@ def test_remove(repo_dir):
 
     remove(path_info)
     assert not os.path.isfile(path_info.fspath)
+
+
+def test_path_isin_positive():
+    child = os.path.join("path", "to", "folder")
+
+    assert path_isin(child, os.path.join("path", "to", ""))
+    assert path_isin(child, os.path.join("path", "to"))
+    assert path_isin(child, os.path.join("path", ""))
+    assert path_isin(child, os.path.join("path"))
+
+
+def test_path_isin_on_same_path():
+    path = os.path.join("path", "to", "folder")
+    path_with_sep = os.path.join(path, "")
+
+    assert not path_isin(path, path)
+    assert not path_isin(path, path_with_sep)
+    assert not path_isin(path_with_sep, path)
+    assert not path_isin(path_with_sep, path_with_sep)
+
+
+def test_path_isin_on_common_substring_path():
+    path1 = os.path.join("path", "to", "folder1")
+    path2 = os.path.join("path", "to", "folder")
+
+    assert not path_isin(path1, path2)
+
+
+def test_path_isin_accepts_pathinfo():
+    child = os.path.join("path", "to", "folder")
+    parent = PathInfo(child) / ".."
+
+    assert path_isin(child, parent)
+    assert not path_isin(parent, child)
+
+
+def test_path_isin_with_absolute_path():
+    parent = os.path.abspath("path")
+    child = os.path.join(parent, "to", "folder")
+
+    assert path_isin(child, parent)