diff --git a/graphify/detect.py b/graphify/detect.py index 4cb123104c..a97af4314d 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -1277,6 +1277,12 @@ def _matches(path_idx: int, pattern_idx: int) -> bool: return _matches(0, 0) +# Sentinels for the per-scan caches in _is_ignored. A str key cannot collide with the +# Path keys the ancestor memo uses. +_PREPARED_KEY = "__prepared_patterns__" +_UNSET = object() + + def _is_ignored( path: Path, root: Path, @@ -1320,14 +1326,46 @@ def _matches(rel: str, p: str, path_relative: bool) -> bool: return False result = False - for anchor, pattern in patterns: - negated = pattern.startswith("!") - raw = pattern[1:] if negated else pattern - directory_only = raw.endswith("/") - path_relative = "/" in raw.rstrip("/") - p = raw.strip("/") - if not p: - continue + + # Pattern parsing does not depend on the target, so do it once per scan and + # keep it in the shared cache. It used to run for every target: on a repo with + # ~55 patterns and ~14k targets that is ~770k redundant string splits. + # + # The entry is keyed on the pattern list's identity AND its length, and it + # keeps a reference to that list: + # - length, because detect() appends patterns from nested .gitignore files + # *during* the walk (see ignore_patterns.extend below), so a frozen parse + # would silently stop applying the later ones; + # - identity, because a caller may pass a different list of the same length + # with the same _cache, and length alone would serve the wrong parse. + # Holding the list keeps it alive, so an `is` check cannot be fooled by a new + # list landing on a freed one's address (which `id()` alone would allow). + prepared = None + if _cache is not None: + cached = _cache.get(_PREPARED_KEY) + if cached is not None and cached[0] is patterns and cached[1] == len(patterns): + prepared = cached[2] + if prepared is None: + prepared = [] + for _anchor, _pattern in patterns: + _negated = _pattern.startswith("!") + _raw = _pattern[1:] if _negated else _pattern + _p = _raw.strip("/") + if not _p: + continue + prepared.append((_anchor, _negated, _raw.endswith("/"), + "/" in _raw.rstrip("/"), _p)) + if _cache is not None: + _cache[_PREPARED_KEY] = (patterns, len(patterns), prepared) + + # relative_to() + _nfc() are the dominant cost in this loop and depend only on + # the anchor (or on root), never on the individual pattern. Anchors are few, + # patterns are many, so memoise per target. + rel_by_anchor: dict = {} + rel_root_cached = _UNSET + target_is_dir = None + + for anchor, negated, directory_only, path_relative, p in prepared: # gitignore semantics: patterns from A/.gitignore apply ONLY to paths # under A. Matching non-anchored patterns against root-relative paths @@ -1335,21 +1373,33 @@ def _matches(rel: str, p: str, path_relative: bool) -> bool: # (detect() returned 0 files). The anchor dir itself is exempt — an # ignore file governs its directory's contents, not the directory. matched = False - try: - rel_anchor = _nfc(str(target.relative_to(anchor)).replace(os.sep, "/")) - except ValueError: - continue # target outside this pattern's anchor: cannot match + if anchor in rel_by_anchor: + rel_anchor = rel_by_anchor[anchor] + else: + try: + rel_anchor = _nfc(str(target.relative_to(anchor)).replace(os.sep, "/")) + except ValueError: + rel_anchor = None # target outside this anchor: cannot match + rel_by_anchor[anchor] = rel_anchor + if rel_anchor is None: + continue if rel_anchor != ".": rel = rel_anchor - if not path_relative: - try: - if len(root.parts) > len(anchor.parts): - rel = _nfc(str(target.relative_to(root)).replace(os.sep, "/")) - except ValueError: - pass + if not path_relative and len(root.parts) > len(anchor.parts): + if rel_root_cached is _UNSET: + try: + rel_root_cached = _nfc( + str(target.relative_to(root)).replace(os.sep, "/")) + except ValueError: + rel_root_cached = None + if rel_root_cached is not None: + rel = rel_root_cached matched = _matches(rel, p, path_relative=path_relative) - if matched and directory_only and not target.is_dir(): - matched = False + if matched and directory_only: + if target_is_dir is None: + target_is_dir = target.is_dir() + if not target_is_dir: + matched = False if matched: result = not negated # last match wins; ! flips to un-ignore diff --git a/tests/test_detect.py b/tests/test_detect.py index 5be697ce72..24a76b666d 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -3066,3 +3066,56 @@ def test_sensitive_env_template_inside_secrets_dir_still_dropped(path): """Stage 1 dir guard runs before the Stage 2 template exemption: anything under a secrets/credentials dir stays excluded, template suffix or not.""" assert _is_sensitive(Path(path)), f"{path} is under a secrets dir, must stay excluded (#2184)" + +def test_nested_gitignore_applies_when_patterns_grow_mid_walk(tmp_path): + """Patterns from a nested .gitignore are appended while the walk is already + running, so any per-scan caching of the parsed pattern list must invalidate + when the list grows. + + The root .gitignore matters: it makes the parse cache fill up early, before + the walk reaches pkg/sub. Without a root pattern _is_ignored returns before + touching the cache and the bug cannot reproduce. + """ + (tmp_path / ".gitignore").write_text("*.log\n") + (tmp_path / "root.log").write_text("noise") + (tmp_path / "keep.py").write_text("x = 1") + + deep = tmp_path / "pkg" / "sub" + deep.mkdir(parents=True) + (deep / "kept.py").write_text("y = 2") + (deep / "skipped.py").write_text("z = 3") + (deep / ".gitignore").write_text("skipped.py\n") + + result = detect(tmp_path) + all_files = [f for files in result["files"].values() for f in files] + + assert not any("root.log" in f for f in all_files) # root pattern still applies + assert any("keep.py" in f for f in all_files) + assert any("kept.py" in f for f in all_files) + assert not any("skipped.py" in f for f in all_files) # nested pattern applies too + + +def test_prepared_pattern_cache_is_not_shared_across_pattern_lists(tmp_path): + """A shared _cache must not serve one pattern list's parse to another. + + The per-scan parse cache is keyed on the pattern list, not just on its + length: detect() only ever grows one list (so length alone suffices there), + but _is_ignored takes both `patterns` and `_cache` as arguments, so any + other caller can hand it two different lists of equal length. Keyed on + length alone, the second call silently matches against the first list's + patterns. + """ + (tmp_path / "a.log").write_text("noise") + (tmp_path / "b.tmp").write_text("noise") + (tmp_path / "c.log").write_text("noise") + + logs = [(tmp_path, "*.log")] + temps = [(tmp_path, "*.tmp")] + cache: dict = {} + + # First call fills the parse cache from `logs`. + assert _is_ignored(tmp_path / "a.log", tmp_path, logs, _cache=cache) is True + + # Same length, different contents: `temps` must be parsed on its own. + assert _is_ignored(tmp_path / "b.tmp", tmp_path, temps, _cache=cache) is True + assert _is_ignored(tmp_path / "c.log", tmp_path, temps, _cache=cache) is False