dvc: get rid of CleanTree (#4221)

* dvcignore: move file/dir/parent checks into DvcIgnoreFilter

* dvc: get rid of CleanTree

CleanTree is a very awkward wrapper that spreads the context across
the codebase and results in unexpected behaviour when using it.

This PR starts moving dvcignore-related logic into the trees themselves
(it makes a lot of sense, kinda like `state`) so they could deal with
it however they like.

There are at least two temporary ugly parts about this PR:

1) dvcignore is used by individual trees and not packed into
tree/base.py yet;

2) `dvcignore_root` argument. This one is caused by the dvcignore trying
to collect everything topdown starting from the certain root dir. What
it should do instead is for a certain path that is being checked look up
the tree through the parents until it finds repo root (.dvc dir) and
then stop. That would handle subrepos as well. At the same time we need
to leverage existing dvcignore trie structure to cache those results.

dvc/config.py

@@ -249,7 +249,7 @@ class Config(dict):
             self.dvc_dir = os.path.abspath(os.path.realpath(dvc_dir))
 
         self.wtree = LocalRemoteTree(None, {"url": self.dvc_dir})
-        self.tree = tree.tree if tree else self.wtree
+        self.tree = tree or self.wtree
 
         self.load(validate=validate)
 

dvc/ignore.py

@@ -3,14 +3,12 @@ import os
 import re
 from itertools import groupby
 
-from funcy import cached_property
 from pathspec.patterns import GitWildMatchPattern
 from pathspec.util import normalize_file
 from pygtrie import StringTrie
 
 from dvc.path_info import PathInfo
 from dvc.pathspec_math import merge_patterns
-from dvc.scm.tree import BaseTree
 from dvc.system import System
 from dvc.utils import relpath
 
@@ -162,6 +160,20 @@ class DvcIgnoreRepo(DvcIgnore):
         return dirs, files
 
 
+class DvcIgnoreFilterNoop:
+    def __init__(self, tree, root_dir):
+        pass
+
+    def __call__(self, root, dirs, files):
+        return dirs, files
+
+    def is_ignored_dir(self, _):
+        return False
+
+    def is_ignored_file(self, _):
+        return False
+
+
 class DvcIgnoreFilter:
     def __init__(self, tree, root_dir):
         self.tree = tree
@@ -190,118 +202,52 @@ class DvcIgnoreFilter:
 
         return dirs, files
 
+    def is_ignored_dir(self, path):
+        if not self._parents_exist(path):
+            return True
 
-class CleanTree(BaseTree):
-    def __init__(self, tree, tree_root=None):
-        self.tree = tree
-        if tree_root:
-            self._tree_root = tree_root
-        else:
-            self._tree_root = self.tree.tree_root
-
-    @cached_property
-    def dvcignore(self):
-        return DvcIgnoreFilter(self.tree, self.tree_root)
-
-    @property
-    def tree_root(self):
-        return self._tree_root
-
-    def open(self, path, mode="r", encoding="utf-8"):
-        if self.isfile(path):
-            return self.tree.open(path, mode, encoding)
-        raise FileNotFoundError
-
-    def exists(self, path):
-        if self.tree.exists(path) and self._parents_exist(path):
-            if self.tree.isdir(path):
-                return self._valid_dirname(path)
-            return self._valid_filename(path)
-        return False
-
-    def isdir(self, path):
-        return (
-            self.tree.isdir(path)
-            and self._parents_exist(path)
-            and self._valid_dirname(path)
-        )
-
-    def _valid_dirname(self, path):
         path = os.path.abspath(path)
-        if path == self.tree_root:
-            return True
+        if path == self.root_dir:
+            return False
         dirname, basename = os.path.split(path)
-        dirs, _ = self.dvcignore(dirname, [basename], [])
-        if dirs:
+        dirs, _ = self(dirname, [basename], [])
+        return not dirs
+
+    def is_ignored_file(self, path):
+        if not self._parents_exist(path):
             return True
