optimize dvcignore

dvc/ignore.py

@@ -60,13 +60,16 @@ class DvcIgnorePatterns(DvcIgnore):
         self.pattern_list: list[PatternInfo] = []
         self.dirname = dirname
         self.find_matching_pattern = functools.cache(self._find_matching_pattern)
+        self._ignore_parents = functools.cache(self._ignore_parents)
 
         regex_pattern_list: list[tuple[str, bool, bool, PatternInfo]] = []
-        for count, pattern_info in enumerate(pattern_infos):
+        for pattern_info in pattern_infos:
             regex, ignore = GitIgnoreSpecPattern.pattern_to_regex(pattern_info.patterns)
             if regex is not None and ignore is not None:
                 self.pattern_list.append(pattern_info)
-                regex = regex.replace(f"<{_DIR_MARK}>", f"<{_DIR_MARK}{count}>")
+                # DVC resolves directory matches itself; only the outer rule
+                # group is needed to identify a match in the combined regex.
+                regex = regex.replace(f"(?P<{_DIR_MARK}>", "(?:")
                 regex_pattern_list.append(
                     (regex, ignore, pattern_info.patterns.endswith("/"), pattern_info)
                 )
@@ -126,8 +129,20 @@ class DvcIgnorePatterns(DvcIgnore):
     def __call__(
         self, root: str, dirs: list[str], files: list[str]
     ) -> tuple[list[str], list[str]]:
-        files = [f for f in files if not self.matches(root, f)]
-        dirs = [d for d in dirs if not self.matches(root, d, True)]
+        # Entries are basenames from one directory listing, so normalize the
+        # shared prefix and check its ancestors only once.
+        prefix = self._get_normalize_path(root, "")
+        if prefix is None:
+            return dirs.copy(), files.copy()
+        if prefix and self._ignore_parents(prefix[:-1])[0]:
+            return [], []
+
+        files = [
+            f for f in files if not self.find_matching_pattern(f"{prefix}{f}", False)[0]
+        ]
+        dirs = [
+            d for d in dirs if not self.find_matching_pattern(f"{prefix}{d}", True)[0]
+        ]
 
         return dirs, files
 
@@ -189,55 +204,51 @@ class DvcIgnorePatterns(DvcIgnore):
         _match: list[PatternInfo] = []
         if path:
             result, _match = self._ignore(path, is_dir)
-        return (result, _match) if details else result
+        return (result, _match.copy()) if details else result
 
     def _find_matching_pattern(
         self, path: str, is_dir: bool
     ) -> tuple[bool, list[PatternInfo]]:
-        paths = [path]
-        if is_dir and not path.endswith("/"):
-            paths.append(f"{path}/")
-
         for pattern, ignore, dir_only_pattern, pattern_map in reversed(
             self.ignore_spec
         ):
             if dir_only_pattern and not is_dir:
                 continue
-            for p in paths:
-                match = pattern.match(p)
-                if not match:
-                    continue
-                if ignore:
-                    group_name, _match = next(
-                        (
-                            (name, _match)
-                            for name, _match in match.groupdict().items()
-                            if name.startswith("rule_") and _match is not None
-                        )
-                    )
-                else:
-                    # unignored patterns are not combined with `|`,
-                    # so there are no groups.
-                    group_name = None
-                _regex, pattern_info = pattern_map[group_name]
-                return ignore, [pattern_info]
+            match = pattern.match(path)
+            # Only directory-only patterns need the trailing slash. Probing it
+            # for other patterns would make X/** exclude X itself.
+            if not match and dir_only_pattern and not path.endswith("/"):
+                match = pattern.match(f"{path}/")
+            if not match:
+                continue
+            if ignore:
+                group_name = match.lastgroup
+            else:
+                # unignored patterns are not combined with `|`,
+                # so there are no groups.
+                group_name = None
+            _regex, pattern_info = pattern_map[group_name]
+            return ignore, [pattern_info]
         return False, []
 
     def _ignore(self, path: str, is_dir: bool) -> tuple[bool, list[PatternInfo]]:
+        parent, sep, _ = path.rpartition("/")
+        if sep:
+            result = self._ignore_parents(parent)
+            if result[0]:
+                return result
+        return self.find_matching_pattern(path, is_dir)
+
+    def _ignore_parents(self, path: str) -> tuple[bool, list[PatternInfo]]:
+        # Files in the same directory share the ancestor verdict, not just
+        # individual regex matches. Cache it to avoid rebuilding every prefix.
         parts = path.split("/")
-        result = False
-        matches: list[PatternInfo] = []
         for i in range(1, len(parts) + 1):
             rel_path = "/".join(parts[:i])
-            result, _matches = self.find_matching_pattern(
-                rel_path, is_dir or i < len(parts)
-            )
-            if i < len(parts) and not result:
-                continue
-            matches.extend(_matches)
+            result, matches = self.find_matching_pattern(rel_path, True)
             if result:
-                break
-        return result, matches
+                return result, matches
+        return False, []
 
     def __hash__(self) -> int:
         return hash(self.dirname + ":" + str(self.pattern_list))

tests/func/test_check_ignore.py

@@ -19,6 +19,36 @@ def test_check_ignore(tmp_dir, dvc, file, ret, output, caplog, capsys):
     assert "Having any troubles?" not in caplog.text
 
 
