Fix GitIgnoreSpec re-including files under an excluded directory (#129)

pathspec/gitignore.py

@@ -37,6 +37,7 @@
 	override)  # Added in 3.12.
 from pathspec.util import (
 	_is_iterable,
+	check_match_file,
 	lookup_pattern)
 
 Self = TypeVar("Self", bound='GitIgnoreSpec')
@@ -46,6 +47,71 @@
 """
 
 
+class _AncestorDirBackend(_Backend):
+	"""
+	.. warning:: This class is not part of the public API. It is subject to
+		change.
+
+	The :class:`_AncestorDirBackend` class wraps a *gitignore* backend to enforce
+	Git's rule that "it is not possible to re-include a file if a parent directory
+	of that file is excluded" (see issue #129).
+
+	The wrapped backend resolves patterns with a flat last-match, so a file-level
+	negation (e.g. ``!keep.log``) can incorrectly re-include a file whose ancestor
+	directory is excluded (e.g. ``build``). Git evaluates paths hierarchically:
+	once a directory is excluded, nothing beneath it can be re-included.
+
+	This wrapper walks the ancestor directory prefixes of a path. If any ancestor
+	directory is itself excluded, the path is ignored regardless of a later
+	file-level negation. Each ancestor is resolved as a directory (its path plus a
+	trailing slash) using Git's plain last-match order (:func:`.check_match_file`),
+	so a directory-level re-inclusion (e.g. ``!build/`` or ``!*/``) correctly keeps
+	the directory matchable. This distinguishes excluding a directory (``build``,
+	which matches the directory) from excluding only its contents (``build/*``,
+	which does not match ``build`` itself and still allows re-inclusion). It is
+	backend-agnostic and costs O(path-depth) extra resolutions per file, and only
+	when the file was not already ignored.
+	"""
+
+	__slots__ = ('_backend', '_indexed')
+
+	def __init__(
+		self,
+		backend: _Backend,
+		patterns: Sequence[Pattern],
+	) -> None:
+		"""
+		Initialize the :class:`_AncestorDirBackend` instance.
+
+		*backend* (:class:`._Backend`) is the wrapped *gitignore* backend used to
+		resolve the file itself.
+
+		*patterns* (:class:`~collections.abc.Sequence` of :class:`.Pattern`) is the
+		compiled patterns, used to resolve ancestor directories with Git's plain
+		last-match order.
+		"""
+		self._backend = backend
+		self._indexed: list[tuple[int, Pattern]] = list(enumerate(patterns))
+
+	@override
+	def match_file(self, file: str) -> tuple[Optional[bool], Optional[int]]:
+		include, index = self._backend.match_file(file)
+		if include is not True:
+			# The file was not ignored by its own last-match. Check whether any
+			# ancestor directory is itself excluded; if so, Git keeps the file
+			# ignored regardless of a later file-level re-inclusion.
+			sep = file.find('/')
+			while sep != -1:
+				anc_include, anc_index = check_match_file(
+					self._indexed, file[:sep] + '/', False,
+				)
+				if anc_include is True:
+					return (True, anc_index)
+				sep = file.find('/', sep + 1)
+
+		return (include, index)
+
+
 class GitIgnoreSpec(PathSpec[GitIgnoreSpecPattern]):
 	"""
 	The :class:`GitIgnoreSpec` class extends :class:`.PathSpec` to replicate
@@ -170,3 +236,24 @@ def _make_backend(
 		Returns the backend (:class:`._Backend`).
 		"""
 		return make_gitignore_backend(name, patterns)
+
+	@override
+	def _wrap_backend(
+		self,
+		backend: _Backend,
+		patterns: Sequence[Pattern],
+	) -> _Backend:
+		"""
+		.. warning:: This method is not part of the public API. It is subject to
+			change.
+
+		Wrap the *gitignore* *backend* (:class:`._Backend`) to enforce Git's rule
+		that a file cannot be re-included if a parent directory of that file is
+		excluded (see :class:`._AncestorDirBackend` and issue #129).
+
+		*patterns* (:class:`~collections.abc.Sequence` of :class:`.Pattern`) is the
+		compiled patterns, used to resolve ancestor directories.
+
+		Returns the wrapped backend (:class:`._Backend`).
+		"""
+		return _AncestorDirBackend(backend, patterns)

pathspec/pathspec.py

@@ -91,7 +91,7 @@ def __init__(
 		else:
 			use_backend = self._make_backend(backend_name, use_patterns)
 
-		self._backend: _Backend = use_backend
+		self._backend: _Backend = self._wrap_backend(use_backend, use_patterns)
 		"""
 		*_backend* (:class:`._Backend`) is the pattern (regular expression) matching
 		backend.
@@ -142,7 +142,10 @@ def __iadd__(self: Self, other: PathSpec) -> Self:  # type: ignore[misc]
 		"""
 		if isinstance(other, PathSpec):
 			self.patterns = [*self.patterns, *other.patterns]
-			self._backend = self._make_backend(self._backend_name, self.patterns)
+			self._backend = self._wrap_backend(
+				self._make_backend(self._backend_name, self.patterns),
+				self.patterns,
+			)
 			return self
 		else:
 			return NotImplemented
@@ -347,6 +350,30 @@ def _make_backend(
 		"""
 		return make_pathspec_backend(name, patterns)
 
+	def _wrap_backend(
+		self,
+		backend: _Backend,
+		patterns: Sequence[Pattern],
+	) -> _Backend:
+		"""
+		.. warning:: This method is not part of the public API. It is subject to
+			change.
+
+		Optionally post-process the constructed *backend* (:class:`._Backend`)
+		before it is stored. This is applied to every backend regardless of how it
+		was created (including via the internal test backend factory).
+
+		*patterns* (:class:`~collections.abc.Sequence` of :class:`.Pattern`)
+		contains the compiled patterns.
+
+		The base implementation returns *backend* unchanged. Subclasses may
+		override it (e.g., :class:`.GitIgnoreSpec` wraps the backend to enforce
+		Git's parent-directory exclusion rule).
+
+		Returns the backend to use (:class:`._Backend`).
+		"""
+		return backend
+
 	def match_entries(
 		self,
 		entries: Iterable[TreeEntry],

tests/test_06_gitignore.py

@@ -689,3 +689,94 @@ def test_09_issue_100(self):
 				includes = get_includes(results)
 				debug = debug_results(spec, results)
 				self.assertEqual(includes, set(), debug)
+
+	def test_10_issue_129_a(self):
+		"""
+		Test that a file cannot be re-included when a parent directory is excluded,
+		scenario A.
+
+		Git forbids re-including a file if a parent directory is excluded, so
+		"build/keep.log" stays ignored even though "!keep.log" would re-include it.
+		A root-level "keep.log" has no excluded parent, so it is re-included.
+		"""
+		for sub_test in self.parameterize_from_lines([
+			'build',
+			'!keep.log',
+		]):
+			with sub_test() as spec:
+				# Confirmed results with git check-ignore (v2.54.0).
+				files = {
+					'build/keep.log',   # ignored: parent "build" is excluded
+					'build/other.txt',  # ignored: parent "build" is excluded
+					'keep.log',         # -: re-included, no excluded parent
+				}
+
+				results = list(spec.check_files(files))
+				ignores = get_includes(results)
+				debug = debug_results(spec, results)
+
+				self.assertEqual(ignores, {
+					'build/keep.log',
+					'build/other.txt',
+				}, debug)
+				self.assertEqual(files - ignores, {
+					'keep.log',
+				}, debug)
+
+	def test_10_issue_129_b(self):
+		"""
+		Test that a file cannot be re-included when a parent directory is excluded,
+		scenario B.
+
+		The excluded directory is a single-character name to guard against a naive
+		prefix comparison.
+		"""
+		for sub_test in self.parameterize_from_lines([
+			'a',
+			'!keep.log',
+		]):
+			with sub_test() as spec:
+				# Confirmed results with git check-ignore (v2.54.0).
+				files = {
+					'a/keep.log',  # ignored: parent "a" is excluded
+				}
+
+				results = list(spec.check_files(files))
+				ignores = get_includes(results)
+				debug = debug_results(spec, results)
+
+				self.assertEqual(ignores, {
+					'a/keep.log',
+				}, debug)
+				self.assertEqual(files - ignores, set(), debug)
+
+	def test_10_issue_129_c(self):
+		"""
+		Test that excluding a directory's contents (not the directory itself) still
+		allows re-inclusion, scenario C.
+
+		"build/*" excludes only the contents of "build" (it does not match the
+		"build" directory itself), so "!build/keep.log" re-includes the file. This
+		must not be broken by the parent-exclusion fix.
+		"""
+		for sub_test in self.parameterize_from_lines([
+			'build/*',
+			'!build/keep.log',
+		]):
+			with sub_test() as spec:
+				# Confirmed results with git check-ignore (v2.54.0).
+				files = {
+					'build/keep.log',   # -: re-included (only contents excluded)
+					'build/other.txt',  # ignored: 1:build/*
+				}
+
+				results = list(spec.check_files(files))
+				ignores = get_includes(results)
+				debug = debug_results(spec, results)
+
+				self.assertEqual(ignores, {
+					'build/other.txt',
+				}, debug)
+				self.assertEqual(files - ignores, {
+					'build/keep.log',
+				}, debug)