-        return False
 
-    def isfile(self, path):
-        return (
-            self.tree.isfile(path)
-            and self._parents_exist(path)
-            and self._valid_filename(path)
-        )
-
-    def _valid_filename(self, path):
         dirname, basename = os.path.split(os.path.normpath(path))
-        _, files = self.dvcignore(os.path.abspath(dirname), [], [basename])
-        if files:
-            return True
-        return False
-
-    def isexec(self, path):
-        return self.exists(path) and self.tree.isexec(path)
+        _, files = self(os.path.abspath(dirname), [], [basename])
+        return not files
 
     def _parents_exist(self, path):
         from dvc.repo import Repo
 
         path = PathInfo(path)
 
-        # if parent is tree_root or inside a .dvc dir we can skip this check
-        if path.parent == self.tree_root or Repo.DVC_DIR in path.parts:
+        # if parent is root_dir or inside a .dvc dir we can skip this check
+        if path.parent == self.root_dir or Repo.DVC_DIR in path.parts:
             return True
 
-        # paths outside of the CleanTree root should be ignored
-        path = relpath(path, self.tree_root)
+        # paths outside of the repo should be ignored
+        path = relpath(path, self.root_dir)
         if path.startswith("..") or (
             os.name == "nt"
             and not os.path.commonprefix(
-                [os.path.abspath(path), self.tree_root]
+                [os.path.abspath(path), self.root_dir]
             )
         ):
             return False
 
         # check if parent directories are in our ignores, starting from
-        # tree_root
+        # root_dir
         for parent_dir in reversed(PathInfo(path).parents):
             dirname, basename = os.path.split(parent_dir)
             if basename == ".":
-                # parent_dir == tree_root
+                # parent_dir == root_dir
                 continue
-            dirs, _ = self.dvcignore(os.path.abspath(dirname), [basename], [])
+            dirs, _ = self(os.path.abspath(dirname), [basename], [])
             if not dirs:
                 return False
         return True
-
-    def walk(self, top, topdown=True, onerror=None):
-        for root, dirs, files in self.tree.walk(
-            top, topdown=topdown, onerror=onerror
-        ):
-            dirs[:], files[:] = self.dvcignore(
-                os.path.abspath(root), dirs, files
-            )
-
-            yield root, dirs, files
-
-    def stat(self, path):
-        if self.exists(path):
-            return self.tree.stat(path)
-        raise FileNotFoundError
-
-    @property
-    def hash_jobs(self):
-        return self.tree.hash_jobs
-
-    def makedirs(self, path, mode=0o777, exist_ok=True):
-        self.tree.makedirs(path, mode=mode, exist_ok=exist_ok)

dvc/repo/__init__.py

@@ -14,10 +14,8 @@ from dvc.exceptions import (
     NotDvcRepoError,
     OutputNotFoundError,
 )
-from dvc.ignore import CleanTree
 from dvc.path_info import PathInfo
 from dvc.repo.tree import RepoTree
-from dvc.scm.tree import is_working_tree
 from dvc.utils.fs import path_isin
 
 from ..stage.exceptions import StageFileDoesNotExistError, StageNotFound
@@ -85,12 +83,19 @@ class Repo:
             tree = scm.get_tree(rev)
             self.root_dir = self.find_root(root_dir, tree)
             self.scm = scm
-            self.tree = tree
+            self.tree = scm.get_tree(
+                rev, use_dvcignore=True, dvcignore_root=self.root_dir
+            )
             self.state = StateNoop()
         else:
             root_dir = self.find_root(root_dir)
             self.root_dir = os.path.abspath(os.path.realpath(root_dir))
-            self.tree = LocalRemoteTree(None, {"url": self.root_dir})
+            self.tree = LocalRemoteTree(
+                self,
+                {"url": self.root_dir},
+                use_dvcignore=True,
+                dvcignore_root=self.root_dir,
+            )
 
         self.dvc_dir = os.path.join(self.root_dir, self.DVC_DIR)
         self.config = Config(self.dvc_dir, tree=self.tree)
