Skip to content

Prevent Windows directory replacement from deleting destination files - #825

Open
Shubham-Padkonde wants to merge 1 commit into
google:mainfrom
Shubham-Padkonde:fix/windows-replace-directory
Open

Shubham-Padkonde wants to merge 1 commit into
google:mainfrom
Shubham-Padkonde:fix/windows-replace-directory

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown

Fixes #816.

On Windows, os.replace(directory, existing_file) succeeds and destroys the file's contents. _OsPathBackend.replace() now rejects this case with NotADirectoryError, matching the existing backend contract.

Check os.lstat()'s FILE_ATTRIBUTE_DIRECTORY on Windows when the destination entry exists. This also covers directory junctions and directory symlinks, including dangling links, without following the source. POSIX behavior remains delegated to os.replace, so replacing a file with a directory symlink continues to work. Directory moves to unused paths and ordinary file replacement are preserved.

Add a focused local-backend test module that can collect on Windows without the POSIX-only imports in backend_test.py (covered separately by #819).

Validation:

  • Windows 11 / Python 3.12: original code had 4 failures and 4 passes in the new cases; after the fix, 8 pass. Two real-symlink cases skip because this account lacks Windows symlink privileges. Windows directory-link attributes are also exercised with mocked lstat results.
  • Manually tested a real Windows directory junction: replacement raises and leaves the junction, target contents, and destination bytes intact.
  • Linux / Python 3.11: backend suites pass (26 tests, 154 subtests; 2 Windows-only skips), including real valid and dangling directory-symlink replacement.
  • Full Linux epath suite: 76 passed, 154 subtests passed, 2 skipped, 1 failure. The unchanged test_use_backend assumes TensorFlow is installed; the same failure reproduces on the base revision without TensorFlow.
  • Broader Windows path suites: 47 passed, 2 skipped, 3 failures, with the public-cloud test deselected. All three failures reproduce on the base revision (POSIX separator assumption, TensorFlow assumption, and existing path-flavour equality issue epath.Path equality depends on pathlib flavour, so a local path never equals a gs:// derived one on Windows #820).
  • New test module passes Pyink formatting; git diff --check passes. Unrelated formatting changes were discarded.

OpenAI Codex (GPT-6) assisted with investigation, implementation, tests, and review of the diff. No human review is claimed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

epath.Path.replace() silently destroys the destination file on Windows when the source is a directory

1 participant