From a99b1298c2d81ed6afd7064fed17694956959877 Mon Sep 17 00:00:00 2001 From: youdie006 Date: Tue, 18 Aug 2026 15:08:27 +0900 Subject: [PATCH] Fix GitIgnoreSpec re-including files under an excluded directory GitIgnoreSpec resolves patterns with a flat last-match, so a file-level negation could re-include a file whose parent directory is excluded, which git forbids ("It is not possible to re-include a file if a parent directory of that file is excluded"). For example ["build", "!keep.log"] wrongly treated build/keep.log as not-ignored, while real git check-ignore ignores it. Wrap the gitignore backend (_AncestorDirBackend): when a file is not already ignored by its own resolution, walk the file's ancestor directory prefixes and, for each, ask whether that directory is excluded, resolved as a directory (ancestor + "/") using git's plain last-match order via util.check_match_file. If any ancestor directory is excluded, the file is ignored regardless of a later file-level negation. Resolving the ancestor as a directory preserves directory-level re-inclusions, so "build/*" + "!build/keep.log" still re-includes (build/* excludes only the contents, not the build directory itself), as do the !libfoo/!libfoo/** and !*/ idioms. A small _wrap_backend extension point is added on PathSpec (a no-op by default, overridden by GitIgnoreSpec) so the wrapper applies uniformly, including through the internal test backend factory. Fixes #129. --- pathspec/gitignore.py | 87 ++++++++++++++++++++++++++++++++++++ pathspec/pathspec.py | 31 ++++++++++++- tests/test_06_gitignore.py | 91 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 207 insertions(+), 2 deletions(-) diff --git a/pathspec/gitignore.py b/pathspec/gitignore.py index 9e8e6f8..3f00f2d 100644 --- a/pathspec/gitignore.py +++ b/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) diff --git a/pathspec/pathspec.py b/pathspec/pathspec.py index 3e3872a..5137bef 100644 --- a/pathspec/pathspec.py +++ b/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], diff --git a/tests/test_06_gitignore.py b/tests/test_06_gitignore.py index 9d00c90..c1ae6f1 100644 --- a/tests/test_06_gitignore.py +++ b/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)