@@ -134,13 +139,7 @@ class Repo:
 
     @tree.setter
     def tree(self, tree):
-        if is_working_tree(tree) or tree.tree_root == self.root_dir:
-            root = None
-        else:
-            root = self.root_dir
-        self._tree = (
-            tree if isinstance(tree, CleanTree) else CleanTree(tree, root)
-        )
+        self._tree = tree
         # Our graph cache is no longer valid, as it was based on the previous
         # tree.
         self._reset()
@@ -605,4 +604,4 @@ class Repo:
         self.__dict__.pop("graph", None)
         self.__dict__.pop("stages", None)
         self.__dict__.pop("pipelines", None)
-        self.__dict__.pop("dvcignore", None)
+        self.tree.__dict__.pop("dvcignore", None)

dvc/repo/add.py

@@ -109,17 +109,17 @@ def add(
 def _find_all_targets(repo, target, recursive):
     if os.path.isdir(target) and recursive:
         return [
-            fname
-            for fname in Tqdm(
+            os.fspath(path)
+            for path in Tqdm(
                 repo.tree.walk_files(target),
                 desc="Searching " + target,
                 bar_format=Tqdm.BAR_FMT_NOTOTAL,
                 unit="file",
             )
-            if not repo.is_dvc_internal(fname)
-            if not is_dvc_file(fname)
-            if not repo.scm.belongs_to_scm(fname)
-            if not repo.scm.is_tracked(fname)
+            if not repo.is_dvc_internal(os.fspath(path))
+            if not is_dvc_file(os.fspath(path))
+            if not repo.scm.belongs_to_scm(os.fspath(path))
+            if not repo.scm.is_tracked(os.fspath(path))
         ]
     return [target]
 

dvc/repo/brancher.py

@@ -29,7 +29,9 @@ def brancher(  # noqa: E302
 
     scm = self.scm
 
-    self.tree = LocalRemoteTree(self, {"url": self.root_dir})
+    self.tree = LocalRemoteTree(
+        self, {"url": self.root_dir}, use_dvcignore=True
+    )
     yield "workspace"
 
     if revs and "workspace" in revs:
@@ -47,7 +49,9 @@ def brancher(  # noqa: E302
     try:
         if revs:
             for sha, names in group_by(scm.resolve_rev, revs).items():
-                self.tree = scm.get_tree(sha)
+                self.tree = scm.get_tree(
+                    sha, use_dvcignore=True, dvcignore_root=self.root_dir
+                )
                 # ignore revs that don't contain repo root
                 # (i.e. revs from before a subdir=True repo was init'ed)
                 if self.tree.exists(self.root_dir):

dvc/scm/git/__init__.py

@@ -375,8 +375,8 @@ class Git(Base):
         path_parts = os.path.normpath(path).split(os.path.sep)
         return basename == self.ignore_file or Git.GIT_DIR in path_parts
 
-    def get_tree(self, rev):
-        return GitTree(self.repo, self.resolve_rev(rev))
+    def get_tree(self, rev, **kwargs):
+        return GitTree(self.repo, self.resolve_rev(rev), **kwargs)
 
     def get_rev(self):
         return self.repo.rev_parse("HEAD").hexsha

dvc/scm/git/tree.py

@@ -3,6 +3,8 @@ import io
 import os
 import stat
 
+from funcy import cached_property
+
 from dvc.exceptions import DvcException
 from dvc.scm.tree import BaseTree
 from dvc.utils import relpath
@@ -22,7 +24,7 @@ def _item_basename(item):
 class GitTree(BaseTree):  # pylint:disable=abstract-method
     """Proxies the repo file access methods to Git objects"""
 
-    def __init__(self, git, rev):
+    def __init__(self, git, rev, use_dvcignore=False, dvcignore_root=None):
         """Create GitTree instance
 
         Args:
@@ -31,11 +33,25 @@ class GitTree(BaseTree):  # pylint:disable=abstract-method
         """
         self.git = git
         self.rev = rev
+        self.use_dvcignore = use_dvcignore
+        self.dvcignore_root = dvcignore_root
 
     @property
     def tree_root(self):
         return self.git.working_dir
 
+    @cached_property
+    def dvcignore(self):
+        from dvc.ignore import DvcIgnoreFilter, DvcIgnoreFilterNoop
+
+        root = self.dvcignore_root or self.tree_root
+        if not self.use_dvcignore:
+            return DvcIgnoreFilterNoop(self, root)
+        self.use_dvcignore = False
+        ret = DvcIgnoreFilter(self, root)
+        self.use_dvcignore = True
+        return ret
+
     def open(self, path, mode="r", encoding="utf-8"):
         assert mode in {"r", "rb"}
 
@@ -58,20 +74,29 @@ class GitTree(BaseTree):  # pylint:disable=abstract-method
         return io.StringIO(data.decode(encoding))
 
     def exists(self, path):
-        return self._git_object_by_path(path) is not None
+        if self._git_object_by_path(path) is None:
+            return False
+
+        return not self.dvcignore.is_ignored_file(
+            path
+        ) and not self.dvcignore.is_ignored_dir(path)
 
     def isdir(self, path):
         obj = self._git_object_by_path(path)
         if obj is None:
             return False
-        return obj.mode == GIT_MODE_DIR
+        if obj.mode != GIT_MODE_DIR:
+            return False
+        return not self.dvcignore.is_ignored_dir(path)
 
     def isfile(self, path):
         obj = self._git_object_by_path(path)
         if obj is None:
             return False
         # according to git-fast-import(1) file mode could be 644 or 755
-        return obj.mode & GIT_MODE_FILE == GIT_MODE_FILE
+        if obj.mode & GIT_MODE_FILE != GIT_MODE_FILE:
+            return False
+        return not self.dvcignore.is_ignored_file(path)
 
     @staticmethod
     def _is_tree_and_contains(obj, path):
@@ -145,7 +170,11 @@ class GitTree(BaseTree):  # pylint:disable=abstract-method
                 onerror(NotADirectoryError(top))
             return
 
-        yield from self._walk(tree, topdown=topdown)
+        for root, dirs, files in self._walk(tree, topdown=topdown):
+            dirs[:], files[:] = self.dvcignore(
+                os.path.abspath(root), dirs, files
+            )
+            yield root, dirs, files
 
     def isexec(self, path):
         if not self.exists(path):

dvc/state.py

@@ -94,7 +94,7 @@ class State:  # pylint: disable=too-many-instance-attributes
         repo = local_cache.repo
         self.repo = repo
         self.root_dir = repo.root_dir
-        self.tree = LocalRemoteTree(None, {})
+        self.tree = LocalRemoteTree(None, {"url": self.root_dir})
 
         state_config = repo.config.get("state", {})
         self.row_limit = state_config.get("row_limit", self.STATE_ROW_LIMIT)

dvc/tree/base.py

@@ -8,6 +8,8 @@ from multiprocessing import cpu_count
 from operator import itemgetter
 from urllib.parse import urlparse
 
+from funcy import cached_property
+
 from dvc.exceptions import (
     DvcException,
     DvcIgnoreInCollectedDirError,
@@ -86,13 +88,16 @@ class BaseRemoteTree:
         shared = config.get("shared")
         self._file_mode, self._dir_mode = self.SHARED_MODE_MAP[shared]
 
-        self.hash_jobs = (
-            config.get("hash_jobs")
+        self.verify = config.get("verify", self.DEFAULT_VERIFY)
+        self.path_info = None
+
+    @cached_property
+    def hash_jobs(self):
+        return (
+            self.config.get("hash_jobs")
             or (self.repo and self.repo.config["core"].get("hash_jobs"))
             or self.HASH_JOBS
         )
-        self.verify = config.get("verify", self.DEFAULT_VERIFY)
-        self.path_info = None
 
     @classmethod
     def get_missing_deps(cls):

dvc/tree/local.py

@@ -3,6 +3,7 @@ import logging
 import os
 import stat
 
+from funcy import cached_property
 from shortuuid import uuid
 
 from dvc.exceptions import DvcException
@@ -35,10 +36,12 @@ class LocalRemoteTree(BaseRemoteTree):
     CACHE_MODE = 0o444
     SHARED_MODE_MAP = {None: (0o644, 0o755), "group": (0o664, 0o775)}
 
-    def __init__(self, repo, config):
+    def __init__(self, repo, config, use_dvcignore=False, dvcignore_root=None):
         super().__init__(repo, config)
         url = config.get("url")
         self.path_info = self.PATH_CLS(url) if url else None
+        self.use_dvcignore = use_dvcignore
+        self.dvcignore_root = dvcignore_root
 
     @property
     def tree_root(self):
@@ -50,25 +53,46 @@ class LocalRemoteTree(BaseRemoteTree):
 
         return self.repo.state if self.repo else StateNoop()
 
+    @cached_property
+    def dvcignore(self):
+        from dvc.ignore import DvcIgnoreFilter, DvcIgnoreFilterNoop
+
+        root = self.dvcignore_root or self.tree_root
+        if not self.use_dvcignore:
+            return DvcIgnoreFilterNoop(self, root)
+        self.use_dvcignore = False
+        ret = DvcIgnoreFilter(self, root)
+        self.use_dvcignore = True
+        return ret
+
     @staticmethod
     def open(path_info, mode="r", encoding=None):
         return open(path_info, mode=mode, encoding=encoding)
 
     def exists(self, path_info):
         assert isinstance(path_info, str) or path_info.scheme == "local"
-        if not self.repo:
-            return os.path.exists(path_info)
-        return os.path.lexists(path_info)
+        if self.repo:
+            ret = os.path.lexists(path_info)
+        else:
+            ret = os.path.exists(path_info)
+        if not ret:
+            return False
+
+        return not self.dvcignore.is_ignored_file(
+            path_info
+        ) and not self.dvcignore.is_ignored_dir(path_info)
 
     def isfile(self, path_info):
-        if not self.repo:
-            return os.path.isfile(path_info)
-        return os.path.isfile(path_info)
+        if not os.path.isfile(path_info):
+            return False
+
+        return not self.dvcignore.is_ignored_file(path_info)
 
     def isdir(self, path_info):
-        if not self.repo:
-            return os.path.isdir(path_info)
-        return os.path.isdir(path_info)
+        if not os.path.isdir(path_info):
+            return False
+
+        return not self.dvcignore.is_ignored_dir(path_info)
 
     def iscopy(self, path_info):
         return not (
@@ -84,6 +108,10 @@ class LocalRemoteTree(BaseRemoteTree):
         for root, dirs, files in os.walk(
             top, topdown=topdown, onerror=onerror
         ):
+            dirs[:], files[:] = self.dvcignore(
+                os.path.abspath(root), dirs, files
+            )
+
             yield os.path.normpath(root), dirs, files
 
     def walk_files(self, path_info, **kwargs):

tests/func/test_ignore.py

@@ -17,30 +17,33 @@ from dvc.tree.local import LocalRemoteTree
 from dvc.utils import relpath
 from dvc.utils.fs import get_mtime_and_size
 from tests.dir_helpers import TmpDir
-from tests.utils import to_posixpath
 
 
 def test_ignore(tmp_dir, dvc, monkeypatch):
     tmp_dir.gen({"dir": {"ignored": "text", "other": "text2"}})
     tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "dir/ignored")
 
-    assert _files_set("dir", dvc.tree) == {"dir/other"}
+    dvc.tree.__dict__.pop("dvcignore", None)
 
-    monkeypatch.chdir("dir")
-    assert _files_set(".", dvc.tree) == {"./other"}
+    path = PathInfo(tmp_dir)
+
+    assert set(dvc.tree.walk_files(path / "dir")) == {path / "dir" / "other"}
 
 
 def test_ignore_unicode(tmp_dir, dvc):
     tmp_dir.gen({"dir": {"other": "text", "тест": "проверка"}})
     tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "dir/тест")
-
-    assert _files_set("dir", dvc.tree) == {"dir/other"}
+    dvc.tree.__dict__.pop("dvcignore", None)
+    path = PathInfo(tmp_dir)
+    assert set(dvc.tree.walk_files(path / "dir")) == {path / "dir" / "other"}
 
 
 def test_rename_ignored_file(tmp_dir, dvc):
     tmp_dir.gen({"dir": {"ignored": "...", "other": "text"}})
 
     tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored*")
+    dvc.tree.__dict__.pop("dvcignore", None)
+
     mtime, size = get_mtime_and_size("dir", dvc.tree)
 
     shutil.move("dir/ignored", "dir/ignored_new")
@@ -62,6 +65,7 @@ def test_rename_file(tmp_dir, dvc):
 def test_remove_ignored_file(tmp_dir, dvc):
     tmp_dir.gen({"dir": {"ignored": "...", "other": "text"}})
     tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "dir/ignored")
+    dvc.tree.__dict__.pop("dvcignore", None)
 
     mtime, size = get_mtime_and_size("dir", dvc.tree)
 
@@ -94,6 +98,7 @@ def test_ignore_collecting_dvcignores(tmp_dir, dvc, dname):
 
     top_ignore_file = (tmp_dir / dname).with_name(DvcIgnore.DVCIGNORE_FILE)
     top_ignore_file.write_text(os.path.basename(dname))
+    dvc.tree.__dict__.pop("dvcignore", None)
 
     ignore_file = tmp_dir / dname / DvcIgnore.DVCIGNORE_FILE
     ignore_file.write_text("foo")
@@ -125,17 +130,18 @@ def test_ignore_on_branch(tmp_dir, scm, dvc):
     with tmp_dir.branch("branch", new=True):
         tmp_dir.scm_gen(DvcIgnore.DVCIGNORE_FILE, "foo", commit="add ignore")
 
-    assert _files_set(".", dvc.tree) == {"./foo", "./bar"}
-
-    dvc.tree = scm.get_tree("branch")
-    assert _files_set(".", dvc.tree) == {
-        to_posixpath(os.path.join(dvc.root_dir, DvcIgnore.DVCIGNORE_FILE)),
-        to_posixpath(os.path.join(dvc.root_dir, "bar")),
+    dvc.tree.__dict__.pop("dvcignore", None)
+    path = PathInfo(tmp_dir)
+    assert set(dvc.tree.walk_files(path)) == {
+        path / "foo",
+        path / "bar",
     }
 
-
-def _files_set(root, tree):
-    return {to_posixpath(f) for f in tree.walk_files(root)}
+    dvc.tree = scm.get_tree("branch", use_dvcignore=True)
+    assert set(dvc.tree.walk_files(path)) == {
+        os.fspath(path / DvcIgnore.DVCIGNORE_FILE),
+        os.fspath(path / "bar"),
+    }
 
 
 def test_match_nested(tmp_dir, dvc):
@@ -147,7 +153,7 @@ def test_match_nested(tmp_dir, dvc):
             "dir": {"x.backup": "x backup", "tmp": "content"},
         }
     )
-
+    dvc.tree.__dict__.pop("dvcignore", None)
     result = {os.fspath(os.path.normpath(f)) for f in dvc.tree.walk_files(".")}
     assert result == {".dvcignore", "foo"}
 
@@ -165,6 +171,7 @@ def test_ignore_subrepo(tmp_dir, scm, dvc):
     tmp_dir.gen({".dvcignore": "foo", "subdir": {"foo": "foo"}})
     scm.add([".dvcignore"])
     scm.commit("init parent dvcignore")
+    dvc.tree.__dict__.pop("dvcignore", None)
 
     subrepo_dir = tmp_dir / "subdir"
     assert not dvc.tree.exists(PathInfo(subrepo_dir / "foo"))
@@ -181,8 +188,9 @@ def test_ignore_subrepo(tmp_dir, scm, dvc):
 def test_ignore_blank_line(tmp_dir, dvc):
     tmp_dir.gen({"dir": {"ignored": "text", "other": "text2"}})
     tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "foo\n\ndir/ignored")
-
-    assert _files_set("dir", dvc.tree) == {"dir/other"}
+    dvc.tree.__dict__.pop("dvcignore", None)
+    path = PathInfo(tmp_dir)
+    assert set(dvc.tree.walk_files(path / "dir")) == {path / "dir" / "other"}
 
 
 # It is not possible to re-include a file if a parent directory of
@@ -196,7 +204,7 @@ def test_ignore_blank_line(tmp_dir, dvc):
         (
             {"dir": {"subdir": {"not_ignore": "121"}}},
             ["subdir/*", "!not_ignore"],
-            {"dir/subdir/not_ignore"},
+            {os.path.join("dir", "subdir", "not_ignore")},
         ),
         (
             {"dir": {"subdir": {"should_ignore": "121"}}},
@@ -215,7 +223,11 @@ def test_ignore_file_in_parent_path(
 ):
     tmp_dir.gen(data_struct)
     tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "\n".join(pattern_list))
-    assert _files_set("dir", dvc.tree) == result_set
+    dvc.tree.__dict__.pop("dvcignore", None)
+    path = PathInfo(tmp_dir)
+    assert set(dvc.tree.walk_files(path / "dir")) == {
+        path / relpath for relpath in result_set
+    }
 
 
 # If there is a separator at the end of the pattern then the pattern
@@ -233,9 +245,12 @@ def test_ignore_sub_directory(tmp_dir, dvc):
         }
     )
     tmp_dir.gen({"dir": {DvcIgnore.DVCIGNORE_FILE: "doc/fortz"}})
