diff --git a/dvc/external_repo.py b/dvc/external_repo.py index b8061a94c..ba470140c 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -5,7 +5,7 @@ import threading from contextlib import contextmanager from typing import Dict -from funcy import cached_property, retry, wrap_with +from funcy import retry, wrap_with from dvc.exceptions import ( FileMissingError, @@ -22,7 +22,9 @@ logger = logging.getLogger(__name__) @contextmanager -def external_repo(url, rev=None, for_write=False, cache_dir=None, **kwargs): +def external_repo( + url, rev=None, for_write=False, cache_dir=None, cache_types=None, **kwargs +): from dvc.config import NoRemoteError from dvc.scm.git import Git @@ -33,22 +35,41 @@ def external_repo(url, rev=None, for_write=False, cache_dir=None, **kwargs): # the tip of the default branch rev = rev or "refs/remotes/origin/HEAD" + cache_config = { + "cache": { + "dir": cache_dir or _get_cache_dir(url), + "type": cache_types, + } + } + + config = _get_remote_config(url) if os.path.isdir(url) else {} + config.update(cache_config) + + def make_repo(path, **_kwargs): + _config = cache_config.copy() + if os.path.isdir(url): + rel = os.path.relpath(path, _kwargs["scm"].root_dir) + repo_path = os.path.join(url, rel) + _config.update(_get_remote_config(repo_path)) + return Repo(path, config=_config, **_kwargs) + root_dir = path if for_write else os.path.realpath(path) - conf = dict( + repo_kwargs = dict( root_dir=root_dir, url=url, scm=None if for_write else Git(root_dir), rev=None if for_write else rev, subrepos=not for_write, uninitialized=True, - cache_dir=cache_dir or _get_cache_dir(url), + config=config, + repo_factory=make_repo, **kwargs, ) - if "fetch" not in conf: - conf["fetch"] = True + if "fetch" not in repo_kwargs: + repo_kwargs["fetch"] = True - repo = ExternalRepo(**conf) + repo = Repo(**repo_kwargs) try: yield repo @@ -115,62 +136,6 @@ def _get_remote_config(url): repo.close() -class ExternalRepo(Repo): - # pylint: disable=no-member - - def __init__( - self, - root_dir, - url, - scm=None, - rev=None, - cache_dir=None, - cache_types=None, - uninitialized=False, - subrepos=False, - **kwargs, - ): - self.url = url - self.tree_confs = kwargs - - self._cache_config = {"cache": {"dir": cache_dir, "type": cache_types}} - - config = self._cache_config.copy() - if os.path.isdir(url): - config.update(_get_remote_config(url)) - - super().__init__( - root_dir, - scm=scm, - rev=rev, - uninitialized=uninitialized, - config=config, - subrepos=subrepos, - ) - - def __str__(self): - return self.url - - @cached_property - def repo_tree(self): - from dvc.tree.repo import RepoTree - - return RepoTree( - self, - subrepos=self.subrepos, - repo_factory=self.make_repo, - **self.tree_confs, - ) - - def make_repo(self, path): - config = self._cache_config.copy() - if os.path.isdir(self.url): - rel = os.path.relpath(path, self.root_dir) - repo_path = os.path.join(self.url, rel) - config.update(_get_remote_config(repo_path)) - return Repo(path, scm=self.scm, rev=self.get_rev(), config=config) - - def _cached_clone(url, rev, for_write=False): """Clone an external git repo to a temporary directory. diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 8f2278918..797d427f1 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -128,6 +128,10 @@ class Repo: subrepos=False, uninitialized=False, config=None, + url=None, + repo_factory=None, + fetch=None, + stream=None, ): from dvc.cache import Cache from dvc.config import Config @@ -142,6 +146,13 @@ class Repo: from dvc.state import State, StateNoop from dvc.tree.local import LocalTree + self.url = url + self._tree_conf = { + "stream": stream, + "fetch": fetch, + "repo_factory": repo_factory, + } + self.root_dir, self.dvc_dir, self.tmp_dir = self._get_repo_dirs( root_dir=root_dir, scm=scm, rev=rev, uninitialized=uninitialized ) @@ -192,6 +203,9 @@ class Repo: ] = None self._lock_depth = 0 + def __str__(self): + return self.url or self.root_dir + @cached_property def scm(self): from dvc.scm import SCM @@ -442,7 +456,7 @@ class Repo: def repo_tree(self): from dvc.tree.repo import RepoTree - return RepoTree(self, subrepos=self.subrepos, fetch=True) + return RepoTree(self, subrepos=self.subrepos, **self._tree_conf) @contextmanager def open_by_relpath(self, path, remote=None, mode="r", encoding=None): diff --git a/dvc/tree/repo.py b/dvc/tree/repo.py index 829815ff5..ff4c3093f 100644 --- a/dvc/tree/repo.py +++ b/dvc/tree/repo.py @@ -94,7 +94,14 @@ class RepoTree(BaseTree): # pylint:disable=abstract-method repo = starting_repo for d in dirs: if self._is_dvc_repo(d): - repo = self.repo_factory(d) + repo = self.repo_factory( + d, + scm=self.repo.scm, + rev=self.repo.get_rev(), + repo_factory=self.repo_factory, + fetch=self.fetch, + stream=self.stream, + ) self._dvctrees[repo.root_dir] = DvcTree( repo, **self._dvctree_configs ) @@ -126,11 +133,11 @@ class RepoTree(BaseTree): # pylint:disable=abstract-method @property def fetch(self): - return "fetch" in self._dvctree_configs + return self._dvctree_configs.get("fetch") @property def stream(self): - return "stream" in self._dvctree_configs + return self._dvctree_configs.get("stream") def open( self, path, mode="r", encoding="utf-8", **kwargs diff --git a/tests/unit/test_external_repo.py b/tests/unit/test_external_repo.py index d823821d1..c237671f2 100644 --- a/tests/unit/test_external_repo.py +++ b/tests/unit/test_external_repo.py @@ -27,13 +27,26 @@ def test_hook_is_called(tmp_dir, erepo_dir, mocker): repo.dvc_gen("bar", "bar", commit=f"dvc add {repo}/bar") with external_repo(str(erepo_dir)) as repo: - spy = mocker.spy(repo, "make_repo") + spy = mocker.spy(repo.repo_tree, "repo_factory") list(repo.repo_tree.walk(repo.root_dir)) # drain assert spy.call_count == len(subrepos) paths = [os.path.join(repo.root_dir, path) for path in subrepo_paths] - spy.assert_has_calls([call(path) for path in paths], any_order=True) + spy.assert_has_calls( + [ + call( + path, + scm=repo.scm, + rev=repo.get_rev(), + fetch=True, + stream=None, + repo_factory=repo.repo_tree.repo_factory, + ) + for path in paths + ], + any_order=True, + ) @pytest.mark.parametrize("root_is_dvc", [False, True]) @@ -54,18 +67,18 @@ def test_subrepo_is_constructed_properly( with external_repo( str(tmp_dir), cache_dir=str(cache_dir), cache_types=["symlink"] ) as repo: - spy = mocker.spy(repo, "make_repo") + spy = mocker.spy(repo.repo_tree, "repo_factory") list(repo.repo_tree.walk(repo.root_dir)) # drain assert spy.call_count == 1 subrepo = spy.return_value assert repo.url == str(tmp_dir) - assert repo._cache_config["cache"]["dir"] == str(cache_dir) + assert repo.config["cache"]["dir"] == str(cache_dir) assert repo.cache.local.cache_dir == str(cache_dir) assert subrepo.cache.local.cache_dir == str(cache_dir) - assert repo._cache_config["cache"]["type"] == ["symlink"] + assert repo.config["cache"]["type"] == ["symlink"] assert repo.cache.local.cache_types == ["symlink"] assert subrepo.cache.local.cache_types == ["symlink"] diff --git a/tests/unit/tree/test_repo.py b/tests/unit/tree/test_repo.py index 78c547dae..b10ee9651 100644 --- a/tests/unit/tree/test_repo.py +++ b/tests/unit/tree/test_repo.py @@ -422,11 +422,15 @@ def test_subrepo_walk(tmp_dir, scm, dvc, dvcfiles, extra_expected): ] actual = [] - for root, dirs, files in tree.walk("dir", dvcfiles=dvcfiles): + for root, dirs, files in tree.walk( + os.path.join(tree.root_dir, "dir"), dvcfiles=dvcfiles + ): for entry in dirs + files: actual.append(os.path.join(root, entry)) - expected = [str(path) for path in expected + extra_expected] + expected = [ + os.path.join(tree.root_dir, path) for path in expected + extra_expected + ] assert set(actual) == set(expected) assert len(actual) == len(expected) @@ -608,7 +612,7 @@ def test_walk_nested_subrepos(tmp_dir, dvc, scm, traverse_subrepos): expected[str(tmp_dir / "subrepo1")].add("subrepo3") actual = {} - tree = RepoTree(dvc, subrepos=traverse_subrepos) + tree = RepoTree(dvc, subrepos=traverse_subrepos, fetch=True) for root, dirs, files in tree.walk(str(tmp_dir)): actual[root] = set(dirs + files) assert expected == actual