Stop the excluded directory rule from firing on a re-included ancestor

CHANGES.rst

@@ -27,6 +27,7 @@ Bug fixes:
 - `Pull #135`_: Escape trailing spaces in `GitIgnoreSpecPattern.escape()`.
 - `Issue #137`_ / `Pull #138`_: Patterns ending in `/**` no longer match their bare parent directory, preserving traversal to re-included children.
 - `Pull #139`_: Match newline characters in paths with `*` and `**`.
+- `Issue #137`_: The excluded directory rule no longer applies to an ancestor directory which the spec itself re-includes.
 
 
 .. _`Issue #116`: https://github.com/cpburnz/python-pathspec/issues/116

pathspec/_backends/hyperscan/gitignore.py

@@ -131,14 +131,17 @@ def _init_db(
 					# Found directory marker.
 					if regex_str.endswith(_DIR_MARK_OPT):
 						# Regex has optional directory marker. Split regex into directory
-						# and file variants.
+						# and file variants. The directory variant must require a child
+						# segment: without one the query *is* that directory, which is a
+						# direct match and not an excluded ancestor.
 						base_regex = regex_str[:-len(_DIR_MARK_OPT)]
-						use_regexes.append((f'{base_regex}/', True))
-						use_regexes.append((f'{base_regex}$', False))
+						use_regexes.append((f'{base_regex}/(?s:.)', True))
+						use_regexes.append((f'{base_regex}/?$', False))
 					else:
 						# Remove capture group.
 						base_regex = regex_str.replace(_DIR_MARK_CG, '/')
-						use_regexes.append((base_regex, True))
+						use_regexes.append((f'{base_regex}(?s:.)', True))
+						use_regexes.append((f'{base_regex}$', False))
 
 			if not use_regexes:
 				# No special case for regex.
@@ -205,15 +208,35 @@ def match_file(self, file: str) -> tuple[Optional[bool], Optional[int]]:
 		db.scan(file.encode('utf8'), match_event_handler=self.__on_match)
 
 		dir_include, dir_index, file_include, file_index = self._out
-		if dir_include:
+		if dir_include and self._ancestor_excluded(file):
 			out_include, out_index = dir_include, dir_index
 		elif 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
+			# re-includes that ancestor, so the rule does not apply.
+			out_include, out_index = None, -1
 		else:
 			out_include, out_index = dir_include, dir_index
 
 		return (out_include, out_index if out_index != -1 else None)
 
+	def _ancestor_excluded(self, file: str) -> bool:
+		"""
+		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).
+		"""
+		index = file.find('/')
+		while index != -1 and index + 1 < len(file):
+			ancestor_include, _ancestor_index = self.match_file(file[:index + 1])
+			if ancestor_include:
+				return True
+			index = file.find('/', index + 1)
+
+		return False
+
+
 	@override
 	def __on_match(
 		self,

pathspec/_backends/re2/gitignore.py

@@ -97,14 +97,17 @@ def _init_set(
 					# Found directory marker.
 					if regex_str.endswith(_DIR_MARK_OPT):
 						# Regex has optional directory marker. Split regex into directory
-						# and file variants.
+						# and file variants. The directory variant must require a child
+						# segment: without one the query *is* that directory, which is a
+						# direct match and not an excluded ancestor.
 						base_regex = regex_str[:-len(_DIR_MARK_OPT)]
-						use_regexes.append((f'{base_regex}/', True))
-						use_regexes.append((f'{base_regex}$', False))
+						use_regexes.append((f'{base_regex}/(?s:.)', True))
+						use_regexes.append((f'{base_regex}/?$', False))
 					else:
 						# Remove capture group.
 						base_regex = regex_str.replace(_DIR_MARK_CG, '/')
-						use_regexes.append((base_regex, True))
+						use_regexes.append((f'{base_regex}(?s:.)', True))
+						use_regexes.append((f'{base_regex}$', False))
 
 			if not use_regexes:
 				# No special case for regex.
@@ -173,9 +176,29 @@ def match_file(self, file: str) -> tuple[Optional[bool], Optional[int]]:
 				file_index = index
 
 		assert dir_index != -1 or file_index != -1, (dir_index, file_index)
-		if dir_include:
+		if dir_include and self._ancestor_excluded(file):
 			return (dir_include, dir_index)
 		elif 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
+			# re-includes that ancestor, so the rule does not apply.
+			return (None, None)
 		else:
 			return (dir_include, dir_index)
+
+	def _ancestor_excluded(self, file: str) -> bool:
+		"""
+		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).
+		"""
+		index = file.find('/')
+		while index != -1 and index + 1 < len(file):
+			ancestor_include, _ancestor_index = self.match_file(file[:index + 1])
+			if ancestor_include:
+				return True
+			index = file.find('/', index + 1)
+
+		return False
+

pathspec/_backends/simple/gitignore.py

@@ -78,18 +78,54 @@ def match_file(self, file: str) -> tuple[Optional[bool], Optional[int]]:
 			):
 				# Pattern matched.
 				if match.match.groupdict().get(_DIR_MARK):
-					# Pattern matched by a directory pattern.
-					if dir_include is None or not is_reversed:
+					# A pattern can match both a strict ancestor of the file and the
+					# file itself, and the engine only ever hands back the leftmost
+					# match. Ask for every directory separator it can match.
+					is_ancestor = is_self = False
+					assert pattern.regex is not None, pattern
+					for dir_match in pattern.regex.finditer(file):
+						if dir_match.groupdict().get(_DIR_MARK) is None:
+							continue
+						elif dir_match.end(_DIR_MARK) < len(file):
+							is_ancestor = True
+						else:
+							is_self = True
+
+					if is_ancestor and (dir_include is None or not is_reversed):
 						dir_include = include
 						dir_index = index
+
+					if is_self and (file_include is None or not is_reversed):
+						file_include = include
+						file_index = index
 				elif file_include is None or not is_reversed:
 					# Pattern matched by a file pattern.
 					file_include = include
 					file_index = index
 
-		if dir_include:
+		if dir_include and self._ancestor_excluded(file):
 			return (dir_include, dir_index)
 		elif 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
+			# re-includes that ancestor, so the rule does not apply.
+			return (None, None)
 		else:
 			return (dir_include, dir_index)
+
+	def _ancestor_excluded(self, file: str) -> bool:
+		"""
+		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).
+		"""
+		index = file.find('/')
+		while index != -1 and index + 1 < len(file):
+			ancestor_include, _ancestor_index = self.match_file(file[:index + 1])
+			if ancestor_include:
+				return True
+			index = file.find('/', index + 1)
+
+		return False
+

tests/test_06_gitignore.py

@@ -955,3 +955,35 @@ def test_13_issue_139(self):
 			for sub_test in self.parameterize_from_lines([pattern]):
 				with sub_test() as spec:
 					self.assertTrue(spec.match_file(path))
+
+	def test_14_issue_137_b(self):
+		"""
+		Test that the excluded ancestor rule does not fire on an ancestor which
+		the spec itself re-includes.
+		"""
+		for sub_test in self.parameterize_from_lines([
+			".*",
+			"!**/node_modules/**",
+		]):
+			with sub_test() as spec:
+				# Confirmed results with git (v2.55.0). Asked two ways which
+				# agree on every row: "check-ignore -v" (which also prints a
+				# path whose deciding pattern is a negation, so the pattern
+				# column is what answers), and the consequence of "git add -A",
+				# which stages exactly the files that are not ignored.
+				files = {
+					".hidden",                                      # 1:.*
+					"vendor/keep.txt",                              # -
+					"vendor/.cache/y.txt",                          # 1:.*
+					"vendor/deps/npm/node_modules/keep.txt",        # 2:!**/node_modules/**
+					"vendor/deps/npm/node_modules/.bin/x.txt",      # 2:!**/node_modules/**
+					"vendor/deps/npm/node_modules/.bin/.hide.txt",  # 2:!**/node_modules/**
+					"node_modules/.bin/z.txt",                      # 2:!**/node_modules/**
+				}
+				results = list(spec.check_files(files))
+				ignores = get_includes(results)
+				debug = debug_results(spec, results)
+				self.assertEqual(ignores, {
+					".hidden",
+					"vendor/.cache/y.txt",
+				}, debug)