ensure absolute paths for dvc files work but not for git

dvc/exceptions.py

@@ -268,12 +268,10 @@ class FileMissingError(DvcException):
         )
 
 
-class NoAbsolutePathError(DvcException):
+class FileOutsideRepoError(DvcException):
     def __init__(self, path, cause=None):
-        super(NoAbsolutePathError, self).__init__(
-            "The path '{}' is an absolute path. ".format(path)
-            + "Provide a relative path.",
-            cause=cause,
+        super(FileOutsideRepoError, self).__init__(
+            "The path '{}' is outside the target repository.".format(path)
         )
 
 

dvc/repo/get.py

@@ -8,7 +8,8 @@ from dvc.exceptions import GetDVCFileError
 from dvc.exceptions import NotDvcRepoError
 from dvc.exceptions import OutputNotFoundError
 from dvc.exceptions import UrlNotDvcRepoError
-from dvc.exceptions import NoAbsolutePathError
+from dvc.exceptions import NoOutputInExternalRepoError
+from dvc.exceptions import FileOutsideRepoError
 from dvc.external_repo import external_repo
 from dvc.path_info import PathInfo
 from dvc.stage import Stage
@@ -19,11 +20,24 @@ from dvc.utils.fs import remove
 logger = logging.getLogger(__name__)
 
 
+def copy_git_file(repo, src, dst):
+    try:
+        # Assert that the file does indeed exist in git before attempting to
+        # copy it.
+        repo.scm.repo.head.commit.tree[src]  # noqa
+    except KeyError:
+        raise FileOutsideRepoError(src)
+
+    src_full_path = os.path.join(repo.root_dir, src)
+    dst_full_path = os.path.abspath(dst)
+    if os.path.isdir(src_full_path):
+        shutil.copytree(src_full_path, dst_full_path)
+    else:
+        shutil.copy2(src_full_path, dst_full_path)
+
+
 @staticmethod
 def get(url, path, out=None, rev=None):
-    if os.path.isabs(path):
-        raise NoAbsolutePathError(path)
-
     out = resolve_output(path, out)
 
     if Stage.is_valid_filename(out):
@@ -34,42 +48,35 @@ def get(url, path, out=None, rev=None):
     # reflink and/or hardlink. Not using tempfile.TemporaryDirectory
     # because it will create a symlink to tmpfs, which defeats the purpose
     # and won't work with reflink/hardlink.
-    abspath = os.path.abspath(out)
-    dpath = os.path.dirname(abspath)
+    dpath = os.path.dirname(os.path.abspath(out))
     tmp_dir = os.path.join(dpath, "." + str(shortuuid.uuid()))
     try:
         with external_repo(cache_dir=tmp_dir, url=url, rev=rev) as repo:
-            tmp_repo_path = os.path.join(repo.root_dir, path)
-            if os.path.exists(tmp_repo_path):
-                if os.path.isdir(tmp_repo_path):
-                    shutil.copytree(tmp_repo_path, abspath)
-                else:
-                    shutil.copy2(tmp_repo_path, abspath)
-            else:
-                # Note: we need to replace state, because in case of getting
-                # DVC dependency on CIFS or NFS filesystems, sqlite-based state
-                # will be unable to obtain lock
-                repo.state = StateNoop()
+            # Note: we need to replace state, because in case of getting DVC
+            # dependency on CIFS or NFS filesystems, sqlite-based state
+            # will be unable to obtain lock
+            repo.state = StateNoop()
 
-                # Try any links possible to avoid data duplication.
-                #
-                # Not using symlink, because we need to remove cache after we
-                # are done, and to make that work we would have to copy data
-                # over anyway before removing the cache, so we might just copy
-                # it right away.
-                #
-                # Also, we can't use theoretical "move" link type here, because
-                # the same cache file might be used a few times in a directory.
-                repo.cache.local.cache_types = ["reflink", "hardlink", "copy"]
+            # Try any links possible to avoid data duplication.
+            #
+            # Not using symlink, because we need to remove cache after we are
+            # done, and to make that work we would have to copy data over
+            # anyway before removing the cache, so we might just copy it
+            # right away.
+            #
+            # Also, we can't use theoretical "move" link type here, because
+            # the same cache file might be used a few times in a directory.
+            repo.cache.local.cache_types = ["reflink", "hardlink", "copy"]
 
-                o = repo.find_out_by_relpath(path)
-
-                with repo.state:
-                    repo.cloud.pull(o.get_used_cache())
-                o.path_info = PathInfo(os.path.abspath(out))
-                with o.repo.state:
-                    o.checkout()
+            o = repo.find_out_by_relpath(path)
+            with repo.state:
+                repo.cloud.pull(o.get_used_cache())
+            o.path_info = PathInfo(os.path.abspath(out))
+            with o.repo.state:
+                o.checkout()
 
+    except NoOutputInExternalRepoError:
+        copy_git_file(repo, path, out)
     except NotDvcRepoError:
         raise UrlNotDvcRepoError(url)
     except OutputNotFoundError:

tests/func/test_get.py

@@ -9,7 +9,7 @@ import pytest
 from dvc.config import Config
 from dvc.exceptions import GetDVCFileError
 from dvc.exceptions import UrlNotDvcRepoError
-from dvc.exceptions import NoAbsolutePathError
+from dvc.exceptions import FileOutsideRepoError
 from dvc.repo import Repo
 from dvc.system import System
 from dvc.utils import makedirs
@@ -108,10 +108,28 @@ def test_get_a_dvc_file(erepo):
         Repo.get(erepo.root_dir, "some_file.dvc")
 
 
-# Prevent `Repo.get()` from copying any file on the filesystem
 # https://github.com/iterative/dvc/pull/2837#discussion_r352123053
-def test_fails_with_absolute_paths(erepo):
-    with pytest.raises(NoAbsolutePathError):
+def test_get_full_dvc_path(erepo):
+    external_data_dir = erepo.mkdtemp()
+    external_data = os.path.join(external_data_dir, "ext_data")
+    with open(external_data, "w+") as fobj:
+        fobj.write("ext_data")
+
+    cur_dir = os.getcwd()
+    os.chdir(erepo.root_dir)
+    erepo.dvc.add(external_data)
+    erepo.dvc.scm.add(["ext_data.dvc"])
+    erepo.dvc.scm.commit("add external data")
+    os.chdir(cur_dir)
+
+    Repo.get(erepo.root_dir, external_data, "ext_data_imported")
+    assert os.path.isfile("ext_data_imported")
+    assert filecmp.cmp(external_data, "ext_data_imported", shallow=False)
+
+
+# https://github.com/iterative/dvc/pull/2837#discussion_r352123053
+def test_fails_with_non_git_files(erepo):
+    with pytest.raises(FileOutsideRepoError):
         Repo.get(erepo.root_dir, "/root/")