From 956fec02ce445fa809ccd6f43556adf30ca42c88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Wed, 18 Dec 2019 16:08:56 +0100 Subject: [PATCH] repo: move dvcignore from repo to tree --- dvc/remote/local.py | 4 ++-- dvc/repo/__init__.py | 10 +++------- dvc/repo/add.py | 2 +- dvc/repo/brancher.py | 4 ++-- dvc/scm/git/__init__.py | 5 ++++- dvc/scm/git/tree.py | 20 ++++++++++++++------ dvc/scm/tree.py | 30 ++++++++++++++++++++++++++++++ dvc/state.py | 10 ++++++---- tests/func/test_checkout.py | 2 +- tests/func/test_ignore.py | 33 ++++++++++++++++++--------------- tests/func/test_repo.py | 3 ++- tests/func/test_tree.py | 8 ++++---- 12 files changed, 87 insertions(+), 44 deletions(-) diff --git a/dvc/remote/local.py b/dvc/remote/local.py index 9c68e3a80..0da62ff25 100644 --- a/dvc/remote/local.py +++ b/dvc/remote/local.py @@ -138,7 +138,7 @@ class RemoteLOCAL(RemoteBASE): return os.path.getsize(fspath_py35(path_info)) def walk_files(self, path_info): - for fname in walk_files(path_info, self.repo.dvcignore): + for fname in walk_files(path_info, self.repo.tree.dvcignore): yield PathInfo(fname) def get_file_checksum(self, path_info): @@ -427,7 +427,7 @@ class RemoteLOCAL(RemoteBASE): os.chmod(path, os.stat(path).st_mode | stat.S_IWRITE) def _unprotect_dir(self, path): - for fname in walk_files(path, self.repo.dvcignore): + for fname in walk_files(path, self.repo.tree.dvcignore): RemoteLOCAL._unprotect_file(fname) def unprotect(self, path_info): diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 3df374160..d81234782 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -9,13 +9,13 @@ from dvc.utils.compat import FileNotFoundError, fspath_py35, open as _open from funcy import cached_property +from dvc.scm.tree import IgnoreTree from dvc.config import Config from dvc.exceptions import ( FileMissingError, NotDvcRepoError, OutputNotFoundError, ) -from dvc.ignore import DvcIgnoreFilter from dvc.path_info import PathInfo from dvc.remote.base import RemoteActionNotImplemented from dvc.utils import relpath @@ -84,7 +84,7 @@ class Repo(object): self.scm = SCM(self.root_dir) - self.tree = WorkingTree(self.root_dir) + self.tree = IgnoreTree(WorkingTree(self.root_dir)) self.tmp_dir = os.path.join(self.dvc_dir, "tmp") makedirs(self.tmp_dir, exist_ok=True) @@ -392,7 +392,7 @@ class Repo(object): outs = [] for root, dirs, files in self.tree.walk( - self.root_dir, dvcignore=self.dvcignore + self.root_dir, dvcignore=self.tree.dvcignore ): for fname in files: path = os.path.join(root, fname) @@ -487,10 +487,6 @@ class Repo(object): return _open(cache_file, mode=mode, encoding=encoding) - @cached_property - def dvcignore(self): - return DvcIgnoreFilter(self.root_dir, self.tree) - def close(self): self.scm.close() diff --git a/dvc/repo/add.py b/dvc/repo/add.py index 8fe180d41..f0353482b 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -68,7 +68,7 @@ def _find_all_targets(repo, target, recursive): if os.path.isdir(target) and recursive: return [ fname - for fname in walk_files(target, repo.dvcignore) + for fname in walk_files(target, repo.tree.dvcignore) if not repo.is_dvc_internal(fname) if not Stage.is_stage_file(fname) if not repo.scm.belongs_to_scm(fname) diff --git a/dvc/repo/brancher.py b/dvc/repo/brancher.py index acca98c2c..ff3e25d39 100644 --- a/dvc/repo/brancher.py +++ b/dvc/repo/brancher.py @@ -1,6 +1,6 @@ from funcy import group_by -from dvc.scm.tree import WorkingTree +from dvc.scm.tree import WorkingTree, IgnoreTree def brancher( # noqa: E302 @@ -34,7 +34,7 @@ def brancher( # noqa: E302 scm = self.scm - self.tree = WorkingTree(self.root_dir) + self.tree = IgnoreTree(WorkingTree(self.root_dir)) yield "working tree" if all_commits: diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 668dc6fd5..9a71a3afb 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -14,6 +14,7 @@ from dvc.scm.base import FileNotInRepoError from dvc.scm.base import RevError from dvc.scm.base import SCMError from dvc.scm.git.tree import GitTree +from dvc.scm.tree import IgnoreTree from dvc.utils import fix_env from dvc.utils import is_binary from dvc.utils import relpath @@ -307,7 +308,9 @@ class Git(Base): return basename == self.ignore_file or Git.GIT_DIR in path_parts def get_tree(self, rev): - return GitTree(self.repo, rev) + tree = GitTree(self.repo, rev) + + return IgnoreTree(tree) def _get_diff_trees(self, a_ref, b_ref): """Private method for getting the trees and commit hashes of 2 git diff --git a/dvc/scm/git/tree.py b/dvc/scm/git/tree.py index 0601c322d..a7991d53e 100644 --- a/dvc/scm/git/tree.py +++ b/dvc/scm/git/tree.py @@ -118,7 +118,7 @@ class GitTree(BaseTree): tree = tree[i] return tree - def _walk(self, tree, topdown=True): + def _walk(self, tree, topdown=True, dvcignore=None): dirs, nondirs = [], [] for i in _iter_tree(tree): if i.mode == GIT_MODE_DIR: @@ -127,14 +127,22 @@ class GitTree(BaseTree): nondirs.append(i.name) if topdown: - yield os.path.normpath(tree.abspath), dirs, nondirs + root = os.path.normpath(tree.abspath) + if dvcignore: + dirs[:], nondirs[:] = dvcignore(root, dirs, nondirs) + yield root, dirs, nondirs for i in dirs: - for x in self._walk(tree[i], topdown=True): - yield x + for root, dirs, nondirs in self._walk(tree[i], topdown=True): + if dvcignore: + dirs[:], nondirs[:] = dvcignore(root, dirs, nondirs) + yield root, dirs, nondirs if not topdown: - yield os.path.normpath(tree.abspath), dirs, nondirs + root = os.path.normpath(tree.abspath) + if dvcignore: + dirs[:], nondirs[:] = dvcignore(root, dirs, nondirs) + yield root, dirs, nondirs def walk(self, top, topdown=True, dvcignore=None): """Directory tree generator. @@ -148,5 +156,5 @@ class GitTree(BaseTree): if tree is None: raise IOError(errno.ENOENT, "No such file") - for x in self._walk(tree, topdown): + for x in self._walk(tree, topdown, dvcignore): yield x diff --git a/dvc/scm/tree.py b/dvc/scm/tree.py index 0931a59d0..06062f996 100644 --- a/dvc/scm/tree.py +++ b/dvc/scm/tree.py @@ -1,5 +1,6 @@ import os +from dvc.ignore import DvcIgnoreFilter from dvc.utils import dvc_walk from dvc.utils.compat import open @@ -75,3 +76,32 @@ class WorkingTree(BaseTree): os.path.abspath(top), dvcignore, topdown=topdown, onerror=onerror ): yield os.path.normpath(root), dirs, files + + +class IgnoreTree(BaseTree): + def __init__(self, tree): + self.tree = tree + + @property + def dvcignore(self): + return DvcIgnoreFilter(self.tree.tree_root, self.tree) + + @property + def tree_root(self): + return self.tree.tree_root + + def open(self, path, mode="r", encoding="utf-8"): + return self.tree.open(path, mode, encoding) + + def exists(self, path): + return self.tree.exists(path) + + def isdir(self, path): + return self.tree.isdir(path) + + def isfile(self, path): + return self.tree.isfile(path) + + def walk(self, top, topdown=True, dvcignore=None): + for r, d, fs in self.tree.walk(top, topdown, self.dvcignore): + yield r, d, fs diff --git a/dvc/state.py b/dvc/state.py index 9803e40c2..a3e27a8e9 100644 --- a/dvc/state.py +++ b/dvc/state.py @@ -378,7 +378,7 @@ class State(object): # pylint: disable=too-many-instance-attributes assert os.path.exists(fspath_py35(path_info)) actual_mtime, actual_size = get_mtime_and_size( - path_info, self.repo.dvcignore + path_info, self.repo.tree.dvcignore ) actual_inode = get_inode(path_info) @@ -411,7 +411,7 @@ class State(object): # pylint: disable=too-many-instance-attributes return None actual_mtime, actual_size = get_mtime_and_size( - path, self.repo.dvcignore + path, self.repo.tree.dvcignore ) actual_inode = get_inode(path) @@ -439,7 +439,7 @@ class State(object): # pylint: disable=too-many-instance-attributes if not os.path.exists(path): return - mtime, _ = get_mtime_and_size(path, self.repo.dvcignore) + mtime, _ = get_mtime_and_size(path, self.repo.tree.dvcignore) inode = get_inode(path) relative_path = relpath(path, self.root_dir) @@ -469,7 +469,9 @@ class State(object): # pylint: disable=too-many-instance-attributes continue actual_inode = get_inode(path) - actual_mtime, _ = get_mtime_and_size(path, self.repo.dvcignore) + actual_mtime, _ = get_mtime_and_size( + path, self.repo.tree.dvcignore + ) if inode == actual_inode and mtime == actual_mtime: logger.debug("Removing '{}' as unused link.".format(path)) diff --git a/tests/func/test_checkout.py b/tests/func/test_checkout.py index 797e42470..0760bd2d5 100644 --- a/tests/func/test_checkout.py +++ b/tests/func/test_checkout.py @@ -137,7 +137,7 @@ class CheckoutBase(TestDvcGit): paths = [ path for output in stage["outs"] - for path in walk_files(output["path"], self.dvc.dvcignore) + for path in walk_files(output["path"], self.dvc.tree.dvcignore) ] return [ diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index 3bf878a4a..a2a4aae2d 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -20,10 +20,10 @@ 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.dvcignore) == {"dir/other"} + assert _files_set("dir", dvc.tree.dvcignore) == {"dir/other"} monkeypatch.chdir("dir") - assert _files_set(".", dvc.dvcignore) == {"./other"} + assert _files_set(".", dvc.tree.dvcignore) == {"./other"} def test_ignore_unicode(tmp_dir, dvc): @@ -35,27 +35,27 @@ def test_ignore_unicode(tmp_dir, dvc): tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "dir/ั‚ะตัั‚") - assert _files_set("dir", dvc.dvcignore) == {"dir/other"} + assert _files_set("dir", dvc.tree.dvcignore) == {"dir/other"} def test_rename_ignored_file(tmp_dir, dvc): tmp_dir.gen({"dir": {"ignored": "...", "other": "text"}}) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored*") - mtime, size = get_mtime_and_size("dir", dvc.dvcignore) + mtime, size = get_mtime_and_size("dir", dvc.tree.dvcignore) shutil.move("dir/ignored", "dir/ignored_new") - new_mtime, new_size = get_mtime_and_size("dir", dvc.dvcignore) + new_mtime, new_size = get_mtime_and_size("dir", dvc.tree.dvcignore) assert new_mtime == mtime and new_size == size def test_rename_file(tmp_dir, dvc): tmp_dir.gen({"dir": {"foo": "foo", "bar": "bar"}}) - mtime, size = get_mtime_and_size("dir", dvc.dvcignore) + mtime, size = get_mtime_and_size("dir", dvc.tree.dvcignore) shutil.move("dir/foo", "dir/foo_new") - new_mtime, new_size = get_mtime_and_size("dir", dvc.dvcignore) + new_mtime, new_size = get_mtime_and_size("dir", dvc.tree.dvcignore) assert new_mtime != mtime and new_size == size @@ -64,20 +64,20 @@ def test_remove_ignored_file(tmp_dir, dvc): tmp_dir.gen({"dir": {"ignored": "...", "other": "text"}}) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "dir/ignored") - mtime, size = get_mtime_and_size("dir", dvc.dvcignore) + mtime, size = get_mtime_and_size("dir", dvc.tree.dvcignore) os.remove("dir/ignored") - new_mtime, new_size = get_mtime_and_size("dir", dvc.dvcignore) + new_mtime, new_size = get_mtime_and_size("dir", dvc.tree.dvcignore) assert new_mtime == mtime and new_size == size def test_remove_file(tmp_dir, dvc): tmp_dir.gen({"dir": {"foo": "foo", "bar": "bar"}}) - mtime, size = get_mtime_and_size("dir", dvc.dvcignore) + mtime, size = get_mtime_and_size("dir", dvc.tree.dvcignore) os.remove("dir/foo") - new_mtime, new_size = get_mtime_and_size("dir", dvc.dvcignore) + new_mtime, new_size = get_mtime_and_size("dir", dvc.tree.dvcignore) assert new_mtime != mtime and new_size != size @@ -99,7 +99,7 @@ def test_ignore_collecting_dvcignores(tmp_dir, dvc, dname): ignore_file = tmp_dir / dname / DvcIgnore.DVCIGNORE_FILE ignore_file.write_text("foo") - assert dvc.dvcignore.ignores == { + assert dvc.tree.dvcignore.ignores == { DvcIgnoreDirs([".git", ".hg", ".dvc"]), DvcIgnorePatterns(fspath(top_ignore_file), WorkingTree(dvc.root_dir)), } @@ -112,10 +112,13 @@ def test_ignore_on_branch(tmp_dir, scm, dvc): tmp_dir.scm_gen(DvcIgnore.DVCIGNORE_FILE, "foo", commit="add ignore") scm.checkout("master") - assert _files_set(".", dvc.dvcignore) == {"./foo", "./bar"} + assert _files_set(".", dvc.tree.dvcignore) == {"./foo", "./bar"} - dvc.tree = scm.get_tree("branch") - assert _files_set(".", dvc.dvcignore) == {"./bar"} + tree = scm.get_tree("branch") + files_on_branch = { + f for r, ds, fs in tree.walk(tree.tree_root) for f in fs + } + assert "foo" not in files_on_branch def _files_set(root, dvcignore): diff --git a/tests/func/test_repo.py b/tests/func/test_repo.py index 51f3a8754..28f881693 100644 --- a/tests/func/test_repo.py +++ b/tests/func/test_repo.py @@ -3,6 +3,7 @@ import os from dvc.scm.git.tree import GitTree from dvc.cache import Cache from dvc.repo import Repo +from dvc.scm.tree import IgnoreTree from dvc.system import System from dvc.utils.compat import fspath @@ -50,7 +51,7 @@ def test_collect(tmp_dir, scm, dvc, run_copy): assert collect_outs("bar.dvc", with_deps=True) == {"foo", "bar"} - dvc.tree = GitTree(scm.repo, "new-branch") + dvc.tree = IgnoreTree(GitTree(scm.repo, "new-branch")) assert collect_outs("buzz.dvc", with_deps=True) == {"foo", "bar", "buzz"} assert collect_outs("buzz.dvc", with_deps=False) == {"buzz"} diff --git a/tests/func/test_tree.py b/tests/func/test_tree.py index c2ae316c2..2b2ad38b2 100644 --- a/tests/func/test_tree.py +++ b/tests/func/test_tree.py @@ -6,7 +6,7 @@ from os.path import join from dvc.ignore import DvcIgnoreFilter from dvc.scm import SCM from dvc.scm.git import GitTree -from dvc.scm.tree import WorkingTree +from dvc.scm.tree import WorkingTree, IgnoreTree from tests.basic_env import TestDir from tests.basic_env import TestGit from tests.basic_env import TestGitSubmodule @@ -82,14 +82,14 @@ class TestGitTree(TestGit, GitTreeTests): def setUp(self): super(TestGitTree, self).setUp() self.scm = SCM(self._root_dir) - self.tree = GitTree(self.git, "master") + self.tree = IgnoreTree(GitTree(self.git, "master")) class TestGitSubmoduleTree(TestGitSubmodule, GitTreeTests): def setUp(self): super(TestGitSubmoduleTree, self).setUp() self.scm = SCM(self._root_dir) - self.tree = GitTree(self.git, "master") + self.tree = IgnoreTree(GitTree(self.git, "master")) self._pushd(self._root_dir) @@ -177,7 +177,7 @@ class TestWalkInGit(AssertWalkEqualMixin, TestGit): scm = SCM(self._root_dir) scm.add([self.DATA_SUB_DIR]) scm.commit("add data_dir/data_sub_dir/data_sub") - tree = GitTree(self.git, "master") + tree = IgnoreTree(GitTree(self.git, "master")) self.assertWalkEqual( tree.walk("."), [