diff --git a/dvc/external_repo.py b/dvc/external_repo.py index 470646c6e..3c35e205e 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -1,9 +1,11 @@ +import logging import os import tempfile from contextlib import contextmanager from distutils.dir_util import copy_tree +import threading -from funcy import retry, suppress, memoize, cached_property +from funcy import retry, suppress, wrap_with, cached_property from dvc.compat import fspath from dvc.repo import Repo @@ -16,6 +18,9 @@ from dvc.utils.fs import remove, fs_copy from dvc.scm import SCM +logger = logging.getLogger(__name__) + + @contextmanager def external_repo(url, rev=None): path = _cached_clone(url, rev) @@ -40,8 +45,8 @@ def external_repo(url, rev=None): def clean_repos(): # Outside code should not see cache while we are removing - repo_paths = list(_cached_clone.memory.values()) - _cached_clone.memory.clear() + repo_paths = list(REPOS_CACHE.values()) + REPOS_CACHE.clear() for path in repo_paths: _remove(path) @@ -137,19 +142,28 @@ class ExternalGitRepo: raise PathMissingError(path, self.url) -@memoize +REPOS_CACHE = {} + + +@wrap_with(threading.Lock()) def _cached_clone(url, rev): """Clone an external git repo to a temporary directory. Returns the path to a local temporary directory with the specified revision checked out. """ + if (url, rev) in REPOS_CACHE: + path = REPOS_CACHE[url, rev] + _git_pull(path, rev) + return path + new_path = tempfile.mkdtemp("dvc-erepo") - if url in _cached_clone.memory: + if url in REPOS_CACHE: # Copy and an existing clean clone # This one unlike shutil.copytree() works with an existing dir - copy_tree(_cached_clone.memory[url], new_path) + copy_tree(REPOS_CACHE[url], new_path) + _git_pull(new_path, None) else: # Create a new clone _clone_repo(url, new_path) @@ -157,21 +171,37 @@ def _cached_clone(url, rev): # Save clean clone dir so that we will have access to a default branch clean_clone_path = tempfile.mkdtemp("dvc-erepo") copy_tree(new_path, clean_clone_path) - _cached_clone.memory[url] = clean_clone_path + REPOS_CACHE[url] = clean_clone_path # Check out the specified revision if rev is not None: _git_checkout(new_path, rev) + REPOS_CACHE[url, rev] = new_path return new_path -def _git_checkout(repo_path, revision): +def _git_checkout(repo_path, rev): from dvc.scm import Git git = Git(repo_path) try: - git.checkout(revision) + git.checkout(rev) + finally: + git.close() + + +def _git_pull(repo_path, rev): + import git + + # Do not try to pull in a detached mode + if rev and git.Repo.re_hexsha_only.search(rev): + return + + git = git.Repo(repo_path) + try: + msg = git.git.pull() + logger.debug("external repo: git pull: {}", msg) finally: git.close() diff --git a/tests/func/test_external_repo.py b/tests/func/test_external_repo.py index 4426ef1e1..8aa6837ca 100644 --- a/tests/func/test_external_repo.py +++ b/tests/func/test_external_repo.py @@ -23,3 +23,16 @@ def test_external_repo(erepo_dir): assert fd.read() == "branch" assert mock.call_count == 1 + + +def test_source_change(erepo_dir): + url = fspath(erepo_dir) + with external_repo(url) as repo: + old_rev = repo.scm.get_rev() + + erepo_dir.scm_gen("file", "text", commit="a change") + + with external_repo(url) as repo: + new_rev = repo.scm.get_rev() + + assert old_rev != new_rev diff --git a/tests/func/test_status.py b/tests/func/test_status.py index f8e5964bc..35b66aa51 100644 --- a/tests/func/test_status.py +++ b/tests/func/test_status.py @@ -4,7 +4,6 @@ from mock import patch from dvc.main import main from dvc.compat import fspath -from dvc.external_repo import clean_repos from tests.basic_env import TestDvc @@ -33,19 +32,12 @@ def test_status_non_dvc_repo_import(tmp_dir, dvc, git_dir): dvc.imp(fspath(git_dir), "file", "file", rev="branch") - status = dvc.status(["file.dvc"]) - - assert status == {} - - # Caching in external repos doesn't see upstream updates within single - # cli call, so we need to clean the caches to see the changes. - clean_repos() + assert dvc.status(["file.dvc"]) == {} with git_dir.branch("branch", new=False): git_dir.scm_gen("file", "second version", commit="update file") status, = dvc.status(["file.dvc"])["file.dvc"] - assert status == { "changed deps": {"file ({})".format(git_dir): "update available"} } @@ -68,12 +60,7 @@ def test_status_before_and_after_dvc_init(tmp_dir, dvc, git_dir): assert old_rev != new_rev - # Caching in external repos doesn't see upstream updates within single - # cli call, so we need to clean the caches to see the changes. - clean_repos() - status, = dvc.status(["file.dvc"])["file.dvc"] - assert status == { "changed deps": { "file ({})".format(fspath(git_dir)): "update available" diff --git a/tests/func/test_update.py b/tests/func/test_update.py index 37639e9aa..e2c68de40 100644 --- a/tests/func/test_update.py +++ b/tests/func/test_update.py @@ -3,7 +3,6 @@ import os from dvc.stage import Stage from dvc.compat import fspath -from dvc.external_repo import clean_repos @pytest.mark.parametrize("cached", [True, False]) @@ -16,43 +15,21 @@ def test_update_import(tmp_dir, dvc, erepo_dir, cached): stage = dvc.imp(fspath(erepo_dir), "version", "version", rev="branch") - imported = tmp_dir / "version" - assert imported.is_file() - assert imported.read_text() == "branch" - assert stage.deps[0].def_repo == { - "url": fspath(erepo_dir), - "rev": "branch", - "rev_lock": old_rev, - } + assert (tmp_dir / "version").read_text() == "branch" + assert stage.deps[0].def_repo["rev_lock"] == old_rev + # Update version file with erepo_dir.branch("branch", new=False), erepo_dir.chdir(): gen("version", "updated", "update version content") new_rev = erepo_dir.scm.get_rev() assert old_rev != new_rev - # Caching in external repos doesn't see upstream updates within single - # cli call, so we need to clean the caches to see the changes. - clean_repos() - - status, = dvc.status([stage.path])["version.dvc"] - changed_dep, = list(status["changed deps"].items()) - assert changed_dep[0].startswith("version ") - assert changed_dep[1] == "update available" - dvc.update(stage.path) - - assert dvc.status([stage.path]) == {} - - assert imported.is_file() - assert imported.read_text() == "updated" + assert (tmp_dir / "version").read_text() == "updated" stage = Stage.load(dvc, stage.path) - assert stage.deps[0].def_repo == { - "url": fspath(erepo_dir), - "rev": "branch", - "rev_lock": new_rev, - } + assert stage.deps[0].def_repo["rev_lock"] == new_rev def test_update_import_after_remote_updates_to_dvc(tmp_dir, dvc, erepo_dir): @@ -82,10 +59,6 @@ def test_update_import_after_remote_updates_to_dvc(tmp_dir, dvc, erepo_dir): assert old_rev != new_rev - # Caching in external repos doesn't see upstream updates within single - # cli call, so we need to clean the caches to see the changes. - clean_repos() - status, = dvc.status([stage.path])["version.dvc"] changed_dep, = list(status["changed deps"].items()) assert changed_dep[0].startswith("version ") @@ -122,10 +95,6 @@ def test_update_before_and_after_dvc_init(tmp_dir, dvc, git_dir): assert old_rev != new_rev - # Caching in external repos doesn't see upstream updates within single - # cli call, so we need to clean the caches to see the changes. - clean_repos() - assert dvc.status([stage.path]) == { "file.dvc": [ {