From a2c91d20581df2e402a020a5887a72fdee7bc73a Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Mon, 11 Jan 2021 16:16:26 +0545 Subject: [PATCH] scm: implement gitignore (#5243) * scm: implement gitignore; use `pygit2` backend * Add is_ignored implementation on `pygit2` and `gitpython` backends. * Also implement `._reset()` on `repo.scm` which is most of the changes. * fix tests * use _backend_func * minor fixups --- dvc/repo/__init__.py | 22 ++++++++++++++++++---- dvc/scm/base.py | 3 +++ dvc/scm/git/__init__.py | 7 +++++++ dvc/scm/git/backend/base.py | 5 ++++- dvc/scm/git/backend/dulwich.py | 20 ++++++++++++++++---- dvc/scm/git/backend/gitpython.py | 9 ++++++--- dvc/scm/git/backend/pygit2.py | 5 +++-- tests/func/test_scm.py | 2 ++ 8 files changed, 59 insertions(+), 14 deletions(-) diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 37c653334..aee323509 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -148,11 +148,12 @@ class Repo: tree_kwargs = {"use_dvcignore": True, "dvcignore_root": self.root_dir} if scm: - self.tree = scm.get_tree(rev, **tree_kwargs) + self._tree = scm.get_tree(rev, **tree_kwargs) else: - self.tree = LocalTree(self, {"url": self.root_dir}, **tree_kwargs) + self._tree = LocalTree(self, {"url": self.root_dir}, **tree_kwargs) self.config = Config(self.dvc_dir, tree=self.tree, config=config) + self._uninitialized = uninitialized self._scm = scm # used by RepoTree to determine if it should traverse subrepos @@ -184,16 +185,28 @@ class Repo: self.metrics = Metrics(self) self.plots = Plots(self) self.params = Params(self) - self.stage_collection_error_handler = None self.live = Live(self) + + self.stage_collection_error_handler = None self._lock_depth = 0 @cached_property def scm(self): from dvc.scm import SCM + from dvc.scm.base import SCMError + + if self._scm: + return self._scm no_scm = self.config["core"].get("no_scm", False) - return self._scm if self._scm else SCM(self.root_dir, no_scm=no_scm) + try: + return SCM(self.root_dir, no_scm=no_scm) + except SCMError: + if self._uninitialized: + # might not be a git/dvc repo at all + # used in `params/metrics/plots/live` targets + return SCM(self.root_dir, no_scm=True) + raise @cached_property def experiments(self): @@ -442,6 +455,7 @@ class Repo: self.scm.close() def _reset(self): + self.scm._reset() # pylint: disable=protected-access self.__dict__.pop("outs_trie", None) self.__dict__.pop("outs_graph", None) self.__dict__.pop("graph", None) diff --git a/dvc/scm/base.py b/dvc/scm/base.py index 3a7c3f067..65547b7f7 100644 --- a/dvc/scm/base.py +++ b/dvc/scm/base.py @@ -174,3 +174,6 @@ class Base: def close(self): """ Method to close the files """ + + def _reset(self) -> None: + pass diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index aefd3f31a..309d4db99 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -64,6 +64,10 @@ class GitBackends(Mapping): for backend in self.initialized.values(): backend.close() + def reset_all(self) -> None: + for backend in self.initialized.values(): + backend._reset() # pylint: disable=protected-access + class Git(Base): """Class for managing Git.""" @@ -450,3 +454,6 @@ class Git(Base): if rev: logger.debug("Restoring stashed workspace") self.stash.pop() + + def _reset(self) -> None: + self.backends.reset_all() diff --git a/dvc/scm/git/backend/base.py b/dvc/scm/git/backend/base.py index d6411e4d8..bc7171567 100644 --- a/dvc/scm/git/backend/base.py +++ b/dvc/scm/git/backend/base.py @@ -23,7 +23,7 @@ class BaseGitBackend(ABC): pass @abstractmethod - def is_ignored(self, path): + def is_ignored(self, path: str) -> bool: """Return True if the specified path is gitignored.""" @property @@ -305,3 +305,6 @@ class BaseGitBackend(ABC): If ignored is True, gitignored files will be included in untracked_paths. """ + + def _reset(self) -> None: + pass diff --git a/dvc/scm/git/backend/dulwich.py b/dvc/scm/git/backend/dulwich.py index 4ba5c74fe..6f9bc2fa2 100644 --- a/dvc/scm/git/backend/dulwich.py +++ b/dvc/scm/git/backend/dulwich.py @@ -6,6 +6,8 @@ import stat from io import BytesIO, StringIO from typing import Callable, Dict, Iterable, Mapping, Optional, Tuple +from funcy import cached_property + from dvc.path_info import PathInfo from dvc.progress import Tqdm from dvc.scm.base import SCMError @@ -223,11 +225,18 @@ class DulwichBackend(BaseGitBackend): # pylint:disable=abstract-method self._stashes[ref] = DulwichStash(self.repo, ref=os.fsencode(ref)) return self._stashes[ref] - def is_ignored(self, path): - from dulwich import ignore + @cached_property + def ignore_manager(self): + from dulwich.ignore import IgnoreFilterManager - manager = ignore.IgnoreFilterManager.from_repo(self.repo) - return manager.is_ignored(relpath(path, self.root_dir)) + return IgnoreFilterManager.from_repo(self.repo) + + def is_ignored(self, path: str) -> bool: + # `is_ignored` returns `false` if excluded in `.gitignore` and + # `None` if it's not mentioned at all. `True` if it is ignored. + return bool( + self.ignore_manager.is_ignored(relpath(path, self.root_dir)) + ) def set_ref( self, @@ -550,3 +559,6 @@ class DulwichBackend(BaseGitBackend): # pylint:disable=abstract-method [os.fsdecode(name) for name in unstaged], [os.fsdecode(name) for name in untracked], ) + + def _reset(self) -> None: + self.__dict__.pop("ignore_manager", None) diff --git a/dvc/scm/git/backend/gitpython.py b/dvc/scm/git/backend/gitpython.py index 1084be818..4578d6e78 100644 --- a/dvc/scm/git/backend/gitpython.py +++ b/dvc/scm/git/backend/gitpython.py @@ -5,7 +5,7 @@ import os from functools import partial from typing import Callable, Iterable, Mapping, Optional, Tuple -from funcy import first +from funcy import first, ignore from dvc.progress import Tqdm from dvc.scm.base import CloneError, RevError, SCMError @@ -111,8 +111,11 @@ class GitPythonBackend(BaseGitBackend): # pylint:disable=abstract-method def git(self): return self.repo.git - def is_ignored(self, path): - raise NotImplementedError + def is_ignored(self, path: str) -> bool: + from git.exc import GitCommandError + + func = ignore(GitCommandError)(self.repo.git.check_ignore) + return bool(func(path)) @property def root_dir(self) -> str: diff --git a/dvc/scm/git/backend/pygit2.py b/dvc/scm/git/backend/pygit2.py index d24772b8f..642b1fef9 100644 --- a/dvc/scm/git/backend/pygit2.py +++ b/dvc/scm/git/backend/pygit2.py @@ -5,6 +5,7 @@ from io import BytesIO, StringIO from typing import Callable, Iterable, Mapping, Optional, Tuple from dvc.scm.base import SCMError +from dvc.utils import relpath from ..objects import GitObject from .base import BaseGitBackend @@ -146,8 +147,8 @@ class Pygit2Backend(BaseGitBackend): # pylint:disable=abstract-method def _get_stash(self, ref: str): raise NotImplementedError - def is_ignored(self, path): - raise NotImplementedError + def is_ignored(self, path: str) -> bool: + return self.repo.path_is_ignored(relpath(path, self.root_dir)) def set_ref( self, diff --git a/tests/func/test_scm.py b/tests/func/test_scm.py index ae5f9f700..8a14ceb4e 100644 --- a/tests/func/test_scm.py +++ b/tests/func/test_scm.py @@ -78,10 +78,12 @@ def test_ignore(tmp_dir, scm): scm.ignore(foo) assert (tmp_dir / ".gitignore").is_file() + scm._reset() assert _count_gitignore_entries(target) == 1 scm.ignore(foo) assert (tmp_dir / ".gitignore").is_file() + scm._reset() assert _count_gitignore_entries(target) == 1 scm.ignore_remove(foo)