tree: repo: handle dirty state (#4554)

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)

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)

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")

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):

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

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:

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

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