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

CHANGES.rst

@@ -27,6 +27,7 @@ Bug fixes:
 - `Issue #134`_: GitIgnoreSpec: reverse and forward evaluation disagree.
 - `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.
+- `Issue #137`_: The excluded directory rule no longer applies to an ancestor directory which the spec itself re-includes.
 - `Pull #139`_: Match newline characters in paths with `*` and `**`.
 - `Pull #142`_: Fix trailing-space trimming after escaped backslashes.
 - `Issue #146`_: pattern_to_regex raises on three lines git accepts (bare '!', '! ', lone '\').

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.
@@ -193,6 +196,16 @@ def match_file(self, file: str) -> tuple[Optional[bool], Optional[int]]:
 		or :data:`None`), and the index of the last matched pattern (:class:`int` or
 		:data:`None`).
 		"""
+		return self._match(file, check_ancestors=True)
+
+	def _match(
+		self, file: str, check_ancestors: bool,
+	) -> tuple[Optional[bool], Optional[int]]:
+		"""
+		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.
+		"""
 		# NOTICE: According to benchmarking, a method callback is 13% faster than
 		# using a closure here.
 		db = self._db
@@ -205,15 +218,41 @@ 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 check_ancestors 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).
+		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
+		exponentially with depth.
+		"""
+		index = file.find('/')
+		while index != -1 and index + 1 < len(file):
+			ancestor_include, _ancestor_index = self._match(
+				file[:index + 1], check_ancestors=False,
+			)
+			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.
@@ -142,6 +145,16 @@ def match_file(self, file: str) -> tuple[Optional[bool], Optional[int]]:
 		or :data:`None`), and the index of the last matched pattern (:class:`int` or
 		:data:`None`).
 		"""
+		return self._match(file, check_ancestors=True)
+
+	def _match(
+		self, file: str, check_ancestors: bool,
+	) -> tuple[Optional[bool], Optional[int]]:
+		"""
+		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.
+		"""
 		# Find best match.
 		match_ids: Optional[list[int]] = self._set.Match(file)  # type: ignore[assignment]
 		if not match_ids:
@@ -173,9 +186,35 @@ 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 check_ancestors 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).
+		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
+		exponentially with depth.
+		"""
+		index = file.find('/')
+		while index != -1 and index + 1 < len(file):
+			ancestor_include, _ancestor_index = self._match(
+				file[:index + 1], check_ancestors=False,
+			)
+			if ancestor_include:
+				return True
+			index = file.find('/', index + 1)
+
+		return False
+

pathspec/_backends/simple/gitignore.py

@@ -62,6 +62,16 @@ def match_file(self, file: str) -> tuple[Optional[bool], Optional[int]]:
 		or :data:`None`), and the index of the last matched pattern (:class:`int` or
 		:data:`None`).
 		"""
+		return self._match(file, check_ancestors=True)
+
+	def _match(
+		self, file: str, check_ancestors: bool,
+	) -> tuple[Optional[bool], Optional[int]]:
+		"""
+		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.
+		"""
 		is_reversed = self._is_reversed
 
 		# Resolve the ancestor directory and the file separately: a file negation
@@ -78,18 +88,59 @@ 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. For anchored patterns there is only one match, but
+					# `**/` compiles to the unanchored `(?P<ps_d>/)`: on "a/b/" it
+					# matches at "a/" and at "b/", and `match` only returns the first.
+					is_ancestor = is_self = False
+					assert pattern.regex is not None, pattern
+					for dir_match in pattern.regex.finditer(file):
+						if 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 check_ancestors 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).
+		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
+		exponentially with depth.
+		"""
+		index = file.find('/')
+		while index != -1 and index + 1 < len(file):
+			ancestor_include, _ancestor_index = self._match(
+				file[:index + 1], check_ancestors=False,
+			)
+			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)