git: Keep trailing slash when checking if directory is gitignored (#6232)

* Keep trailing slash when checking if directory is gitignored

* Calling is_ignored with correct type (could be Path-like)

* Fix test to comply to annotated types

* Fix separator issues

* Relaxed typing in is_ignored

* Added test

* Test scm

* Fix test on Windows

* Expanded test_ignored_dir_unignored_subdirs

* Even more assertions

dvc/repo/stage.py

@@ -462,7 +462,8 @@ class StageLoad:
 
         def is_out_or_ignored(root, directory):
             dir_path = f"{root}{sep}{directory}"
-            return dir_path in outs or is_ignored(dir_path)
+            # trailing slash needed to check if a directory is gitignored
+            return dir_path in outs or is_ignored(f"{dir_path}{sep}")
 
         stages = []
         for root, dirs, files in self.repo.dvcignore.walk(

dvc/scm/git/backend/dulwich.py

@@ -27,6 +27,8 @@ from ..objects import GitObject
 from .base import BaseGitBackend
 
 if TYPE_CHECKING:
+    from dvc.types import StrPath
+
     from ..objects import GitCommit
 
 logger = logging.getLogger(__name__)
@@ -278,12 +280,14 @@ class DulwichBackend(BaseGitBackend):  # pylint:disable=abstract-method
 
         return IgnoreFilterManager.from_repo(self.repo)
 
-    def is_ignored(self, path: str) -> bool:
+    def is_ignored(self, path: "StrPath") -> 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))
-        )
+        relative_path = relpath(path, self.root_dir)
+        # if checking a directory, a trailing slash must be included
+        if str(path)[-1] == os.sep:
+            relative_path += os.sep
+        return bool(self.ignore_manager.is_ignored(relative_path))
 
     def set_ref(
         self,

dvc/scm/git/backend/gitpython.py

@@ -3,7 +3,16 @@ import locale
 import logging
 import os
 from functools import partial
-from typing import Callable, Iterable, List, Mapping, Optional, Tuple, Union
+from typing import (
+    TYPE_CHECKING,
+    Callable,
+    Iterable,
+    List,
+    Mapping,
+    Optional,
+    Tuple,
+    Union,
+)
 
 from funcy import ignore
 
@@ -14,6 +23,9 @@ from dvc.utils import fix_env, is_binary, relpath
 from ..objects import GitCommit, GitObject
 from .base import BaseGitBackend
 
+if TYPE_CHECKING:
+    from dvc.types import StrPath
+
 logger = logging.getLogger(__name__)
 
 
@@ -111,11 +123,11 @@ class GitPythonBackend(BaseGitBackend):  # pylint:disable=abstract-method
     def git(self):
         return self.repo.git
 
-    def is_ignored(self, path: str) -> bool:
+    def is_ignored(self, path: "StrPath") -> bool:
         from git.exc import GitCommandError
 
         func = ignore(GitCommandError)(self.repo.git.check_ignore)
-        return bool(func(path))
+        return bool(func(str(path)))
 
     @property
     def root_dir(self) -> str:

dvc/scm/git/backend/pygit2.py

@@ -4,7 +4,16 @@ import os
 import stat
 from contextlib import contextmanager
 from io import BytesIO, StringIO
-from typing import Callable, Iterable, List, Mapping, Optional, Tuple, Union
+from typing import (
+    TYPE_CHECKING,
+    Callable,
+    Iterable,
+    List,
+    Mapping,
+    Optional,
+    Tuple,
+    Union,
+)
 
 from funcy import cached_property
 
@@ -17,6 +26,10 @@ from .base import BaseGitBackend
 logger = logging.getLogger(__name__)
 
 
+if TYPE_CHECKING:
+    from dvc.types import StrPath
+
+
 class Pygit2Object(GitObject):
     def __init__(self, obj):
         self.obj = obj
@@ -234,7 +247,7 @@ class Pygit2Backend(BaseGitBackend):  # pylint:disable=abstract-method
     def _get_stash(self, ref: str):
         raise NotImplementedError
 
-    def is_ignored(self, path: str) -> bool:
+    def is_ignored(self, path: "StrPath") -> bool:
         rel = relpath(path, self.root_dir)
         if os.name == "nt":
             rel.replace("\\", "/")

tests/func/test_scm.py

@@ -99,6 +99,35 @@ def test_ignored(tmp_dir, scm):
     assert not scm.is_ignored(tmp_dir / "dir1" / "file2.txt")
 
 
+def test_ignored_dir_unignored_subdirs(tmp_dir, scm):
+    tmp_dir.gen({".gitignore": "data/**\n!data/**/\n!data/**/*.csv"})
+    scm.add([".gitignore"])
+    tmp_dir.gen(
+        {
+            os.path.join("data", "raw", "tracked.csv"): "cont",
+            os.path.join("data", "raw", "not_tracked.json"): "cont",
+        }
+    )
+
+    assert not scm.is_ignored(tmp_dir / "data" / "raw" / "tracked.csv")
+    assert scm.is_ignored(tmp_dir / "data" / "raw" / "not_tracked.json")
+    assert not scm.is_ignored(tmp_dir / "data" / "raw" / "non_existent.csv")
+    assert scm.is_ignored(tmp_dir / "data" / "raw" / "non_existent.json")
+    assert not scm.is_ignored(tmp_dir / "data" / "non_existent.csv")
+    assert scm.is_ignored(tmp_dir / "data" / "non_existent.json")
+
+    assert not scm.is_ignored(f"data{os.sep}")
+    # git check-ignore would now mark "data/raw" as ignored
+    # after detecting it's a directory in the file system;
+    # instead, we rely on the trailing separator to determine if handling a
+    # a directory - for consistency between existent and non-existent paths
+    assert scm.is_ignored(os.path.join("data", "raw"))
+    assert not scm.is_ignored(os.path.join("data", f"raw{os.sep}"))
+
+    assert scm.is_ignored(os.path.join("data", "non_existent"))
+    assert not scm.is_ignored(os.path.join("data", f"non_existent{os.sep}"))
+
+
 def test_get_gitignore(tmp_dir, scm):
     tmp_dir.gen({"file1": "contents", "dir": {}})
 

tests/func/test_stage.py

@@ -227,6 +227,13 @@ def test_parent_repo_collect_stages(tmp_dir, scm, dvc):
     assert deep_subrepo_stages != []
 
 
+def test_collect_repo_ignored_dir_unignored_pattern(tmp_dir, dvc, scm):
+    tmp_dir.gen({".gitignore": "data/**\n!data/**/\n!data/**/*.dvc"})
+    scm.add([".gitignore"])
+    (stage,) = tmp_dir.dvc_gen({"data/raw/tracked.csv": "5,6,7,8"})
+    assert dvc.stage.collect_repo() == [stage]
+
+
 def test_stage_strings_representation(tmp_dir, dvc, run_copy):
     tmp_dir.dvc_gen("foo", "foo")
     stage1 = run_copy("foo", "bar", single_stage=True)