From 2048251e24ede8df403d7397c1f5bd1e68eb130a Mon Sep 17 00:00:00 2001 From: Cheruku Sri Charan Reddy <185475071+sricharanreddycheruku@users.noreply.github.com> Date: Wed, 7 Oct 2026 05:09:34 +0000 Subject: [PATCH] Resolve excluded ancestor directories before descendant negations --- CHANGES.rst | 2 + README.rst | 7 ++ pathspec/_backends/hyperscan/gitignore.py | 23 ++--- pathspec/_backends/re2/gitignore.py | 24 ++--- pathspec/_backends/simple/gitignore.py | 24 ++--- tests/test_09_excluded_ancestors.py | 101 ++++++++++++++++++++++ 6 files changed, 149 insertions(+), 32 deletions(-) create mode 100644 tests/test_09_excluded_ancestors.py diff --git a/CHANGES.rst b/CHANGES.rst index b3e7efc..f6e6ade 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -20,6 +20,8 @@ New features: Bug fixes: +- Resolve excluded ancestor directories before descendant negations in all GitIgnoreSpec backends, and report the blocking pattern index. + - Honor `on_error` when opening a directory fails during tree traversal. - `Pull #123`_: Ignore invalid gitignore bracket ranges for `GitIgnoreSpec`. diff --git a/README.rst b/README.rst index 9824fb9..61e0aee 100644 --- a/README.rst +++ b/README.rst @@ -93,6 +93,13 @@ handles these cases to more closely replicate Git's behavior:: You do not specify the style of pattern for ``GitIgnoreSpec`` because it should always use ``GitIgnoreSpecPattern`` internally. +A negated file pattern cannot re-include a file while any of its parent +directories remains excluded. For example, ``dir/**`` followed by +``!dir/**/file`` re-includes ``dir/file``, but leaves ``dir/nested/file`` ignored +because ``dir/nested/`` is still excluded. Re-include the necessary parent +directories as well. ``GitIgnoreSpec.check_file()`` reports the pattern index +that excludes the first blocking ancestor in this situation. + Performance ----------- diff --git a/pathspec/_backends/hyperscan/gitignore.py b/pathspec/_backends/hyperscan/gitignore.py index 133b5cc..71b4c93 100644 --- a/pathspec/_backends/hyperscan/gitignore.py +++ b/pathspec/_backends/hyperscan/gitignore.py @@ -204,8 +204,13 @@ 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:`_excluded_ancestor_index` for why it can be skipped. """ + if check_ancestors: + ancestor_index = self._excluded_ancestor_index(file) + if ancestor_index is not None: + return (True, ancestor_index) + # NOTICE: According to benchmarking, a method callback is 13% faster than # using a closure here. db = self._db @@ -218,9 +223,7 @@ 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 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 @@ -231,10 +234,10 @@ def _match( return (out_include, out_index if out_index != -1 else None) - def _ancestor_excluded(self, file: str) -> bool: + def _excluded_ancestor_index(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 + Return the pattern index excluding the first strict ancestor, or None. + Git stops descending at the first excluded directory, so 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; @@ -243,14 +246,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 de8b15f..dbed310 100644 --- a/pathspec/_backends/re2/gitignore.py +++ b/pathspec/_backends/re2/gitignore.py @@ -153,8 +153,13 @@ 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:`_excluded_ancestor_index` for why it can be skipped. """ + if check_ancestors: + ancestor_index = self._excluded_ancestor_index(file) + if ancestor_index is not None: + return (True, ancestor_index) + # Find best match. match_ids: Optional[list[int]] = self._set.Match(file) # type: ignore[assignment] if not match_ids: @@ -186,9 +191,7 @@ 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 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 @@ -197,10 +200,10 @@ def _match( else: return (dir_include, dir_index) - def _ancestor_excluded(self, file: str) -> bool: + def _excluded_ancestor_index(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 + Return the pattern index excluding the first strict ancestor, or None. + Git stops descending at the first excluded directory, so 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; @@ -209,12 +212,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..2e4d3b9 100644 --- a/pathspec/_backends/simple/gitignore.py +++ b/pathspec/_backends/simple/gitignore.py @@ -70,8 +70,13 @@ 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:`_excluded_ancestor_index` for why it can be skipped. """ + if check_ancestors: + ancestor_index = self._excluded_ancestor_index(file) + if ancestor_index is not None: + return (True, ancestor_index) + is_reversed = self._is_reversed # Resolve the ancestor directory and the file separately: a file negation @@ -112,9 +117,7 @@ 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 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,10 +126,10 @@ def _match( else: return (dir_include, dir_index) - def _ancestor_excluded(self, file: str) -> bool: + def _excluded_ancestor_index(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 + Return the pattern index excluding the first strict ancestor, or None. + Git stops descending at the first excluded directory, so 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; @@ -135,12 +138,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/tests/test_09_excluded_ancestors.py b/tests/test_09_excluded_ancestors.py new file mode 100644 index 0000000..78d5d65 --- /dev/null +++ b/tests/test_09_excluded_ancestors.py @@ -0,0 +1,101 @@ +""" +Tests descendant negations when a strict ancestor remains excluded. +""" + +from unittest import TestCase + +from .test_06_gitignore import GitIgnoreSpecMixin + + +class ExcludedAncestorTest(GitIgnoreSpecMixin, TestCase): + """ + Tests Git's rule that a file cannot be re-included under an excluded directory. + """ + + def test_globstar_descendant_negation(self): + """ + A recursive file negation does not open the directories excluded by dir/**. + """ + for begin in self.parameterize_from_lines(['dir/**', '!dir/**/file']): + with begin() as spec: + self.assertEqual(spec.check_file('dir/file').include, False) + for file in ('dir/nested/file', 'dir/a/b/c/file', 'dir/nested/'): + with self.subTest(file=file): + result = spec.check_file(file) + self.assertEqual((result.include, result.index), (True, 0)) + self.assertIsNone(spec.check_file('dir/').include) + self.assertIsNone(spec.check_file('elsewhere/file').include) + + def test_match_all_file_negation(self): + """ + A match-all shortcut also excludes parent directories before file negations. + """ + for begin in self.parameterize_from_lines(['*', '!**/a.py']): + with begin() as spec: + self.assertEqual(spec.check_file('a.py').include, False) + for file in ('dir/a.py', 'dir/nested/a.py'): + with self.subTest(file=file): + result = spec.check_file(file) + self.assertEqual((result.include, result.index), (True, 0)) + + def test_parent_negation_does_not_open_all_descendants(self): + """ + Re-including one directory leaves its still-excluded nested directories closed. + """ + for begin in self.parameterize_from_lines(['*/', '!src/']): + with begin() as spec: + self.assertFalse(spec.match_file('src/a.txt')) + for file in ('src/nested/file', 'src/a/b/file', 'src/nested/'): + with self.subTest(file=file): + result = spec.check_file(file) + self.assertEqual((result.include, result.index), (True, 0)) + + def test_reinclude_each_ancestor(self): + """ + A negated leaf becomes reachable only when each excluded ancestor is reopened. + """ + lines = ['dir/**', '!dir/a/', '!dir/**/file'] + for begin in self.parameterize_from_lines(lines): + with begin() as spec: + self.assertFalse(spec.match_file('dir/file')) + self.assertFalse(spec.match_file('dir/a/file')) + self.assertTrue(spec.match_file('dir/a/b/file')) + self.assertTrue(spec.match_file('dir/a/b/c/file')) + + for begin in self.parameterize_from_lines(lines + ['!dir/a/b/']): + with begin() as spec: + self.assertFalse(spec.match_file('dir/a/b/file')) + self.assertTrue(spec.match_file('dir/a/b/c/file')) + + for begin in self.parameterize_from_lines(lines + ['!dir/a/b/', '!dir/a/b/c/']): + with begin() as spec: + self.assertFalse(spec.match_file('dir/a/b/c/file')) + + def test_first_blocking_ancestor_index(self): + """ + The reported index belongs to the first directory preventing traversal. + """ + for begin in self.parameterize_from_lines([ + 'dir/', 'dir/nested/', '!dir/nested/file', + ]): + with begin() as spec: + for file in ('dir/nested/file', 'dir/nested/'): + result = spec.check_file(file) + self.assertEqual((result.include, result.index), (True, 0)) + + for begin in self.parameterize_from_lines([ + 'dir/', 'dir/nested/', '!dir/', '!dir/nested/file', + ]): + with begin() as spec: + result = spec.check_file('dir/nested/file') + self.assertEqual((result.include, result.index), (True, 1)) + + def test_only_negations_and_unmatched_files(self): + """ + No implicit exclusion is introduced by ancestor inspection. + """ + for begin in self.parameterize_from_lines(['!src/', '!**/a.py']): + with begin() as spec: + self.assertFalse(spec.match_file('src/nested/a.py')) + self.assertFalse(spec.match_file('src/nested/b.py')) + self.assertIsNone(spec.check_file('elsewhere/file').include)