From f70a1d2fcfb24bc1e4d9a9dbadae467645afb23e Mon Sep 17 00:00:00 2001 From: guoxiaojia <90626241+asnlsxx@users.noreply.github.com> Date: Sun, 18 Oct 2026 12:59:11 +0800 Subject: [PATCH] Fix ancestor exclusion checks after gitignore negations Check strict ancestors even when the last directory match is a negation or the matching pattern has no directory marker. Return the excluding ancestor's pattern index consistently across Simple, RE2, and Hyperscan. Cover nested directory exceptions, terminal globstars, batch filtering, re-inclusion controls, and bounded ancestor matching in regression tests. --- CHANGES_1.in.rst | 2 + pathspec/_backends/hyperscan/gitignore.py | 27 +++++---- pathspec/_backends/re2/gitignore.py | 27 +++++---- pathspec/_backends/simple/gitignore.py | 27 +++++---- tests/test_06_gitignore.py | 8 +-- tests/test_09_gitignore_ancestor_negation.py | 64 ++++++++++++++++++++ 6 files changed, 117 insertions(+), 38 deletions(-) create mode 100644 tests/test_09_gitignore_ancestor_negation.py diff --git a/CHANGES_1.in.rst b/CHANGES_1.in.rst index 1d1948b..82f309b 100644 --- a/CHANGES_1.in.rst +++ b/CHANGES_1.in.rst @@ -20,6 +20,8 @@ New features: Bug fixes: +- Directory negations no longer re-include descendants of an excluded ancestor. + All backends return the excluding ancestor's pattern index from ``check_file``. - Hyperscan backend no longer fails on multibyte UTF-8 characters. - `Pull #123`_: Ignore invalid gitignore bracket ranges for `GitIgnoreSpec`. - `Pull #128`_: Support POSIX character classes (e.g. `[[:alpha:]]`) in gitignore bracket expressions. diff --git a/pathspec/_backends/hyperscan/gitignore.py b/pathspec/_backends/hyperscan/gitignore.py index 6b65ecd..057a976 100644 --- a/pathspec/_backends/hyperscan/gitignore.py +++ b/pathspec/_backends/hyperscan/gitignore.py @@ -222,7 +222,7 @@ def _match( """ Implements :meth:`match_file`. *check_ancestors* (:class:`bool`) is whether to ask if an ancestor directory of *file* is excluded; see - :meth:`_ancestor_excluded` for why it can be skipped. + :meth:`_match_ancestor` for why it can be skipped. """ # NOTICE: According to benchmarking, a method callback is 13% faster than # using a closure here. @@ -236,9 +236,14 @@ def _match( db.scan(file.encode('utf8'), match_event_handler=self.__on_match) dir_include, dir_index, file_include, file_index = self._out - if dir_include and check_ancestors and self._ancestor_excluded(file): - out_include, out_index = dir_include, dir_index - elif file_include is not None: + if check_ancestors and (dir_include is not None or file_include is not None): + # Neither file nor directory negations override an excluded ancestor. + # Some patterns (e.g. "a/**") match without a directory marker. + ancestor_index = self._match_ancestor(file) + if ancestor_index is not None: + return (True, ancestor_index) + + if file_include is not None: out_include, out_index = file_include, file_index elif dir_include: # An ancestor matched an exclude pattern, but the spec as a whole @@ -249,11 +254,11 @@ def _match( return (out_include, out_index if out_index != -1 else None) - def _ancestor_excluded(self, file: str) -> bool: + def _match_ancestor(self, file: str) -> Optional[int]: """ - Whether any strict ancestor directory of *file* is excluded. Git stops - descending at the first excluded directory, so the ancestors are asked - outermost first, each as a directory query (trailing slash included). + Return the excluding pattern index for a strict ancestor, or :data:`None`. + Git stops descending at the first excluded directory, so the ancestors are + asked outermost first, each as a directory query (trailing slash included). By the time an ancestor is asked, every ancestor above it is known not to be excluded, so it is matched without checking its own ancestors again; otherwise each level re-asks all the levels above it and the work grows @@ -261,14 +266,14 @@ def _ancestor_excluded(self, file: str) -> bool: """ index = file.find('/') while index != -1 and index + 1 < len(file): - ancestor_include, _ancestor_index = self._match( + ancestor_include, ancestor_index = self._match( file[:index + 1], check_ancestors=False, ) if ancestor_include: - return True + return ancestor_index index = file.find('/', index + 1) - return False + return None @override diff --git a/pathspec/_backends/re2/gitignore.py b/pathspec/_backends/re2/gitignore.py index 02ef06f..879c885 100644 --- a/pathspec/_backends/re2/gitignore.py +++ b/pathspec/_backends/re2/gitignore.py @@ -164,7 +164,7 @@ def _match( """ Implements :meth:`match_file`. *check_ancestors* (:class:`bool`) is whether to ask if an ancestor directory of *file* is excluded; see - :meth:`_ancestor_excluded` for why it can be skipped. + :meth:`_match_ancestor` for why it can be skipped. """ # Find best match. match_ids: Optional[list[int]] = self._set.Match(file) # type: ignore[assignment] @@ -197,9 +197,14 @@ def _match( file_index = index assert dir_index != -1 or file_index != -1, (dir_index, file_index) - if dir_include and check_ancestors and self._ancestor_excluded(file): - return (dir_include, dir_index) - elif file_include is not None: + if check_ancestors and (dir_include is not None or file_include is not None): + # Neither file nor directory negations override an excluded ancestor. + # Some patterns (e.g. "a/**") match without a directory marker. + ancestor_index = self._match_ancestor(file) + if ancestor_index is not None: + return (True, ancestor_index) + + if file_include is not None: return (file_include, file_index) elif dir_include: # An ancestor matched an exclude pattern, but the spec as a whole @@ -208,11 +213,11 @@ def _match( else: return (dir_include, dir_index) - def _ancestor_excluded(self, file: str) -> bool: + def _match_ancestor(self, file: str) -> Optional[int]: """ - Whether any strict ancestor directory of *file* is excluded. Git stops - descending at the first excluded directory, so the ancestors are asked - outermost first, each as a directory query (trailing slash included). + Return the excluding pattern index for a strict ancestor, or :data:`None`. + Git stops descending at the first excluded directory, so the ancestors are + asked outermost first, each as a directory query (trailing slash included). By the time an ancestor is asked, every ancestor above it is known not to be excluded, so it is matched without checking its own ancestors again; otherwise each level re-asks all the levels above it and the work grows @@ -220,11 +225,11 @@ def _ancestor_excluded(self, file: str) -> bool: """ index = file.find('/') while index != -1 and index + 1 < len(file): - ancestor_include, _ancestor_index = self._match( + ancestor_include, ancestor_index = self._match( file[:index + 1], check_ancestors=False, ) if ancestor_include: - return True + return ancestor_index index = file.find('/', index + 1) - return False + return None diff --git a/pathspec/_backends/simple/gitignore.py b/pathspec/_backends/simple/gitignore.py index 0723ab0..1036fcb 100644 --- a/pathspec/_backends/simple/gitignore.py +++ b/pathspec/_backends/simple/gitignore.py @@ -70,7 +70,7 @@ def _match( """ Implements :meth:`match_file`. *check_ancestors* (:class:`bool`) is whether to ask if an ancestor directory of *file* is excluded; see - :meth:`_ancestor_excluded` for why it can be skipped. + :meth:`_match_ancestor` for why it can be skipped. """ is_reversed = self._is_reversed @@ -112,9 +112,14 @@ def _match( file_include = include file_index = index - if dir_include and check_ancestors and self._ancestor_excluded(file): - return (dir_include, dir_index) - elif file_include is not None: + if check_ancestors and (dir_include is not None or file_include is not None): + # Neither file nor directory negations override an excluded ancestor. + # Some patterns (e.g. "a/**") match without a directory marker. + ancestor_index = self._match_ancestor(file) + if ancestor_index is not None: + return (True, ancestor_index) + + if file_include is not None: return (file_include, file_index) elif dir_include: # An ancestor matched an exclude pattern, but the spec as a whole @@ -123,11 +128,11 @@ def _match( else: return (dir_include, dir_index) - def _ancestor_excluded(self, file: str) -> bool: + def _match_ancestor(self, file: str) -> Optional[int]: """ - Whether any strict ancestor directory of *file* is excluded. Git stops - descending at the first excluded directory, so the ancestors are asked - outermost first, each as a directory query (trailing slash included). + Return the excluding pattern index for a strict ancestor, or :data:`None`. + Git stops descending at the first excluded directory, so the ancestors are + asked outermost first, each as a directory query (trailing slash included). By the time an ancestor is asked, every ancestor above it is known not to be excluded, so it is matched without checking its own ancestors again; otherwise each level re-asks all the levels above it and the work grows @@ -135,12 +140,12 @@ def _ancestor_excluded(self, file: str) -> bool: """ index = file.find('/') while index != -1 and index + 1 < len(file): - ancestor_include, _ancestor_index = self._match( + ancestor_include, ancestor_index = self._match( file[:index + 1], check_ancestors=False, ) if ancestor_include: - return True + return ancestor_index index = file.find('/', index + 1) - return False + return None diff --git a/tests/test_06_gitignore.py b/tests/test_06_gitignore.py index 1ef4788..523e905 100644 --- a/tests/test_06_gitignore.py +++ b/tests/test_06_gitignore.py @@ -1139,7 +1139,7 @@ def test_c(self): def test_d(self): """ - Document terminal-globstar negation behavior without changing resolution. + Test terminal-globstar negation with excluded ancestor directories. """ for sub_test in self.parameterize_from_lines([ "d/**/", @@ -1153,10 +1153,8 @@ def test_d(self): self.assertFalse(spec.match_file("d/child/")) self.assertFalse(spec.match_file("d/child/top.txt")) self.assertTrue(spec.match_file("d/child/grand/")) - # Existing flat last-match resolution lets the negated directory - # pattern match this descendant too. Git's ancestor-sensitive result - # differs, but changing that separate behavior is outside this fix. - self.assertFalse(spec.match_file("d/child/grand/deep.txt")) + # Re-including child does not re-include the excluded grand directory. + self.assertTrue(spec.match_file("d/child/grand/deep.txt")) for sub_test in self.parameterize_from_lines(["!d/**/"]): with sub_test() as spec: diff --git a/tests/test_09_gitignore_ancestor_negation.py b/tests/test_09_gitignore_ancestor_negation.py new file mode 100644 index 0000000..99d03c0 --- /dev/null +++ b/tests/test_09_gitignore_ancestor_negation.py @@ -0,0 +1,64 @@ +"""Test directory negations below an excluded ancestor.""" + +from unittest import TestCase +from unittest.mock import patch + +from .test_06_gitignore import GitIgnoreSpecMixin + + +class AncestorNegationTest(GitIgnoreSpecMixin, TestCase): + + def test_excluded_ancestor(self): + # Expected ignore decisions and pattern indices verified with + # git check-ignore --no-index (Git 2.52.0.windows.1). + for lines, paths in ( + (['a/', '!a/b/'], ['a/b/file.txt', 'a/b/c/deep.txt']), + (['a/', '!a/b/', '!*.txt'], ['a/b/file.txt']), + (['a/', '!a/b/', '*.txt'], ['a/b/file.txt']), + (['d/**/', '!d/child/'], ['d/child/grand/deep.txt']), + (['a/**', '!a/b/**'], ['a/b/file.txt', 'a/b/c/deep.txt']), + (['a/**', '!*.txt'], ['a/b/file.txt']), + (['*', '!*.txt'], ['a/top.txt', 'a/b/file.txt']), + ): + for sub_test in self.parameterize_from_lines(lines): + with sub_test() as spec: + results = list(spec.check_files(iter(paths))) + self.assertEqual( + [(r.include, r.index) for r in results], + [(True, 0)] * len(paths), + (lines, results), + ) + self.assertEqual(list(spec.match_files(iter(paths))), paths) + self.assertEqual(list(spec.match_files(iter(paths), negate=True)), []) + + def test_reincluded_ancestors(self): + for lines, path in ( + (['a/', '!a/', '!a/b/'], 'a/b/file.txt'), + (['a/*', '!a/b/'], 'a/b/file.txt'), + (['d/**/', '!d/child/'], 'd/child/top.txt'), + (['d/**/', '!d/child/', '!d/child/grand/'], 'd/child/grand/deep.txt'), + (['!a/b/'], 'a/b/file.txt'), + (['a/**', '!*.txt'], 'a/top.txt'), + (['*', '!a/', '!a/**'], 'a/b/file.txt'), + ): + for sub_test in self.parameterize_from_lines(lines): + with sub_test() as spec: + self.assertFalse(spec.match_file(path), (lines, path)) + + def test_ancestors_checked_once(self): + # Keep the ancestor walk iterative even with many re-included levels. + path = 'a/' * 40 + 'file.txt' + for sub_test in self.parameterize_from_lines(['a/', '!a/']): + with sub_test() as spec: + backend_type = type(spec._backend) + original = backend_type._match + calls = [] + + def counted_match(backend, file, check_ancestors): + calls.append((file, check_ancestors)) + return original(backend, file, check_ancestors) + + with patch.object(backend_type, '_match', counted_match): + self.assertFalse(spec.match_file(path)) + self.assertLessEqual(len(calls), 41) + self.assertTrue(all(not check for _, check in calls[1:]))