mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-25 08:11:54 +00:00
fix(cache): read and write AST cache entries via extended-length paths on Windows
Under a deep root, the cache entry
<root>/graphify-out/cache/ast/v<ver>-s<schema>-g<grammars>/<64 hex>.json,
and mkstemp's even longer <hash>.<random>.tmp temp name, overrun Windows
MAX_PATH. save_cached raised FileNotFoundError and `graphify extract`
failed with "AST extraction failed" (e.g. any project under the default
%TEMP% of a long user name).
Move detect._os_path (the \?\ helper from #1655) to paths.os_long_path
so cache.py can share it, and use it for the entry and temp file in
save_cached and load_cached. detect._os_path stays as an alias. Keys are
unaffected: _normalize_path still strips the prefix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit abf9c9f6ab)
This commit is contained in:
1 parent
cbcd8dd601
commit
73876cbdc7
4 files changed
+75
-31
No files matched your search
+8
-4
@@ -17,6 +17,7 @@ from pathlib import Path
|
||||
# absolute path ("/shared/graphify-out"). Single source of truth in graphify.paths
|
||||
# (#1423); re-exported here as _GRAPHIFY_OUT for the existing call sites.
|
||||
from graphify.paths import GRAPHIFY_OUT as _GRAPHIFY_OUT
|
||||
from graphify.paths import os_long_path as _os_long_path
|
||||
from graphify.paths import os_replace_with_fallback as _os_replace_with_fallback
|
||||
|
||||
# AST cache entries are the output of graphify's own extractor code, so they
|
||||
@@ -1099,10 +1100,11 @@ def load_cached(path: Path, root: Path = Path("."), kind: str = "ast",
|
||||
return None
|
||||
h = _salted_key(h, salt)
|
||||
prompt_fp = _resolve_prompt_fp(prompt, prompt_file)
|
||||
entry = cache_dir(location, kind, prompt_fp) / f"{h}.json"
|
||||
# Extended-length form so a deep root does not push the entry past MAX_PATH.
|
||||
entry = Path(_os_long_path(cache_dir(location, kind, prompt_fp) / f"{h}.json"))
|
||||
legacy_hit = False
|
||||
if prompt_fp and not entry.exists() and allow_legacy:
|
||||
legacy = cache_dir(location, kind) / f"{h}.json"
|
||||
legacy = Path(_os_long_path(cache_dir(location, kind) / f"{h}.json"))
|
||||
if legacy.exists():
|
||||
entry, legacy_hit = legacy, True
|
||||
if entry.exists():
|
||||
@@ -1263,8 +1265,10 @@ def save_cached(path: Path, result: dict, root: Path = Path("."), kind: str = "a
|
||||
_relativize_ids_in(on_disk, p, root)
|
||||
h = _salted_key(file_hash(p, root, cache_root=cache_root), salt)
|
||||
location = cache_root if cache_root is not None else root
|
||||
target_dir = cache_dir(location, kind, _resolve_prompt_fp(prompt, prompt_file))
|
||||
entry = target_dir / f"{h}.json"
|
||||
# Extended-length form: under a deep root the entry, and its longer temp
|
||||
# name, overrun Windows MAX_PATH and the write fails the whole extraction.
|
||||
target_dir = _os_long_path(cache_dir(location, kind, _resolve_prompt_fp(prompt, prompt_file)))
|
||||
entry = os.path.join(target_dir, f"{h}.json")
|
||||
fd, tmp_path = tempfile.mkstemp(dir=target_dir, prefix=f"{h}.", suffix=".tmp")
|
||||
try:
|
||||
os.write(fd, json.dumps(on_disk).encode())
|
||||
|
||||
+3
-27
@@ -22,7 +22,7 @@ from graphify.google_workspace import (
|
||||
convert_google_workspace_file,
|
||||
google_workspace_enabled,
|
||||
)
|
||||
from graphify.paths import GRAPHIFY_OUT, GRAPHIFY_OUT_NAME, out_path
|
||||
from graphify.paths import GRAPHIFY_OUT, GRAPHIFY_OUT_NAME, os_long_path, out_path
|
||||
from graphify.rcfile import activate_language_overrides, cache_salt, effective_suffix
|
||||
|
||||
|
||||
@@ -2273,32 +2273,8 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
|
||||
}
|
||||
|
||||
|
||||
def _os_path(path: Path) -> str:
|
||||
r"""Return an OS path string safe for open()/stat() on Windows long paths.
|
||||
|
||||
On win32, paths longer than the legacy MAX_PATH (260 chars) are rejected by
|
||||
the plain file APIs unless prefixed with the extended-length marker ``\\?\``
|
||||
(which also requires a fully-qualified path). Without it, _md5_file /
|
||||
save_manifest / count_words silently fail to hash deeply-nested files, so
|
||||
their manifest entry never stabilizes and detect_incremental re-flags them
|
||||
as changed on every run (#1655). cache._normalize_path strips this prefix
|
||||
for stable KEYS; this adds it for I/O. Non-win32 and already-prefixed paths
|
||||
pass through unchanged.
|
||||
"""
|
||||
import sys
|
||||
if sys.platform != "win32":
|
||||
return str(path)
|
||||
s = str(path)
|
||||
if s.startswith("\\\\?\\"):
|
||||
return s
|
||||
try:
|
||||
s = os.path.abspath(s) # \\?\ requires a fully-qualified path
|
||||
except Exception:
|
||||
return str(path)
|
||||
if s.startswith("\\\\"):
|
||||
# UNC share \\server\share -> \\?\UNC\server\share
|
||||
return "\\\\?\\UNC\\" + s[2:]
|
||||
return "\\\\?\\" + s
|
||||
# Shared with cache.py (paths.os_long_path); the name stays for detect's callers.
|
||||
_os_path = os_long_path
|
||||
|
||||
|
||||
def _md5_file(path: Path) -> str:
|
||||
|
||||
@@ -53,6 +53,34 @@ def _out_from_env() -> str:
|
||||
GRAPHIFY_OUT = _out_from_env()
|
||||
|
||||
|
||||
def os_long_path(path: "str | Path") -> str:
|
||||
r"""Return an OS path string safe for open()/stat() on Windows long paths.
|
||||
|
||||
On win32, paths longer than the legacy MAX_PATH (260 chars) are rejected by
|
||||
the plain file APIs unless prefixed with the extended-length marker ``\\?\``
|
||||
(which also requires a fully-qualified path). Without it, _md5_file /
|
||||
save_manifest / count_words silently fail to hash deeply-nested files, so
|
||||
their manifest entry never stabilizes and detect_incremental re-flags them
|
||||
as changed on every run (#1655), and cache entries under a deep root cannot
|
||||
be written at all. cache._normalize_path strips this prefix for stable
|
||||
KEYS; this adds it for I/O. Non-win32 and already-prefixed paths pass
|
||||
through unchanged.
|
||||
"""
|
||||
if sys.platform != "win32":
|
||||
return str(path)
|
||||
s = str(path)
|
||||
if s.startswith("\\\\?\\"):
|
||||
return s
|
||||
try:
|
||||
s = os.path.abspath(s) # \\?\ requires a fully-qualified path
|
||||
except Exception:
|
||||
return str(path)
|
||||
if s.startswith("\\\\"):
|
||||
# UNC share \\server\share -> \\?\UNC\server\share
|
||||
return "\\\\?\\UNC\\" + s[2:]
|
||||
return "\\\\?\\" + s
|
||||
|
||||
|
||||
def os_replace_with_fallback(src: "str | Path", dst: "str | Path") -> None:
|
||||
"""``os.replace(src, dst)``, falling back to a copy for a known set of
|
||||
Windows quirks (#3508) that raise even when ``src``/``dst`` are the same
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
"""A cache entry whose path exceeds Windows MAX_PATH must still save and load.
|
||||
|
||||
Under a deep root (a nested temp dir, OneDrive, a long user name) the AST cache
|
||||
path — <root>/graphify-out/cache/ast/v<ver>-s<schema>-g<grammars>/<64 hex>.json,
|
||||
plus mkstemp's longer ``<hash>.<random>.tmp`` temp name — overruns 260 chars.
|
||||
The write raised FileNotFoundError and ``graphify extract`` failed outright.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
|
||||
import pytest
|
||||
|
||||
from graphify.cache import cache_dir, load_cached, save_cached
|
||||
from graphify.paths import _WINDOWS_MAX_PATH
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name != "nt", reason="Windows MAX_PATH semantics")
|
||||
def test_cache_entry_round_trips_past_windows_max_path(tmp_path):
|
||||
src = tmp_path / "a.py"
|
||||
src.write_text("def f():\n return 1\n", encoding="utf-8")
|
||||
|
||||
# Pad the cache root so the cache DIRECTORY stays creatable (< 248) while
|
||||
# the entry file inside it (+ "\<64 hex>.json") lands past MAX_PATH.
|
||||
overhead = len(str(cache_dir(tmp_path / "p", "ast"))) - len(str(tmp_path / "p"))
|
||||
deep = tmp_path / ("d" * (230 - overhead - len(str(tmp_path)) - 1))
|
||||
entry_len = len(str(cache_dir(deep, "ast"))) + 1 + 64 + len(".json")
|
||||
assert entry_len > _WINDOWS_MAX_PATH - 1, entry_len
|
||||
|
||||
result = {"nodes": [{"id": "f", "label": "f", "source_file": str(src)}], "edges": []}
|
||||
save_cached(src, result, root=tmp_path, cache_root=deep)
|
||||
|
||||
cached = load_cached(src, root=tmp_path, cache_root=deep)
|
||||
assert cached is not None
|
||||
assert [n["label"] for n in cached["nodes"]] == ["f"]
|
||||
assert not list(cache_dir(deep, "ast").glob("*.tmp"))
|
||||
Reference in new issue
Block a user