diff --git a/dvc/exceptions.py b/dvc/exceptions.py index fbb8c7eaa..92c84ed0e 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -268,12 +268,6 @@ class FileMissingError(DvcException): ) -class PathOutsideRepoError(DvcException): - def __init__(self, path, repo): - msg = "The path '{}' does not exist in the target repository '{}'." - super(PathOutsideRepoError, self).__init__(msg.format(path, repo)) - - class DvcIgnoreInCollectedDirError(DvcException): def __init__(self, ignore_dirname): super(DvcIgnoreInCollectedDirError, self).__init__( @@ -289,14 +283,6 @@ class UrlNotDvcRepoError(DvcException): ) -class GetDVCFileError(DvcException): - def __init__(self): - super(GetDVCFileError, self).__init__( - "the given path is a DVC-file, you must specify a data file " - "or a directory" - ) - - class GitHookAlreadyExistsError(DvcException): def __init__(self, hook_name): super(GitHookAlreadyExistsError, self).__init__( diff --git a/dvc/repo/get.py b/dvc/repo/get.py index 7fbe6a5d7..60c4c2e4a 100644 --- a/dvc/repo/get.py +++ b/dvc/repo/get.py @@ -1,37 +1,40 @@ import logging import os import shutil +from dvc.utils.compat import FileNotFoundError import shortuuid -from dvc.exceptions import GetDVCFileError -from dvc.exceptions import NotDvcRepoError -from dvc.exceptions import OutputNotFoundError -from dvc.exceptions import UrlNotDvcRepoError -from dvc.exceptions import PathOutsideRepoError +from dvc.exceptions import ( + DvcException, + NotDvcRepoError, + OutputNotFoundError, + UrlNotDvcRepoError, +) from dvc.external_repo import external_repo from dvc.path_info import PathInfo from dvc.stage import Stage -from dvc.state import StateNoop from dvc.utils import resolve_output from dvc.utils.fs import remove -from dvc.utils.compat import FileNotFoundError logger = logging.getLogger(__name__) -def _copy_git_file(repo, src, dst, repo_url): - src_full_path = os.path.join(repo.root_dir, src) - dst_full_path = os.path.abspath(dst) +class GetDVCFileError(DvcException): + def __init__(self): + super(GetDVCFileError, self).__init__( + "the given path is a DVC-file, you must specify a data file " + "or a directory" + ) - if os.path.isdir(src_full_path): - shutil.copytree(src_full_path, dst_full_path) - return - try: - shutil.copy2(src_full_path, dst_full_path) - except FileNotFoundError: - raise PathOutsideRepoError(src, repo_url) +class PathMissingError(DvcException): + def __init__(self, path, repo): + msg = ( + "The path '{}' does not exist in the target repository '{}'" + " neighther as an output nor a git-handled file." + ) + super(PathMissingError, self).__init__(msg.format(path, repo)) @staticmethod @@ -50,11 +53,6 @@ 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() - # Try any links possible to avoid data duplication. # # Not using symlink, because we need to remove cache after we are @@ -66,31 +64,40 @@ def get(url, path, out=None, rev=None): # the same cache file might be used a few times in a directory. repo.cache.local.cache_types = ["reflink", "hardlink", "copy"] - output = None - output_error = None - try: output = repo.find_out_by_relpath(path) - except OutputNotFoundError as ex: - output_error = ex + except OutputNotFoundError: + output = None - is_git_file = output_error and not os.path.isabs(path) - is_not_cached = output and not output.use_cache + if output and output.use_cache: + _get_cached(repo, output, out) + else: + # Either an uncached out with absolute path or a user error + if os.path.isabs(path): + raise FileNotFoundError - if is_git_file or is_not_cached: - _copy_git_file(repo, path, out, url) - return - - if output_error: - raise OutputNotFoundError(path) - - with repo.state: - repo.cloud.pull(output.get_used_cache()) - output.path_info = PathInfo(os.path.abspath(out)) - with output.repo.state: - output.checkout() + _copy(os.path.join(repo.root_dir, path), out) + except (OutputNotFoundError, FileNotFoundError): + raise PathMissingError(path, url) except NotDvcRepoError: raise UrlNotDvcRepoError(url) finally: remove(tmp_dir) + + +def _get_cached(repo, output, out): + with repo.state: + repo.cloud.pull(output.get_used_cache()) + output.path_info = PathInfo(os.path.abspath(out)) + failed = output.checkout() + # This might happen when pull haven't really pulled all the files + if failed: + raise FileNotFoundError + + +def _copy(src, dst): + if os.path.isdir(src): + shutil.copytree(src, dst) + else: + shutil.copy2(src, dst) diff --git a/tests/func/test_get.py b/tests/func/test_get.py index 11f70ecc3..a48e76bd9 100644 --- a/tests/func/test_get.py +++ b/tests/func/test_get.py @@ -7,10 +7,8 @@ import os import pytest from dvc.config import Config -from dvc.exceptions import GetDVCFileError from dvc.exceptions import UrlNotDvcRepoError -from dvc.exceptions import OutputNotFoundError -from dvc.exceptions import PathOutsideRepoError +from dvc.repo.get import GetDVCFileError, PathMissingError from dvc.repo import Repo from dvc.system import System from dvc.utils import makedirs @@ -145,14 +143,14 @@ def test_non_cached_output(tmp_path, erepo): # https://github.com/iterative/dvc/pull/2837#discussion_r352123053 -def test_fails_with_files_outside_repo(erepo): - with pytest.raises(OutputNotFoundError): +def test_absolute_file_outside_repo(erepo): + with pytest.raises(PathMissingError): Repo.get(erepo.root_dir, "/root/") -def test_fails_with_non_existing_files(erepo): - with pytest.raises(PathOutsideRepoError): - Repo.get(erepo.root_dir, "file_does_not_exist") +def test_unknown_path(erepo): + with pytest.raises(PathMissingError): + Repo.get(erepo.root_dir, "a_non_existing_file") @pytest.mark.parametrize("dname", [".", "dir", "dir/subdir"])