From 7d6ecd1ccb3580149682d76698d84763810282bd Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Wed, 15 Jan 2020 00:37:47 +0200 Subject: [PATCH] import: don't forget to set rev_lock for git files Related to #3087 --- dvc/dependency/repo.py | 2 ++ tests/func/test_import.py | 39 +++++++++++++++++++++++++++++++++------ tests/func/test_update.py | 30 +++++++++++++++++++++++++++--- 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/dvc/dependency/repo.py b/dvc/dependency/repo.py index 57244bc4f..3e1878f9c 100644 --- a/dvc/dependency/repo.py +++ b/dvc/dependency/repo.py @@ -11,6 +11,7 @@ from dvc.exceptions import NotDvcRepoError from dvc.exceptions import OutputNotFoundError from dvc.exceptions import PathMissingError from dvc.utils.fs import fs_copy +from dvc.scm import SCM class DependencyREPO(DependencyLOCAL): @@ -105,6 +106,7 @@ class DependencyREPO(DependencyLOCAL): src_full_path = os.path.join(repo_dir, src_path) dst_full_path = os.path.abspath(to_path) fs_copy(src_full_path, dst_full_path) + self.def_repo[self.PARAM_REV_LOCK] = SCM(repo_dir).get_rev() return True def download(self, to): diff --git a/tests/func/test_import.py b/tests/func/test_import.py index c3a2c7b00..380eaeb88 100644 --- a/tests/func/test_import.py +++ b/tests/func/test_import.py @@ -21,11 +21,15 @@ def test_import(tmp_dir, scm, dvc, erepo_dir): with erepo_dir.chdir(): erepo_dir.dvc_gen("foo", "foo content", commit="create foo") - dvc.imp(fspath(erepo_dir), "foo", "foo_imported") + stage = dvc.imp(fspath(erepo_dir), "foo", "foo_imported") assert os.path.isfile("foo_imported") assert (tmp_dir / "foo_imported").read_text() == "foo content" assert scm.repo.git.check_ignore("foo_imported") + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev_lock": erepo_dir.scm.get_rev(), + } @pytest.mark.parametrize("src_is_dvc", [True, False]) @@ -39,13 +43,17 @@ def test_import_git_file(erepo_dir, tmp_dir, dvc, scm, src_is_dvc): erepo_dir.scm_gen({src: "hello"}, commit="add a regular file") - tmp_dir.dvc.imp(fspath(erepo_dir), src, dst) + stage = tmp_dir.dvc.imp(fspath(erepo_dir), src, dst) assert (tmp_dir / dst).is_file() assert filecmp.cmp( fspath(erepo_dir / src), fspath(tmp_dir / dst), shallow=False ) assert tmp_dir.scm.repo.git.check_ignore(fspath(tmp_dir / dst)) + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev_lock": erepo_dir.scm.get_rev(), + } @pytest.mark.parametrize("src_is_dvc", [True, False]) @@ -59,22 +67,30 @@ def test_import_git_dir(erepo_dir, tmp_dir, dvc, scm, src_is_dvc): erepo_dir.scm_gen({src: {"file.txt": "hello"}}, commit="add a dir") - tmp_dir.dvc.imp(fspath(erepo_dir), src, dst) + stage = dvc.imp(fspath(erepo_dir), src, dst) assert (tmp_dir / dst).is_dir() trees_equal(fspath(erepo_dir / src), fspath(tmp_dir / dst)) assert tmp_dir.scm.repo.git.check_ignore(fspath(tmp_dir / dst)) + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev_lock": erepo_dir.scm.get_rev(), + } def test_import_dir(tmp_dir, scm, dvc, erepo_dir): with erepo_dir.chdir(): erepo_dir.dvc_gen({"dir": {"foo": "foo content"}}, commit="create dir") - dvc.imp(fspath(erepo_dir), "dir", "dir_imported") + stage = dvc.imp(fspath(erepo_dir), "dir", "dir_imported") assert os.path.isdir("dir_imported") trees_equal(fspath(erepo_dir / "dir"), "dir_imported") assert scm.repo.git.check_ignore("dir_imported") + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev_lock": erepo_dir.scm.get_rev(), + } def test_import_non_cached(erepo_dir, tmp_dir, dvc, scm): @@ -90,23 +106,34 @@ def test_import_non_cached(erepo_dir, tmp_dir, dvc, scm): erepo_dir.scm.add([fspath(erepo_dir / src)]) erepo_dir.scm.commit("add a non-cached output") - tmp_dir.dvc.imp(fspath(erepo_dir), src, dst) + stage = tmp_dir.dvc.imp(fspath(erepo_dir), src, dst) assert (tmp_dir / dst).is_file() assert filecmp.cmp( fspath(erepo_dir / src), fspath(tmp_dir / dst), shallow=False ) assert tmp_dir.scm.repo.git.check_ignore(dst) + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev_lock": erepo_dir.scm.get_rev(), + } def test_import_rev(tmp_dir, scm, dvc, erepo_dir): + rev = None with erepo_dir.chdir(), erepo_dir.branch("branch", new=True): erepo_dir.dvc_gen("foo", "foo content", commit="create foo on branch") + rev = erepo_dir.scm.get_rev() - dvc.imp(fspath(erepo_dir), "foo", "foo_imported", rev="branch") + stage = dvc.imp(fspath(erepo_dir), "foo", "foo_imported", rev="branch") assert (tmp_dir / "foo_imported").read_text() == "foo content" assert scm.repo.git.check_ignore("foo_imported") + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev": "branch", + "rev_lock": rev, + } def test_pull_imported_stage(tmp_dir, dvc, erepo_dir): diff --git a/tests/func/test_update.py b/tests/func/test_update.py index 108e32b82..5afbac048 100644 --- a/tests/func/test_update.py +++ b/tests/func/test_update.py @@ -1,19 +1,36 @@ +import pytest + +from dvc.stage import Stage from dvc.compat import fspath from dvc.external_repo import clean_repos -def test_update_import(tmp_dir, dvc, erepo_dir): +@pytest.mark.parametrize("cached", [True, False]) +def test_update_import(tmp_dir, dvc, erepo_dir, cached): + old_rev = None with erepo_dir.branch("branch", new=True), erepo_dir.chdir(): - erepo_dir.dvc_gen("version", "branch", "add version file") + gen = erepo_dir.dvc_gen if cached else erepo_dir.scm_gen + gen("version", "branch", "add version file") + old_rev = erepo_dir.scm.get_rev() 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, + } + new_rev = None with erepo_dir.branch("branch", new=False), erepo_dir.chdir(): - erepo_dir.dvc_gen("version", "updated", "update version content") + gen = erepo_dir.dvc_gen if cached else erepo_dir.scm_gen + 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. @@ -26,6 +43,13 @@ def test_update_import(tmp_dir, dvc, erepo_dir): assert imported.is_file() assert imported.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, + } + def test_update_import_url(tmp_dir, dvc, tmp_path_factory): import_src = tmp_path_factory.mktemp("import_url_source")