check root .gitignore in '_ignored' (#3284)

* made '_ignored' check if the given file is already ignored by the root .gitignore. Added corresponding functional test

* code formatting

* Changed method input, adjusted tests, formatted code

* typo

* tests: use git logger fix in conftest

See https://github.com/iterative/dvc/issues/3167

* scm: git: remove redundant comments

Co-authored-by: Ruslan Kuprieiev <[email protected]>

dvc/scm/git/__init__.py

@@ -18,7 +18,6 @@ from dvc.utils import is_binary
 from dvc.utils import relpath
 from dvc.utils.fs import path_isin
 
-
 logger = logging.getLogger(__name__)
 
 
@@ -133,20 +132,19 @@ class Git(Base):
 
         return entry, gitignore
 
-    @staticmethod
-    def _ignored(entry, gitignore_path):
-        if os.path.exists(gitignore_path):
-            with open(gitignore_path, "r") as fobj:
-                ignore_list = fobj.readlines()
-            return any(
-                filter(lambda x: x.strip() == entry.strip(), ignore_list)
-            )
-        return False
+    def _ignored(self, path):
+        from git.exc import GitCommandError
+
+        try:
+            self.repo.git.check_ignore(path)
+            return True
+        except GitCommandError:
+            return False
 
     def ignore(self, path):
         entry, gitignore = self._get_gitignore(path)
 
-        if self._ignored(entry, gitignore):
+        if self._ignored(path):
             return
 
         msg = "Adding '{}' to '{}'.".format(relpath(path), relpath(gitignore))

tests/dir_helpers.py

@@ -45,6 +45,7 @@ from global repo template to creating everything inplace, which:
 
 import os
 import pathlib
+import logging
 from contextlib import contextmanager
 
 import pytest
@@ -65,6 +66,11 @@ __all__ = [
 ]
 
 
+# see https://github.com/iterative/dvc/issues/3167
+git_logger = logging.getLogger("git")
+git_logger.setLevel(logging.CRITICAL)
+
+
 class TmpDir(pathlib.Path):
     def __new__(cls, *args, **kwargs):
         if cls is TmpDir:

tests/func/test_get.py

@@ -163,11 +163,7 @@ def test_get_from_non_dvc_master(tmp_dir, git_dir, caplog):
 
     caplog.clear()
 
-    # removing `git` import in conftest resulted in unexpected logs from
-    # that package, see https://github.com/iterative/dvc/issues/3167
-    with caplog.at_level(logging.INFO, logger="git"), caplog.at_level(
-        logging.INFO, logger="dvc"
-    ):
+    with caplog.at_level(logging.INFO, logger="dvc"):
         Repo.get(fspath(git_dir), "some_file", out="some_dst", rev="branch")
 
     assert caplog.text == ""

tests/func/test_scm.py

@@ -83,6 +83,14 @@ def test_ignore(tmp_dir, scm):
     assert _count_gitignore_entries(target) == 0
 
 
+def test_ignored(tmp_dir, scm):
+    tmp_dir.gen({"dir1": {"file1.jpg": "cont", "file2.txt": "cont"}})
+    tmp_dir.gen({".gitignore": "dir1/*.jpg"})
+
+    assert scm._ignored(fspath(tmp_dir / "dir1" / "file1.jpg"))
+    assert not scm._ignored(fspath(tmp_dir / "dir1" / "file2.txt"))
+
+
 def test_get_gitignore(tmp_dir, scm):
     tmp_dir.gen({"file1": "contents", "dir": {}})