From 3c2633e33ffe6eee3217c321f142ed4a221309c4 Mon Sep 17 00:00:00 2001 From: Dani Hodovic Date: Wed, 4 Dec 2019 22:59:55 +0100 Subject: [PATCH] Pass all tests --- dvc/exceptions.py | 7 ------ dvc/repo/get.py | 52 ++++++++++++++++++++---------------------- tests/func/test_get.py | 20 ++++++++++++++-- 3 files changed, 43 insertions(+), 36 deletions(-) diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 237b0d2e5..e5fb6f00d 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -268,13 +268,6 @@ class FileMissingError(DvcException): ) -class PathOutsideRepoError(DvcException): - def __init__(self, path): - super(PathOutsideRepoError, self).__init__( - "The path '{}' is outside the target repository.".format(path) - ) - - class DvcIgnoreInCollectedDirError(DvcException): def __init__(self, ignore_dirname): super(DvcIgnoreInCollectedDirError, self).__init__( diff --git a/dvc/repo/get.py b/dvc/repo/get.py index f986b31d1..9409e2ad8 100644 --- a/dvc/repo/get.py +++ b/dvc/repo/get.py @@ -8,8 +8,6 @@ from dvc.exceptions import GetDVCFileError from dvc.exceptions import NotDvcRepoError from dvc.exceptions import OutputNotFoundError from dvc.exceptions import UrlNotDvcRepoError -from dvc.exceptions import NoOutputInExternalRepoError -from dvc.exceptions import PathOutsideRepoError from dvc.external_repo import external_repo from dvc.path_info import PathInfo from dvc.stage import Stage @@ -25,9 +23,6 @@ def _copy_git_file(repo, src, dst): src_full_path = os.path.join(repo.root_dir, src) dst_full_path = os.path.abspath(dst) - if not path_isin(src_full_path, repo.root_dir): - raise PathOutsideRepoError(src) - if os.path.isdir(src_full_path): shutil.copytree(src_full_path, dst_full_path) else: @@ -50,31 +45,34 @@ def get(url, path, out=None, rev=None): tmp_dir = os.path.join(dpath, "." + str(shortuuid.uuid())) try: with external_repo(cache_dir=tmp_dir, url=url, rev=rev) as repo: - # 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() + full_path = os.path.join(repo.root_dir, path) + path_in_repo = path_isin(full_path, repo.root_dir) + if os.path.exists(full_path) and path_in_repo: + _copy_git_file(repo, path, out) + 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() - # 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 5d519194d..d9e098c8e 100644 --- a/tests/func/test_get.py +++ b/tests/func/test_get.py @@ -3,13 +3,14 @@ from __future__ import unicode_literals import filecmp import logging import os +import tempfile import pytest from dvc.config import Config from dvc.exceptions import GetDVCFileError from dvc.exceptions import UrlNotDvcRepoError -from dvc.exceptions import PathOutsideRepoError +from dvc.exceptions import NoOutputInExternalRepoError from dvc.repo import Repo from dvc.system import System from dvc.utils import makedirs @@ -127,9 +128,24 @@ def test_get_full_dvc_path(erepo): assert filecmp.cmp(external_data, "ext_data_imported", shallow=False) +def test_non_cached_output(request, erepo): + os.chdir(erepo.root_dir) + erepo.dvc.run( + outs_no_cache=["non_cached_file"], cmd="echo hello > non_cached_file" + ) + erepo.dvc.scm.add(["non_cached_file", "non_cached_file.dvc"]) + erepo.dvc.scm.commit("add non-cached output") + new_dir = tempfile.mkdtemp(prefix=request.node.name + "_") + os.chdir(new_dir) + Repo.get(erepo.root_dir, "non_cached_file") + assert os.path.isfile("non_cached_file") + src = os.path.join(erepo.root_dir, "non_cached_file") + assert filecmp.cmp(src, "non_cached_file", shallow=False) + + # https://github.com/iterative/dvc/pull/2837#discussion_r352123053 def test_fails_with_non_git_files(erepo): - with pytest.raises(PathOutsideRepoError): + with pytest.raises(NoOutputInExternalRepoError): Repo.get(erepo.root_dir, "/root/")