diff --git a/dvc/external_repo.py b/dvc/external_repo.py index 965f0acd5..cbe3d6531 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -22,6 +22,7 @@ from dvc.scm.base import CloneError from dvc.scm.git import Git from dvc.tree.local import LocalTree from dvc.tree.repo import RepoTree +from dvc.utils import relpath from dvc.utils.fs import remove logger = logging.getLogger(__name__) @@ -365,6 +366,11 @@ def _remove(path): if os.name == "nt": # git.exe may hang for a while not permitting to remove temp dir os_retry = retry(5, errors=OSError, timeout=0.1) - os_retry(remove)(path) + try: + os_retry(remove)(path) + except PermissionError: + logger.warning( + "Failed to remove '%s'", relpath(path), exc_info=True + ) else: remove(path) diff --git a/dvc/tree/git.py b/dvc/tree/git.py index e3d72029f..03b86c379 100644 --- a/dvc/tree/git.py +++ b/dvc/tree/git.py @@ -1,6 +1,7 @@ import errno import io import os +from functools import lru_cache from funcy import cached_property @@ -20,6 +21,30 @@ def _item_basename(item): return os.path.basename(item.path) +def _is_tree_and_contains(obj, path): + if obj.mode != GIT_MODE_DIR: + return False + # see https://github.com/gitpython-developers/GitPython/issues/851 + # `return (i in tree)` doesn't work so here is a workaround: + for item in obj: + if _item_basename(item) == path: + return True + return False + + +# NOTE: small lru cache covers sequential exists() calls +@lru_cache(maxsize=1) +def _get_obj(tree, path): + if not path or path == ".": + return tree + for i in path.split(os.sep): + if not _is_tree_and_contains(tree, i): + # there is no tree for specified path + return None + tree = tree[i] + return tree + + class GitTree(BaseTree): # pylint:disable=abstract-method """Proxies the repo file access methods to Git objects""" @@ -102,27 +127,12 @@ class GitTree(BaseTree): # pylint:disable=abstract-method return False return not self.dvcignore.is_ignored_file(path) - @staticmethod - def _is_tree_and_contains(obj, path): - if obj.mode != GIT_MODE_DIR: - return False - # see https://github.com/gitpython-developers/GitPython/issues/851 - # `return (i in tree)` doesn't work so here is a workaround: - for item in obj: - if _item_basename(item) == path: - return True - return False - - def _git_object_by_path(self, path): + @cached_property + def _tree(self): import git - path = relpath(os.path.realpath(path), self.git.working_dir) - if path.split(os.sep, 1)[0] == "..": - # path points outside of git repository - return None - try: - tree = self.git.tree(self.rev) + return self.git.tree(self.rev) except git.exc.BadName as exc: # pylint: disable=no-member raise DvcException( "revision '{}' not found in Git '{}'".format( @@ -130,14 +140,13 @@ class GitTree(BaseTree): # pylint:disable=abstract-method ) ) from exc - if not path or path == ".": - return tree - for i in path.split(os.sep): - if not self._is_tree_and_contains(tree, i): - # there is no tree for specified path - return None - tree = tree[i] - return tree + def _git_object_by_path(self, path): + path = relpath(os.path.realpath(path), self.git.working_dir) + if path.split(os.sep, 1)[0] == "..": + # path points outside of git repository + return None + + return _get_obj(self._tree, path) def _walk(self, tree, topdown=True): dirs, nondirs = [], [] diff --git a/tests/func/test_tree.py b/tests/func/test_tree.py index abe002859..149d3baa4 100644 --- a/tests/func/test_tree.py +++ b/tests/func/test_tree.py @@ -44,53 +44,60 @@ class GitTreeTests: def test_open(self): self.scm.add([self.FOO, self.UNICODE, self.DATA_DIR]) self.scm.commit("add") - with self.tree.open(self.FOO) as fd: + + tree = GitTree(self.git, "master") + with tree.open(self.FOO) as fd: self.assertEqual(fd.read(), self.FOO_CONTENTS) - with self.tree.open(self.UNICODE) as fd: + with tree.open(self.UNICODE) as fd: self.assertEqual(fd.read(), self.UNICODE_CONTENTS) with self.assertRaises(IOError): - self.tree.open("not-existing-file") + tree.open("not-existing-file") with self.assertRaises(IOError): - self.tree.open(self.DATA_DIR) + tree.open(self.DATA_DIR) def test_exists(self): - self.assertFalse(self.tree.exists(self.FOO)) - self.assertFalse(self.tree.exists(self.UNICODE)) - self.assertFalse(self.tree.exists(self.DATA_DIR)) + tree = GitTree(self.git, "master") + self.assertFalse(tree.exists(self.FOO)) + self.assertFalse(tree.exists(self.UNICODE)) + self.assertFalse(tree.exists(self.DATA_DIR)) self.scm.add([self.FOO, self.UNICODE, self.DATA]) self.scm.commit("add") - self.assertTrue(self.tree.exists(self.FOO)) - self.assertTrue(self.tree.exists(self.UNICODE)) - self.assertTrue(self.tree.exists(self.DATA_DIR)) - self.assertFalse(self.tree.exists("non-existing-file")) + + tree = GitTree(self.git, "master") + self.assertTrue(tree.exists(self.FOO)) + self.assertTrue(tree.exists(self.UNICODE)) + self.assertTrue(tree.exists(self.DATA_DIR)) + self.assertFalse(tree.exists("non-existing-file")) def test_isdir(self): self.scm.add([self.FOO, self.DATA_DIR]) self.scm.commit("add") - self.assertTrue(self.tree.isdir(self.DATA_DIR)) - self.assertFalse(self.tree.isdir(self.FOO)) - self.assertFalse(self.tree.isdir("non-existing-file")) + + tree = GitTree(self.git, "master") + self.assertTrue(tree.isdir(self.DATA_DIR)) + self.assertFalse(tree.isdir(self.FOO)) + self.assertFalse(tree.isdir("non-existing-file")) def test_isfile(self): self.scm.add([self.FOO, self.DATA_DIR]) self.scm.commit("add") - self.assertTrue(self.tree.isfile(self.FOO)) - self.assertFalse(self.tree.isfile(self.DATA_DIR)) - self.assertFalse(self.tree.isfile("not-existing-file")) + + tree = GitTree(self.git, "master") + self.assertTrue(tree.isfile(self.FOO)) + self.assertFalse(tree.isfile(self.DATA_DIR)) + self.assertFalse(tree.isfile("not-existing-file")) class TestGitTree(TestGit, GitTreeTests): def setUp(self): super().setUp() self.scm = SCM(self._root_dir) - self.tree = GitTree(self.git, "master") class TestGitSubmoduleTree(TestGitSubmodule, GitTreeTests): def setUp(self): super().setUp() self.scm = SCM(self._root_dir) - self.tree = GitTree(self.git, "master") self._pushd(self._root_dir)