From 7594d2f2ed888275ce33c013a921f35624dbeefc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Saugat=20Pachhai=20=28=E0=A4=B8=E0=A5=8C=E0=A4=97=E0=A4=BE?= =?UTF-8?q?=E0=A4=A4=29?= Date: Mon, 7 Sep 2026 21:58:33 +0545 Subject: [PATCH] optimize dvcignore --- dvc/ignore.py | 85 +++++++++++++++++++-------------- tests/func/test_check_ignore.py | 30 ++++++++++++ tests/func/test_ignore.py | 44 +++++++++++++++++ tests/unit/test_ignore.py | 58 ++++++++++++++++++++++ 4 files changed, 180 insertions(+), 37 deletions(-) diff --git a/dvc/ignore.py b/dvc/ignore.py index 7bd679e1b..2ba820aa0 100644 --- a/dvc/ignore.py +++ b/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)) diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index 2d02cd141..a43d08696 100644 --- a/tests/func/test_check_ignore.py +++ b/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", [ diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index 86b40fb48..1b6aae39f 100644 --- a/tests/func/test_ignore.py +++ b/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( { diff --git a/tests/unit/test_ignore.py b/tests/unit/test_ignore.py index 3dfa458cd..4745cfdae 100644 --- a/tests/unit/test_ignore.py +++ b/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(