+@pytest.mark.parametrize(
+    "pattern, exception, ignored",
+    [
+        ("volumes/functions/**", "volumes/functions/deno.json", False),
+        ("volumes/functions/**", "volumes/functions/deno.json*", False),
+        ("volumes/functions/", "volumes/functions/deno.json", True),
+        ("ignore.txt", "no-ignore.txt", False),
+        ("*ignore.txt", "no-ignore.txt", False),
+    ],
+)
+def test_check_ignore_reinclude(tmp_dir, dvc, pattern, exception, ignored):
+    tmp_dir.gen(
+        {
+            ".dvcignore": f"{pattern}\n!{exception}\n",
+            "volumes": {"functions": {"deno.json": "", "other.py": ""}},
+            "ignore.txt": "",
+            "no-ignore.txt": "",
+        }
+    )
+    file = exception.rstrip("*").replace("/", os.sep)
+    sibling = (
+        os.path.join("volumes", "functions", "other.py")
+        if pattern.startswith("volumes/")
+        else "ignore.txt"
+    )
+
+    assert main(["check-ignore", "-q", file]) == (0 if ignored else 1)
+    assert main(["check-ignore", "-q", sibling]) == 0
+
+
 @pytest.mark.parametrize(
     "file,ret,output",
     [

tests/func/test_ignore.py

@@ -31,6 +31,50 @@ def test_ignore(tmp_dir, dvc, filename):
     }
 
 
+@pytest.mark.parametrize("suffix, ignored", [("/**", False), ("/", True), ("", True)])
+@pytest.mark.parametrize("exception", ["deno.json", "deno.json*"])
+def test_reinclude_directory_contents(tmp_dir, dvc, suffix, ignored, exception):
+    tmp_dir.gen(
+        {
+            ".dvcignore": (
+                f"volumes/functions{suffix}\n!volumes/functions/{exception}\n"
+            ),
+            "volumes": {"functions": {"deno.json": "", "other.py": ""}},
+        }
+    )
+    dvc._reset()
+
+    directory = tmp_dir / "volumes" / "functions"
+    assert dvc.dvcignore.is_ignored_dir(str(directory)) is ignored
+    assert dvc.dvcignore.is_ignored_file(str(directory / "deno.json")) is ignored
+    assert dvc.dvcignore.is_ignored_file(str(directory / "other.py"))
+
+    files = {
+        os.path.join(root, name)
+        for root, _, names in dvc.dvcignore.walk(dvc.fs, tmp_dir)
+        for name in names
+    }
+    expected = {str(tmp_dir / ".dvcignore")}
+    if not ignored:
+        expected.add(str(directory / "deno.json"))
+    assert files == expected
+
+
+@pytest.mark.parametrize("pattern", ["ignore.txt", "*ignore.txt"])
+def test_reinclude_root_file(tmp_dir, dvc, pattern):
+    tmp_dir.gen(
+        {
+            ".dvcignore": f"{pattern}\n!no-ignore.txt\n",
+            "ignore.txt": "",
+            "no-ignore.txt": "",
+        }
+    )
+    dvc._reset()
+
+    assert dvc.dvcignore.is_ignored_file(str(tmp_dir / "ignore.txt"))
+    assert not dvc.dvcignore.is_ignored_file(str(tmp_dir / "no-ignore.txt"))
+
+
 def test_walk(tmp_dir, dvc):
     tmp_dir.gen(
         {

tests/unit/test_ignore.py

@@ -4,6 +4,64 @@ from os.path import join
 import pytest
 
 from dvc.ignore import DvcIgnorePatterns
+from dvc.pathspec_math import PatternInfo
+
+
+def test_match_details_combined_patterns():
+    patterns = [
+        PatternInfo("*.tmp", ".dvcignore:1:*.tmp"),
+        PatternInfo("*.log", ".dvcignore:2:*.log"),
+        PatternInfo("build/", ".dvcignore:3:build/"),
+        PatternInfo("cache/", ".dvcignore:4:cache/"),
+        PatternInfo("!keep.log", ".dvcignore:5:!keep.log"),
+    ]
+    ignore = DvcIgnorePatterns(patterns, "/repo", "/")
+    for _ in range(2):
+        assert ignore.matches("/repo", "data/error.log", details=True) == (
+            True,
+            [patterns[1]],
+        )
+        assert ignore.matches("/repo", "cache/keep.log", details=True) == (
+            True,
+            [patterns[3]],
+        )
+        assert ignore.matches("/repo", "data/keep.log", details=True) == (
+            False,
+            [patterns[4]],
+        )
+
+
+@pytest.mark.parametrize("path", ["data/file.tmp", "build/file.txt"])
+def test_match_details_mutation(path):
+    ignore = DvcIgnorePatterns(["*.tmp", "build/"], "/repo", "/")
+    result, details = ignore.matches("/repo", path, details=True)
+    expected = details.copy()
+    details.clear()
+
+    assert result
+    assert expected
+    assert ignore.matches("/repo", path, details=True) == (True, expected)
+
+
+@pytest.mark.parametrize(
+    "root, expected_dirs, expected_files",
+    [
+        ("/repo", ["child"], ["keep.txt", "logs"]),
+        ("/repo/dir", [], ["keep.txt"]),
+        ("/repo/ignored/dir", [], []),
+        ("/repo2", ["logs", "ignored", "child"], ["keep.txt", "other.tmp", "logs"]),
+    ],
+)
+def test_filter_directory_listing(root, expected_dirs, expected_files):
+    ignore = DvcIgnorePatterns(
+        ["ignored/", "dir/**", "!dir/keep.txt", "logs/", "*.tmp"], "/repo", "/"
+    )
+    dirs = ["logs", "ignored", "child"]
+    files = ["keep.txt", "other.tmp", "logs"]
+    for _ in range(2):
+        assert ignore(root, dirs, files) == (expected_dirs, expected_files)
+    assert dirs == ["logs", "ignored", "child"]
+    assert files == ["keep.txt", "other.tmp", "logs"]
 
 
 @pytest.mark.parametrize(