From 32cbf1f83ef913725fa7eb309822990a96d7fd23 Mon Sep 17 00:00:00 2001 From: ayushcodes10 Date: Sat, 12 Sep 2026 13:29:46 +0530 Subject: [PATCH] Add a shared Windows replace fallback helper os.replace can fail on Windows even when the temp file and the destination are in the same directory on the same drive: WinError 17, "cannot move to a different disk drive", maps to a plain OSError in Python rather than PermissionError, so the existing copy then delete fallback for WinError 5 and 32 never caught it. Adds os_replace_with_fallback, checking both PermissionError and winerror 17, and points the shared atomic writer at it instead of its own inline PermissionError only version. Toward #3508. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh --- graphify/paths.py | 31 ++++++++++++++++++++++--------- tests/test_atomic_writes.py | 25 +++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/graphify/paths.py b/graphify/paths.py index a700ad171..adfb779cc 100644 --- a/graphify/paths.py +++ b/graphify/paths.py @@ -26,6 +26,27 @@ from pathlib import Path, PurePosixPath, PureWindowsPath GRAPHIFY_OUT = os.environ.get("GRAPHIFY_OUT", "graphify-out") +def os_replace_with_fallback(src: "str | Path", dst: "str | Path") -> None: + """``os.replace(src, dst)``, falling back to copy-then-delete for a known + set of Windows quirks (#3508) that raise even when ``src``/``dst`` are the + same directory on the same drive: ``PermissionError`` (WinError 5/32 -- + destination briefly locked by another handle, antivirus, an open reader) + and WinError 17 ("cannot move to a different disk drive", observed on some + Windows/filesystem combinations despite textbook same-volume semantics). + WinError 17 maps to a plain ``OSError`` in Python, not ``PermissionError``, + so it's checked via ``winerror`` rather than the exception type. Any other + failure is a real one and is re-raised. + """ + try: + os.replace(src, dst) + except OSError as exc: + if not isinstance(exc, PermissionError) and getattr(exc, "winerror", None) != 17: + raise + import shutil + shutil.copy2(src, dst) + os.unlink(src) + + def _atomic_replace(path: "str | Path", write_fn) -> None: """Atomically replace ``path`` with content written by ``write_fn(f)``. @@ -63,15 +84,7 @@ def _atomic_replace(path: "str | Path", write_fn) -> None: os.chmod(tmp, mode) except OSError: pass - try: - os.replace(tmp, str(real)) - except PermissionError: - # Windows: os.replace fails (WinError 5/32) when the destination is - # briefly locked by another handle (antivirus, an open reader). Fall - # back to copy-then-delete, matching graphify.cache's atomic writer. - import shutil - shutil.copy2(tmp, str(real)) - os.unlink(tmp) + os_replace_with_fallback(tmp, str(real)) except BaseException: try: os.unlink(tmp) diff --git a/tests/test_atomic_writes.py b/tests/test_atomic_writes.py index 542c7c1c0..21dc2f9fd 100644 --- a/tests/test_atomic_writes.py +++ b/tests/test_atomic_writes.py @@ -167,6 +167,31 @@ def test_write_text_atomic_windows_permission_fallback(tmp_path, monkeypatch): assert sorted(x.name for x in tmp_path.iterdir()) == ["graph.json"] +def test_write_text_atomic_windows_winerror_17_fallback(tmp_path, monkeypatch): + """#3508: `os.replace` can raise WinError 17 ("cannot move to a different + disk drive") even when src/dst are the same directory on the same drive, + on some Windows/filesystem combinations. Unlike WinError 5/32, this is a + plain OSError rather than PermissionError, so the fallback must key off + winerror rather than the exception type alone.""" + p = tmp_path / "graph.json" + p.write_text("original", encoding="utf-8") + + calls = {"n": 0} + + def flaky_replace(src, dst): + calls["n"] += 1 + exc = OSError("cannot move to a different disk drive") + exc.winerror = 17 + raise exc + + monkeypatch.setattr(os, "replace", flaky_replace) + write_text_atomic(p, "new-content") + + assert calls["n"] == 1 + assert p.read_text() == "new-content" + assert sorted(x.name for x in tmp_path.iterdir()) == ["graph.json"] + + def test_write_json_atomic_ensure_ascii_false_preserves_utf8(tmp_path): from graphify.paths import write_json_atomic