diff --git a/dvc/ignore.py b/dvc/ignore.py index 1c7f731ba..ff3259ff4 100644 --- a/dvc/ignore.py +++ b/dvc/ignore.py @@ -172,6 +172,9 @@ class DvcIgnoreFilterNoop: def check_ignore(self, path): return _no_match(path) + def is_ignored(self, _): + return False + class DvcIgnoreFilter: @staticmethod @@ -331,6 +334,11 @@ class DvcIgnoreFilter: return CheckIgnoreResult(target, True, matches) return _no_match(target) + def is_ignored(self, path): + # NOTE: can't use self.check_ignore(path).match for now, see + # https://github.com/iterative/dvc/issues/4555 + return self.is_ignored_dir(path) or self.is_ignored_file(path) + def init(path): dvcignore = os.path.join(path, DvcIgnore.DVCIGNORE_FILE) diff --git a/dvc/tree/dvc.py b/dvc/tree/dvc.py index 2fc8f8d6e..f82283f01 100644 --- a/dvc/tree/dvc.py +++ b/dvc/tree/dvc.py @@ -103,14 +103,14 @@ class DvcTree(BaseTree): # pylint:disable=abstract-method try: self.metadata(path) return True - except OutputNotFoundError: + except FileNotFoundError: return False def isdir(self, path): # pylint: disable=arguments-differ try: meta = self.metadata(path) return meta.isdir - except OutputNotFoundError: + except FileNotFoundError: return False def check_isdir(self, path_info, outs): @@ -123,22 +123,17 @@ class DvcTree(BaseTree): # pylint:disable=abstract-method if out.path_info == path_info: return True - # for dir checksum, we need to check if this is a file inside the - # directory try: self._get_granular_checksum(path_info, out) return False except FileNotFoundError: - # path may be an untracked file from a dirty workspace - if self.repo.tree.isfile(path_info): - return False return True def isfile(self, path): # pylint: disable=arguments-differ try: meta = self.metadata(path) return meta.isfile - except OutputNotFoundError: + except FileNotFoundError: return False def _fetch_dir( @@ -201,7 +196,7 @@ class DvcTree(BaseTree): # pylint:disable=abstract-method root = PathInfo(os.path.abspath(top)) try: meta = self.metadata(root) - except OutputNotFoundError: + except FileNotFoundError: if onerror is not None: onerror(FileNotFoundError(top)) return @@ -229,7 +224,7 @@ class DvcTree(BaseTree): # pylint:disable=abstract-method def isdvc(self, path, recursive=False, strict=True): try: meta = self.metadata(path) - except OutputNotFoundError: + except FileNotFoundError: return False recurse = recursive or not strict @@ -257,21 +252,19 @@ class DvcTree(BaseTree): # pylint:disable=abstract-method raise OutputNotFoundError out = outs[0] if out.is_dir_checksum: - try: - return HashInfo( - out.tree.PARAM_CHECKSUM, - self._get_granular_checksum(path_info, out), - ) - except FileNotFoundError: - # path may be an untracked file from a dirty workspace - if self.repo.tree.isfile(path_info): - return self.repo.tree.get_file_hash(path_info) - raise + return HashInfo( + out.tree.PARAM_CHECKSUM, + self._get_granular_checksum(path_info, out), + ) return out.hash_info def metadata(self, path_info): path_info = PathInfo(os.path.abspath(path_info)) - outs = self._find_outs(path_info, strict=False, recursive=True) + + try: + outs = self._find_outs(path_info, strict=False, recursive=True) + except OutputNotFoundError as exc: + raise FileNotFoundError from exc meta = Metadata(path_info=path_info, outs=outs, repo=self.repo) meta.isdir = meta.isdir or self.check_isdir(meta.path_info, meta.outs) diff --git a/dvc/tree/git.py b/dvc/tree/git.py index 25cc9d68f..34faec2d9 100644 --- a/dvc/tree/git.py +++ b/dvc/tree/git.py @@ -215,6 +215,9 @@ class GitTree(BaseTree): # pylint:disable=abstract-method sec, nano_sec = git_time return sec + nano_sec / 1000000000 + if self.dvcignore.is_ignored(path): + raise FileNotFoundError + obj = self._git_object_by_path(path) if obj is None: raise OSError(errno.ENOENT, "No such file") diff --git a/dvc/tree/local.py b/dvc/tree/local.py index 17b38f013..c45765583 100644 --- a/dvc/tree/local.py +++ b/dvc/tree/local.py @@ -160,8 +160,10 @@ class LocalTree(BaseTree): mode = os.stat(path).st_mode return is_exec(mode) - @staticmethod - def stat(path): + def stat(self, path): + if self.dvcignore.is_ignored(path): + raise FileNotFoundError + return os.stat(path) def move(self, from_info, to_info, mode=None): diff --git a/dvc/tree/repo.py b/dvc/tree/repo.py index 0b5620642..b6c0630dd 100644 --- a/dvc/tree/repo.py +++ b/dvc/tree/repo.py @@ -10,7 +10,6 @@ from funcy import lfilter, wrap_with from pygtrie import StringTrie from dvc.dvcfile import is_valid_filename -from dvc.exceptions import OutputNotFoundError from dvc.hash_info import HashInfo from dvc.path_info import PathInfo from dvc.utils import file_md5, is_exec @@ -163,11 +162,47 @@ class RepoTree(BaseTree): # pylint:disable=abstract-method self, path, use_dvcignore=True ): # pylint: disable=arguments-differ tree, dvc_tree = self._get_tree_pair(path) - return tree.exists(path) or (dvc_tree and dvc_tree.exists(path)) + + if not dvc_tree: + return tree.exists(path) + + if tree.exists(path): + return True + + try: + meta = dvc_tree.metadata(path) + except FileNotFoundError: + return False + + (out,) = meta.outs + assert len(meta.outs) == 1 + if tree.exists(out.path_info): + return False + return True def isdir(self, path): # pylint: disable=arguments-differ tree, dvc_tree = self._get_tree_pair(path) - return tree.isdir(path) or (dvc_tree and dvc_tree.isdir(path)) + + try: + st = tree.stat(path) + return stat.S_ISDIR(st.st_mode) + except (OSError, ValueError): + # from CPython's os.path.isdir() + pass + + if not dvc_tree: + return False + + try: + meta = dvc_tree.metadata(path) + except FileNotFoundError: + return False + + (out,) = meta.outs + assert len(meta.outs) == 1 + if tree.exists(out.path_info): + return False + return meta.isdir def isdvc(self, path, **kwargs): _, dvc_tree = self._get_tree_pair(path) @@ -175,7 +210,27 @@ class RepoTree(BaseTree): # pylint:disable=abstract-method def isfile(self, path): # pylint: disable=arguments-differ tree, dvc_tree = self._get_tree_pair(path) - return tree.isfile(path) or (dvc_tree and dvc_tree.isfile(path)) + + try: + st = tree.stat(path) + return stat.S_ISREG(st.st_mode) + except (OSError, ValueError): + # from CPython's os.path.isfile() + pass + + if not dvc_tree: + return False + + try: + meta = dvc_tree.metadata(path) + except FileNotFoundError: + return False + + (out,) = meta.outs + assert len(meta.outs) == 1 + if tree.exists(out.path_info): + return False + return meta.isfile def isexec(self, path): tree, dvc_tree = self._get_tree_pair(path) @@ -289,27 +344,26 @@ class RepoTree(BaseTree): # pylint:disable=abstract-method return tree, dvc_tree = self._get_tree_pair(top) - dvc_exists = dvc_tree and dvc_tree.exists(top) repo_exists = tree.exists(top) - if dvc_exists: - dvc_walk = dvc_tree.walk(top, topdown=topdown, **kwargs) - if repo_exists: - repo_walk = tree.walk( - top, - topdown=topdown, - ignore_subrepos=not self._traverse_subrepos, - ) - yield from self._walk(repo_walk, dvc_walk, dvcfiles=dvcfiles) - else: - yield from dvc_walk - else: - repo_walk = tree.walk( - top, - topdown=topdown, - onerror=onerror, - ignore_subrepos=not self._traverse_subrepos, - ) + repo_walk = tree.walk( + top, + topdown=topdown, + onerror=onerror, + ignore_subrepos=not self._traverse_subrepos, + ) + + if not dvc_tree or (repo_exists and dvc_tree.isdvc(top)): yield from self._walk(repo_walk, None, dvcfiles=dvcfiles) + return + + if not repo_exists: + yield from dvc_tree.walk(top, topdown=topdown, **kwargs) + + dvc_walk = None + if dvc_tree.exists(top): + dvc_walk = dvc_tree.walk(top, topdown=topdown, **kwargs) + + yield from self._walk(repo_walk, dvc_walk, dvcfiles=dvcfiles) def walk_files(self, top, **kwargs): # pylint: disable=arguments-differ for root, _, files in self.walk(top, **kwargs): @@ -317,12 +371,12 @@ class RepoTree(BaseTree): # pylint:disable=abstract-method yield PathInfo(root) / fname def get_dir_hash(self, path_info, **kwargs): - if not self.exists(path_info): + tree, dvc_tree = self._get_tree_pair(path_info) + if tree.exists(path_info): + return super().get_dir_hash(path_info, **kwargs) + if not dvc_tree: raise FileNotFoundError - _, dvc_tree = self._get_tree_pair(path_info) - 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) + return dvc_tree.get_dir_hash(path_info, **kwargs) def get_file_hash(self, path_info): """Return file checksum for specified path. @@ -337,7 +391,7 @@ class RepoTree(BaseTree): # pylint:disable=abstract-method if dvc_tree and dvc_tree.exists(path_info): try: return dvc_tree.get_file_hash(path_info) - except OutputNotFoundError: + except FileNotFoundError: pass return HashInfo(self.PARAM_CHECKSUM, file_md5(path_info, self)[0]) @@ -374,7 +428,7 @@ class RepoTree(BaseTree): # pylint:disable=abstract-method dvc_meta = None if dvc_tree: - with suppress(OutputNotFoundError): + with suppress(FileNotFoundError): dvc_meta = dvc_tree.metadata(path_info) stat_result = None diff --git a/tests/func/test_tree.py b/tests/func/test_tree.py index f89cbf72c..ad39116ee 100644 --- a/tests/func/test_tree.py +++ b/tests/func/test_tree.py @@ -191,6 +191,7 @@ def test_repotree_walk_fetch(tmp_dir, dvc, scm, local_remote): out = tmp_dir.dvc_gen({"dir": {"foo": "foo"}}, commit="init")[0].outs[0] dvc.push() remove(dvc.cache.local.cache_dir) + remove(tmp_dir / "dir") tree = RepoTree(dvc, fetch=True) with dvc.state: diff --git a/tests/unit/tree/test_dvc.py b/tests/unit/tree/test_dvc.py index af2c0bb5a..0480568fb 100644 --- a/tests/unit/tree/test_dvc.py +++ b/tests/unit/tree/test_dvc.py @@ -242,3 +242,26 @@ def test_get_hash_granular(tmp_dir, dvc): assert tree.get_hash(subdir / "data") == HashInfo( "md5", "8d777f385d3dfec8815d20f7496026dc", ) + + +def test_get_hash_dirty_file(tmp_dir, dvc): + tmp_dir.dvc_gen("file", "file") + (tmp_dir / "file").write_text("something") + + tree = DvcTree(dvc) + with dvc.state: + actual = tree.get_hash(PathInfo(tmp_dir) / "file") + expected = HashInfo("md5", "8c7dd922ad47494fc02c388e12c00eac") + assert actual == expected + + +def test_get_hash_dirty_dir(tmp_dir, dvc): + tmp_dir.dvc_gen({"dir": {"foo": "foo", "bar": "bar"}}) + (tmp_dir / "dir" / "baz").write_text("baz") + + tree = DvcTree(dvc) + with dvc.state: + actual = tree.get_hash(PathInfo(tmp_dir) / "dir") + expected = HashInfo("md5", "5ea40360f5b4ec688df672a4db9c17d1.dir") + + assert actual == expected diff --git a/tests/unit/tree/test_repo.py b/tests/unit/tree/test_repo.py index ef08dee5d..7b23293fe 100644 --- a/tests/unit/tree/test_repo.py +++ b/tests/unit/tree/test_repo.py @@ -90,6 +90,42 @@ def test_isdir_isfile(tmp_dir, dvc): assert tree.isdvc("datafile") +def test_exists_isdir_isfile_dirty(tmp_dir, dvc): + tmp_dir.dvc_gen( + {"datafile": "data", "datadir": {"foo": "foo", "bar": "bar"}} + ) + + tree = RepoTree(dvc, stream=True) + shutil.rmtree(tmp_dir / "datadir") + (tmp_dir / "datafile").unlink() + + root = PathInfo(tmp_dir) + assert tree.exists(root / "datafile") + assert tree.exists(root / "datadir") + assert tree.exists(root / "datadir" / "foo") + assert tree.isfile(root / "datafile") + assert not tree.isfile(root / "datadir") + assert tree.isfile(root / "datadir" / "foo") + assert not tree.isdir(root / "datafile") + assert tree.isdir(root / "datadir") + assert not tree.isdir(root / "datadir" / "foo") + + # NOTE: creating file instead of dir and dir instead of file + tmp_dir.gen({"datadir": "data", "datafile": {"foo": "foo", "bar": "bar"}}) + assert tree.exists(root / "datafile") + assert tree.exists(root / "datadir") + assert not tree.exists(root / "datadir" / "foo") + assert tree.exists(root / "datafile" / "foo") + assert not tree.isfile(root / "datafile") + assert tree.isfile(root / "datadir") + assert not tree.isfile(root / "datadir" / "foo") + assert tree.isfile(root / "datafile" / "foo") + assert tree.isdir(root / "datafile") + assert not tree.isdir(root / "datadir") + assert not tree.isdir(root / "datadir" / "foo") + assert not tree.isdir(root / "datafile" / "foo") + + def test_isdir_mixed(tmp_dir, dvc): tmp_dir.gen({"dir": {"foo": "foo", "bar": "bar"}}) @@ -147,6 +183,59 @@ def test_walk(tmp_dir, dvc, dvcfiles, extra_expected): assert len(actual) == len(expected) +def test_walk_dirty(tmp_dir, dvc): + tmp_dir.dvc_gen( + { + "dir": { + "foo": "foo", + "subdir1": {"foo1": "foo1", "bar1": "bar1"}, + "subdir2": {"foo2": "foo2"}, + } + } + ) + tmp_dir.gen({"dir": {"bar": "bar", "subdir3": {"foo3": "foo3"}}}) + (tmp_dir / "dir" / "foo").unlink() + + tree = RepoTree(dvc) + expected = [ + PathInfo("dir") / "subdir1", + PathInfo("dir") / "subdir2", + PathInfo("dir") / "subdir3", + PathInfo("dir") / "subdir1" / "foo1", + PathInfo("dir") / "subdir1" / "bar1", + PathInfo("dir") / "subdir2" / "foo2", + PathInfo("dir") / "subdir3" / "foo3", + PathInfo("dir") / "bar", + ] + + actual = [] + for root, dirs, files in tree.walk("dir"): + for entry in dirs + files: + actual.append(os.path.join(root, entry)) + + expected = [str(path) for path in expected] + assert set(actual) == set(expected) + assert len(actual) == len(expected) + + +def test_walk_dirty_cached_dir(tmp_dir, scm, dvc): + tmp_dir.dvc_gen( + {"data": {"foo": "foo", "bar": "bar"}}, commit="add data", + ) + (tmp_dir / "data" / "foo").unlink() + + tree = RepoTree(dvc) + + data = PathInfo(tmp_dir) / "data" + + actual = [] + for root, dirs, files in tree.walk(data): + for entry in dirs + files: + actual.append(os.path.join(root, entry)) + + assert actual == [(data / "bar").fspath] + + 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")) @@ -405,9 +494,19 @@ def test_get_hash_cached_dir(tmp_dir, dvc, mocker): tree = RepoTree(dvc) get_file_hash_spy = mocker.spy(tree, "get_file_hash") dvc_tree_spy = mocker.spy(tree._dvctrees[dvc.root_dir], "get_dir_hash") - assert tree.get_hash(PathInfo(tmp_dir) / "dir") == HashInfo( - "md5", "8761c4e9acad696bee718615e23e22db.dir", - ) + with dvc.state: + assert tree.get_hash(PathInfo(tmp_dir) / "dir") == HashInfo( + "md5", "8761c4e9acad696bee718615e23e22db.dir", + ) + assert get_file_hash_spy.called + assert not dvc_tree_spy.called + get_file_hash_spy.reset_mock() + + shutil.rmtree(tmp_dir / "dir") + with dvc.state: + assert tree.get_hash(PathInfo(tmp_dir) / "dir") == HashInfo( + "md5", "8761c4e9acad696bee718615e23e22db.dir", + ) assert not get_file_hash_spy.called assert dvc_tree_spy.called @@ -448,6 +547,29 @@ def test_get_hash_mixed_dir(tmp_dir, scm, dvc): assert actual == expected +def test_get_hash_dirty_file(tmp_dir, dvc): + tmp_dir.dvc_gen("file", "file") + (tmp_dir / "file").write_text("something") + + tree = RepoTree(dvc) + with dvc.state: + actual = tree.get_hash(PathInfo(tmp_dir) / "file") + expected = HashInfo("md5", "8c7dd922ad47494fc02c388e12c00eac") + assert actual == expected + + +def test_get_hash_dirty_dir(tmp_dir, dvc): + tmp_dir.dvc_gen({"dir": {"foo": "foo", "bar": "bar"}}) + (tmp_dir / "dir" / "baz").write_text("baz") + + tree = RepoTree(dvc) + with dvc.state: + actual = tree.get_hash(PathInfo(tmp_dir) / "dir") + expected = HashInfo("md5", "ba75a2162ca9c29acecb7957105a0bc2.dir") + assert actual == expected + assert len(actual.dir_info) == 3 + + @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