-    assert _files_set("dir", dvc.tree) == {
-        "dir/a/doc/fortz/a",
-        "dir/{}".format(DvcIgnore.DVCIGNORE_FILE),
+
+    dvc.tree.__dict__.pop("dvcignore", None)
+    path = PathInfo(tmp_dir)
+    assert set(dvc.tree.walk_files(path / "dir")) == {
+        path / "dir" / "a" / "doc" / "fortz" / "a",
+        path / "dir" / DvcIgnore.DVCIGNORE_FILE,
     }
 
 
@@ -243,8 +258,10 @@ def test_ignore_sub_directory(tmp_dir, dvc):
 def test_ignore_directory(tmp_dir, dvc):
     tmp_dir.gen({"dir": {"fortz": {}, "a": {"fortz": {}}}})
     tmp_dir.gen({"dir": {DvcIgnore.DVCIGNORE_FILE: "fortz"}})
-    assert _files_set("dir", dvc.tree) == {
-        "dir/{}".format(DvcIgnore.DVCIGNORE_FILE),
+    dvc.tree.__dict__.pop("dvcignore", None)
+    path = PathInfo(tmp_dir)
+    assert set(dvc.tree.walk_files(path / "dir")) == {
+        path / "dir" / DvcIgnore.DVCIGNORE_FILE,
     }
 
 
@@ -252,10 +269,11 @@ def test_multi_ignore_file(tmp_dir, dvc, monkeypatch):
     tmp_dir.gen({"dir": {"subdir": {"should_ignore": "1", "not_ignore": "1"}}})
     tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "dir/subdir/*_ignore")
     tmp_dir.gen({"dir": {DvcIgnore.DVCIGNORE_FILE: "!subdir/not_ignore"}})
-
-    assert _files_set("dir", dvc.tree) == {
-        "dir/subdir/not_ignore",
-        "dir/{}".format(DvcIgnore.DVCIGNORE_FILE),
+    dvc.tree.__dict__.pop("dvcignore", None)
+    path = PathInfo(tmp_dir)
+    assert set(dvc.tree.walk_files(path / "dir")) == {
+        path / "dir" / "subdir" / "not_ignore",
+        path / "dir" / DvcIgnore.DVCIGNORE_FILE,
     }
 
 
@@ -276,6 +294,7 @@ def test_pattern_trie_tree(tmp_dir, dvc):
             "other": {DvcIgnore.DVCIGNORE_FILE: "1\n2\n3"},
         }
     )
