diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 1ced27032..6909bcea0 100644 --- a/dvc/exceptions.py +++ b/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) ) diff --git a/dvc/repo/get.py b/dvc/repo/get.py index bb2482302..24d1ae81a 100644 --- a/dvc/repo/get.py +++ b/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: diff --git a/tests/func/test_get.py b/tests/func/test_get.py index 11ab26faf..cf35f4bc5 100644 --- a/tests/func/test_get.py +++ b/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/")