mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-13 23:49:14 +00:00
Use the replace fallback for AST cache writes
The AST extraction cache writes its own temp file and replace, a separate code path from the shared atomic writer, and only caught PermissionError. This was the exact write named in the report: worker failed for a file with WinError 17 while replacing a cache entry under the output directory's ast cache. Also covers the stat index write, which had no Windows fallback at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
This commit is contained in:
1 parent
32cbf1f83e
commit
7638d47565
2 files changed
+34
-9
No files matched your search
+3
-9
@@ -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_replace_with_fallback as _os_replace_with_fallback
|
||||
|
||||
# AST cache entries are the output of graphify's own extractor code, so they
|
||||
# are only valid for the version that wrote them: keying purely on file
|
||||
@@ -399,7 +400,7 @@ def _flush_stat_index() -> None:
|
||||
try:
|
||||
os.write(fd, json.dumps(on_disk, separators=(",", ":")).encode())
|
||||
os.close(fd)
|
||||
os.replace(tmp, p)
|
||||
_os_replace_with_fallback(tmp, p)
|
||||
except Exception:
|
||||
try:
|
||||
os.close(fd)
|
||||
@@ -1126,14 +1127,7 @@ def save_cached(path: Path, result: dict, root: Path = Path("."), kind: str = "a
|
||||
try:
|
||||
os.write(fd, json.dumps(on_disk).encode())
|
||||
os.close(fd)
|
||||
try:
|
||||
os.replace(tmp_path, entry)
|
||||
except PermissionError:
|
||||
# Windows: os.replace can fail with WinError 5 if the target is
|
||||
# briefly locked. Fall back to copy-then-delete.
|
||||
import shutil
|
||||
shutil.copy2(tmp_path, entry)
|
||||
os.unlink(tmp_path)
|
||||
_os_replace_with_fallback(tmp_path, entry)
|
||||
except Exception:
|
||||
try:
|
||||
os.close(fd)
|
||||
|
||||
@@ -221,6 +221,37 @@ def test_save_cached_relativizes_source_file(tmp_path):
|
||||
assert edge_sources == {"src/foo.py"}
|
||||
|
||||
|
||||
def test_save_cached_survives_a_winerror_17_replace(tmp_path, monkeypatch):
|
||||
"""#3508: on some Windows/filesystem combinations, `os.replace` raises
|
||||
WinError 17 ("cannot move to a different disk drive") even for a temp file
|
||||
and destination in the same directory on the same drive -- reported for
|
||||
exactly this AST cache write path. WinError 17 is a plain OSError, not a
|
||||
PermissionError, so it must still trigger the copy-then-delete fallback."""
|
||||
import os
|
||||
from graphify.cache import save_cached, file_hash, cache_dir
|
||||
|
||||
src = tmp_path / "foo.py"
|
||||
src.write_text("def x(): pass\n")
|
||||
|
||||
real_replace = os.replace
|
||||
|
||||
def flaky_replace(a, b):
|
||||
exc = OSError("cannot move to a different disk drive")
|
||||
exc.winerror = 17
|
||||
raise exc
|
||||
|
||||
monkeypatch.setattr(os, "replace", flaky_replace)
|
||||
try:
|
||||
save_cached(src, {"nodes": [], "edges": []}, root=tmp_path, kind="ast")
|
||||
finally:
|
||||
monkeypatch.setattr(os, "replace", real_replace)
|
||||
|
||||
h = file_hash(src, tmp_path)
|
||||
entry = cache_dir(tmp_path, "ast") / f"{h}.json"
|
||||
assert entry.exists()
|
||||
assert not any(p.name.endswith(".tmp") for p in entry.parent.iterdir())
|
||||
|
||||
|
||||
def test_load_cached_absolutizes_source_file(tmp_path):
|
||||
"""``load_cached`` returns the same absolute-path shape that a fresh
|
||||
extraction produces, so consumers don't need to special-case cache
|
||||
|
||||
Reference in new issue
Block a user