From 4901765cd47d1e742ebe4ea3051d0c93c4239713 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Wed, 2 Sep 2020 05:53:09 +0300 Subject: [PATCH] get/import: fix bug when working with mixed dirs (#4516) Fixes #4510 --- dvc/tree/repo.py | 7 ++---- tests/func/test_get.py | 13 ++++++++++ tests/func/test_import.py | 13 ++++++++++ tests/unit/tree/test_repo.py | 47 ++++++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 5 deletions(-) diff --git a/dvc/tree/repo.py b/dvc/tree/repo.py index aa1f5ad14..e736e5583 100644 --- a/dvc/tree/repo.py +++ b/dvc/tree/repo.py @@ -316,11 +316,8 @@ class RepoTree(BaseTree): # pylint:disable=abstract-method if not self.exists(path_info): raise FileNotFoundError _, dvc_tree = self._get_tree_pair(path_info) - if dvc_tree and dvc_tree.exists(path_info): - try: - return dvc_tree.get_dir_hash(path_info, **kwargs) - except OutputNotFoundError: - pass + if dvc_tree and dvc_tree.isdvc(path_info): + return dvc_tree.get_dir_hash(path_info, **kwargs) return super().get_dir_hash(path_info, **kwargs) def get_file_hash(self, path_info): diff --git a/tests/func/test_get.py b/tests/func/test_get.py index 72736f807..a74ebdbca 100644 --- a/tests/func/test_get.py +++ b/tests/func/test_get.py @@ -252,3 +252,16 @@ def test_get_pipeline_tracked_outs( with git_dir.chdir(): Repo.get("file:///{}".format(os.fspath(tmp_dir)), "bar", out="baz") assert (git_dir / "baz").read_text() == "foo" + + +def test_get_mixed_dir(tmp_dir, erepo_dir): + with erepo_dir.chdir(): + erepo_dir.dvc_gen(os.path.join("dir", "foo"), "foo", commit="foo") + erepo_dir.scm_gen(os.path.join("dir", "bar"), "bar", commit="bar") + + Repo.get(os.fspath(erepo_dir), "dir") + assert (tmp_dir / "dir").read_text() == { + ".gitignore": "/foo\n", + "foo": "foo", + "bar": "bar", + } diff --git a/tests/func/test_import.py b/tests/func/test_import.py index 5af224775..f3cdc9184 100644 --- a/tests/func/test_import.py +++ b/tests/func/test_import.py @@ -368,3 +368,16 @@ def test_local_import(tmp_dir, dvc, scm): tmp_dir.dvc_gen("foo", "foo", commit="init") (tmp_dir / "outdir").mkdir() dvc.imp(".", "foo", out="outdir") + + +def test_import_mixed_dir(tmp_dir, dvc, erepo_dir): + with erepo_dir.chdir(): + erepo_dir.dvc_gen(os.path.join("dir", "foo"), "foo", commit="foo") + erepo_dir.scm_gen(os.path.join("dir", "bar"), "bar", commit="bar") + + dvc.imp(os.fspath(erepo_dir), "dir") + assert (tmp_dir / "dir").read_text() == { + ".gitignore": "/foo\n", + "foo": "foo", + "bar": "bar", + } diff --git a/tests/unit/tree/test_repo.py b/tests/unit/tree/test_repo.py index fac40afba..ef08dee5d 100644 --- a/tests/unit/tree/test_repo.py +++ b/tests/unit/tree/test_repo.py @@ -147,6 +147,34 @@ def test_walk(tmp_dir, dvc, dvcfiles, extra_expected): assert len(actual) == len(expected) +def test_walk_mixed_dir(tmp_dir, scm, dvc): + tmp_dir.gen({"dir": {"foo": "foo", "bar": "bar"}}) + tmp_dir.dvc.add(os.path.join("dir", "foo")) + tmp_dir.scm.add( + [ + os.path.join("dir", "bar"), + os.path.join("dir", ".gitignore"), + os.path.join("dir", "foo.dvc"), + ] + ) + tmp_dir.scm.commit("add dir") + + tree = RepoTree(dvc) + + expected = [ + str(PathInfo("dir") / "foo"), + str(PathInfo("dir") / "bar"), + str(PathInfo("dir") / ".gitignore"), + ] + actual = [] + for root, dirs, files in tree.walk("dir"): + for entry in dirs + files: + actual.append(os.path.join(root, entry)) + + assert set(actual) == set(expected) + assert len(actual) == len(expected) + + def test_walk_onerror(tmp_dir, dvc): def onerror(exc): raise exc @@ -401,6 +429,25 @@ def test_get_hash_cached_granular(tmp_dir, dvc, mocker): assert dvc_tree_spy.called +def test_get_hash_mixed_dir(tmp_dir, scm, dvc): + tmp_dir.gen({"dir": {"foo": "foo", "bar": "bar"}}) + tmp_dir.dvc.add(os.path.join("dir", "foo")) + tmp_dir.scm.add( + [ + os.path.join("dir", "bar"), + os.path.join("dir", ".gitignore"), + os.path.join("dir", "foo.dvc"), + ] + ) + tmp_dir.scm.commit("add dir") + + tree = RepoTree(dvc) + with dvc.state: + actual = tree.get_hash(PathInfo(tmp_dir) / "dir") + expected = HashInfo("md5", "e1d9e8eae5374860ae025ec84cfd85c7.dir") + assert actual == expected + + @pytest.mark.parametrize("traverse_subrepos", [True, False]) def test_walk_nested_subrepos(tmp_dir, dvc, scm, traverse_subrepos): # generate a dvc and fs structure, with suffix based on repo's basename