From 07ce100332ea8f67d929ed3461730e70fa34d91d Mon Sep 17 00:00:00 2001 From: youdie006 Date: Thu, 3 Sep 2026 16:00:00 +0900 Subject: [PATCH] Resolve the ancestor directory and the file separately in GitIgnoreSpec A file negation could re-include a file whose parent directory is excluded, which git forbids. The three gitignore backends resolved every match through one priority ladder, so a later file pattern always outranked an earlier directory exclusion. Track two answers instead: a directory-marker match decides the ancestor, every other match decides the file. An excluded ancestor wins; otherwise the file answer stands. The priority field, the is_reversed split and the early break all collapse into the two accumulators, since last-match-wins is first-match-wins in reverse order. Fixes #129. --- pathspec/_backends/hyperscan/gitignore.py | 54 ++++++++-------- pathspec/_backends/re2/gitignore.py | 45 +++++++------- pathspec/_backends/simple/gitignore.py | 51 +++++++-------- tests/test_06_gitignore.py | 76 +++++++++++++++++++++++ 4 files changed, 146 insertions(+), 80 deletions(-) diff --git a/pathspec/_backends/hyperscan/gitignore.py b/pathspec/_backends/hyperscan/gitignore.py index fd98b6c..8d3c323 100644 --- a/pathspec/_backends/hyperscan/gitignore.py +++ b/pathspec/_backends/hyperscan/gitignore.py @@ -45,7 +45,7 @@ class HyperscanGiBackend(HyperscanPsBackend): """ # Change type hint. - _out: tuple[Optional[bool], int, int] # type: ignore[assignment] + _out: tuple[Optional[bool], int, Optional[bool], int] # type: ignore[assignment] def __init__( self, @@ -62,15 +62,17 @@ def __init__( """ super().__init__(patterns, _debug_exprs=_debug_exprs, _test_sort=_test_sort) - self._out = (None, -1, 0) + self._out = (None, -1, None, -1) """ *_out* (:class:`tuple`) stores the current match: - - *0* (:class:`bool` or :data:`None`) is the match include. + - *0* (:class:`bool` or :data:`None`) is the directory match include. - - *1* (:class:`int`) is the match index. + - *1* (:class:`int`) is the directory match index. - - *2* (:class:`int`) is the match priority. + - *2* (:class:`bool` or :data:`None`) is the file match include. + + - *3* (:class:`int`) is the file match index. """ @override @@ -199,15 +201,18 @@ def match_file(self, file: str) -> tuple[Optional[bool], Optional[int]]: # match. return (None, None) - self._out = (None, -1, 0) + self._out = (None, -1, None, -1) db.scan(file.encode('utf8'), match_event_handler=self.__on_match) - out_index: Optional[int] - out_include, out_index = self._out[:2] - if out_index == -1: - out_index = None + dir_include, dir_index, file_include, file_index = self._out + if dir_include: + out_include, out_index = dir_include, dir_index + elif file_include is not None: + out_include, out_index = file_include, file_index + else: + out_include, out_index = dir_include, dir_index - return (out_include, out_index) + return (out_include, out_index if out_index != -1 else None) @override def __on_match( @@ -226,25 +231,18 @@ def __on_match( """ expr_dat = self._expr_data[expr_id] - is_dir_pattern = expr_dat.is_dir_pattern - if is_dir_pattern: - # Pattern matched by a directory pattern. - priority = 1 - else: - # Pattern matched by a file pattern. - priority = 2 - # WARNING: Hyperscan does not guarantee matches will be produced in order! + # Resolve the ancestor directory and the file separately: a file negation + # only applies while no ancestor directory is excluded. include = expr_dat.include index = expr_dat.index - prev_index = self._out[1] - prev_priority = self._out[2] - if ( - (include and is_dir_pattern and index > prev_index) - or (priority == prev_priority and index > prev_index) - or priority > prev_priority - ): - out_tup = (include, expr_dat.index, priority) - self._out = out_tup # type: ignore + dir_include, dir_index, file_include, file_index = self._out + if expr_dat.is_dir_pattern: + # Pattern matched by a directory pattern. + if index > dir_index: + self._out = (include, index, file_include, file_index) # type: ignore + elif index > file_index: + # Pattern matched by a file pattern. + self._out = (dir_include, dir_index, include, index) # type: ignore return None diff --git a/pathspec/_backends/re2/gitignore.py b/pathspec/_backends/re2/gitignore.py index 41bbdef..987c1e6 100644 --- a/pathspec/_backends/re2/gitignore.py +++ b/pathspec/_backends/re2/gitignore.py @@ -147,34 +147,35 @@ def match_file(self, file: str) -> tuple[Optional[bool], Optional[int]]: if not match_ids: return (None, None) - out_include: Optional[bool] = None - out_index: int = -1 - out_priority = -1 + # Resolve the ancestor directory and the file separately: a file negation + # only applies while no ancestor directory is excluded. + dir_include: Optional[bool] = None + dir_index: int = -1 + file_include: Optional[bool] = None + file_index: int = -1 regex_data = self._regex_data for regex_id in match_ids: regex_dat = regex_data[regex_id] - is_dir_pattern = regex_dat.is_dir_pattern - if is_dir_pattern: - # Pattern matched by a directory pattern. - priority = 1 - else: - # Pattern matched by a file pattern. - priority = 2 - # WARNING: According to the documentation on `RE2::Set::Match()`, there is # no guarantee matches will be produced in order! include = regex_dat.include index = regex_dat.index - if ( - (include and is_dir_pattern and index > out_index) - or (priority == out_priority and index > out_index) - or priority > out_priority - ): - out_include = include - out_index = index - out_priority = priority - - assert out_index != -1, (out_index, out_include, out_priority) - return (out_include, out_index) + if regex_dat.is_dir_pattern: + # Pattern matched by a directory pattern. + if index > dir_index: + dir_include = include + dir_index = index + elif index > file_index: + # Pattern matched by a file pattern. + file_include = include + file_index = index + + assert dir_index != -1 or file_index != -1, (dir_index, file_index) + if dir_include: + return (dir_include, dir_index) + elif file_include is not None: + return (file_include, file_index) + else: + return (dir_include, dir_index) diff --git a/pathspec/_backends/simple/gitignore.py b/pathspec/_backends/simple/gitignore.py index 29701f5..94fed90 100644 --- a/pathspec/_backends/simple/gitignore.py +++ b/pathspec/_backends/simple/gitignore.py @@ -64,41 +64,32 @@ def match_file(self, file: str) -> tuple[Optional[bool], Optional[int]]: """ is_reversed = self._is_reversed - out_include: Optional[bool] = None - out_index: Optional[int] = None - out_priority = 0 + # Resolve the ancestor directory and the file separately: a file negation + # only applies while no ancestor directory is excluded. + dir_include: Optional[bool] = None + dir_index: Optional[int] = None + file_include: Optional[bool] = None + file_index: Optional[int] = None + for index, pattern in self._patterns: if ( (include := pattern.include) is not None and (match := pattern.match_file(file)) is not None ): # Pattern matched. - - # Check for directory marker. - dir_mark = match.match.groupdict().get(_DIR_MARK) - - if dir_mark: + if match.match.groupdict().get(_DIR_MARK): # Pattern matched by a directory pattern. - priority = 1 - else: + if dir_include is None or not is_reversed: + dir_include = include + dir_index = index + elif file_include is None or not is_reversed: # Pattern matched by a file pattern. - priority = 2 - - if is_reversed: - if priority > out_priority: - out_include = include - out_index = index - out_priority = priority - else: - # Forward. - if (include and dir_mark) or priority >= out_priority: - out_include = include - out_index = index - out_priority = priority - - if is_reversed and priority == 2: - # Patterns are being checked in reverse order. The first pattern that - # matches with priority 2 takes precedence. - break - - return (out_include, out_index) + file_include = include + file_index = index + + if dir_include: + return (dir_include, dir_index) + elif file_include is not None: + return (file_include, file_index) + else: + return (dir_include, dir_index) diff --git a/tests/test_06_gitignore.py b/tests/test_06_gitignore.py index 04c5194..eb39293 100644 --- a/tests/test_06_gitignore.py +++ b/tests/test_06_gitignore.py @@ -731,3 +731,79 @@ 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 issue 129, a file negation under an excluded directory. + """ + for sub_test in self.parameterize_from_lines([ + "build", + "!keep.log", + ]): + with sub_test() as spec: + # Confirmed results with git (v2.54.0). + files = { + "build/keep.log", # 1:build + "keep.log", # -:!keep.log + } + results = list(spec.check_files(files)) + ignores = get_includes(results) + debug = debug_results(spec, results) + self.assertEqual(ignores, { + "build/keep.log", + }, debug) + self.assertEqual(files - ignores, { + "keep.log", + }, debug) + + def test_10_issue_129_b(self): + """ + Test issue 129, a file negation naming the excluded directory, and a + grandparent. + """ + for sub_test in self.parameterize_from_lines([ + "build", + "!build/keep.log", + "a", + "!a/b/keep.log", + ]): + with sub_test() as spec: + # Confirmed results with git (v2.54.0). + files = { + "build/keep.log", # 1:build + "a/b/keep.log", # 3:a + } + results = list(spec.check_files(files)) + ignores = get_includes(results) + debug = debug_results(spec, results) + self.assertEqual(ignores, files, debug) + + def test_10_issue_129_c(self): + """ + Test issue 129, re-inclusion that must keep working: the directory itself is + not excluded, or its exclusion is undone before the file negation. + """ + for sub_test in self.parameterize_from_lines([ + "build/*", + "!build/keep.log", + "log", + "!log/", + "!log/keep.log", + ]): + with sub_test() as spec: + # Confirmed results with git (v2.54.0). + files = { + "build/keep.log", # -:!build/keep.log + "build/drop.log", # 1:build/* + "log/keep.log", # -:!log/keep.log + } + results = list(spec.check_files(files)) + ignores = get_includes(results) + debug = debug_results(spec, results) + self.assertEqual(ignores, { + "build/drop.log", + }, debug) + self.assertEqual(files - ignores, { + "build/keep.log", + "log/keep.log", + }, debug)