Skip to content

fix: skip directory fsync for Windows moves - #170

Merged
Paul-Kyle merged 1 commit into
phasespace-labs:mainfrom
TLNing260310:fix/168-windows-directory-fsync
Aug 31, 2026
Merged

fix: skip directory fsync for Windows moves#170
Paul-Kyle merged 1 commit into
phasespace-labs:mainfrom
TLNing260310:fix/168-windows-directory-fsync

Conversation

@TLNing260310

Copy link
Copy Markdown
Contributor

Summary

  • skip destination-directory fsync for memory-file moves on native Windows, matching the existing atomic-write fallback
  • document the weaker crash-metadata durability on Windows
  • add a regression test that verifies the file is moved without calling the unsupported directory fsync
  • record the fix in the unreleased changelog

Fixes #168

Testing

  • pytest tests/test_git_tools.py -q (11 passed)
  • related git-tools and consolidation tests (97 passed)
  • ruff check palinode/ tests/ scripts/
  • bandit -r palinode/ -ll
  • git diff --check

@Paul-Kyle

Copy link
Copy Markdown
Member

This is complete, and the docstring is the reason I'm saying so. The old text claimed the destination directory is fsynced so the rename survives a crash — true before your change and false after it, and the easiest thing in the world to leave alone. You rewrote it to say where the fsync happens and what Windows gives up instead, and put the same note at the call site. A guarded call with a stale docstring above it would have been a worse bug than the one you fixed, because the next reader would trust it.

The test also goes past its template: the write-side sibling only asserts the fsync was skipped, and yours checks the file actually moved and kept its contents.

Merging. Thanks for a clean first contribution.

@Paul-Kyle
Paul-Kyle merged commit 0574bc0 into phasespace-labs:main Aug 31, 2026
9 checks passed
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.

fix(git): move_memory_file fsyncs the directory unguarded, so it fails on native Windows

2 participants