+    dvc.tree.__dict__.pop("dvcignore", None)
     ignore_pattern_trie = None
     for ignore in dvc.tree.dvcignore.ignores:
         if isinstance(ignore, DvcIgnorePatternsTrie):

tests/func/test_tree.py

@@ -1,7 +1,6 @@
 import os
 from os.path import join
 
-from dvc.ignore import CleanTree
 from dvc.path_info import PathInfo
 from dvc.repo import Repo
 from dvc.repo.tree import RepoTree
@@ -137,7 +136,9 @@ class TestWalkInNoSCM(AssertWalkEqualMixin, TestDir):
 
 class TestWalkInGit(AssertWalkEqualMixin, TestGit):
     def test_nobranch(self):
-        tree = CleanTree(LocalRemoteTree(None, {"url": self._root_dir}))
+        tree = LocalRemoteTree(
+            None, {"url": self._root_dir}, use_dvcignore=True
+        )
         self.assertWalkEqual(
             tree.walk("."),
             [
@@ -232,13 +233,13 @@ def test_cleantree_subrepo(tmp_dir, dvc, scm, monkeypatch):
 
     path = PathInfo(subrepo_dir)
 
-    assert isinstance(dvc.tree, CleanTree)
+    assert dvc.tree.use_dvcignore
     assert not dvc.tree.exists(path / "foo")
     assert not dvc.tree.isfile(path / "foo")
     assert not dvc.tree.exists(path / "dir")
     assert not dvc.tree.isdir(path / "dir")
 
-    assert isinstance(subrepo.tree, CleanTree)
+    assert subrepo.tree.use_dvcignore
     assert subrepo.tree.exists(path / "foo")
     assert subrepo.tree.isfile(path / "foo")
     assert subrepo.tree.exists(path / "dir")

tests/unit/utils/test_fs.py

@@ -6,7 +6,6 @@ import pytest
 from mock import patch
 
 import dvc
-from dvc.ignore import CleanTree
 from dvc.path_info import PathInfo
 from dvc.system import System
 from dvc.tree.local import LocalRemoteTree
@@ -29,7 +28,9 @@ from tests.basic_env import TestDir
 
 class TestMtimeAndSize(TestDir):
     def test(self):
-        tree = CleanTree(LocalRemoteTree(None, {"url": self.root_dir}))
+        tree = LocalRemoteTree(
+            None, {"url": self.root_dir}, use_dvcignore=True
+        )
         file_time, file_size = get_mtime_and_size(self.DATA, tree)
         dir_time, dir_size = get_mtime_and_size(self.DATA_DIR, tree)
 
@@ -130,7 +131,9 @@ def test_path_object_and_str_are_valid_types_get_mtime_and_size(tmp_dir):
     tmp_dir.gen(
         {"dir": {"dir_file": "dir file content"}, "file": "file_content"}
     )
-    tree = CleanTree(LocalRemoteTree(None, {"url": os.fspath(tmp_dir)}))
+    tree = LocalRemoteTree(
+        None, {"url": os.fspath(tmp_dir)}, use_dvcignore=True
+    )
 
     time, size = get_mtime_and_size("dir", tree)
     object_time, object_size = get_mtime_and_size(PathInfo("dir"), tree)