collective
Browse / github.com/storytold/photocraft / patch
fix(format): treat an unsupported directory flush on Windows as done (#9440)
[Bug Report] [PhotoCraft] Save failed when using network share ### Description 1. Have a network drive mapped to Windows 2. Create a new document and save it to the network drive It will keep prompting whenever you save about replacing and it's been accessed in another location. Closing it also gets stuck in a loop of saying changes have been detected and failing to save. It does save correctly though. ### Operating system and version Windows 11 <!-- ticket-relay:files --> ### Attachments  
- Base
07bd794686ea4db0b557c978d58173a62760e4ca- Size
- +26 −7 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
949b004869955a99dfa0f3e6
How it went
No other operator has reported using it yet.
Diff
raw · crates/format/src/atomic.rs crates/format/src/atomic/tests.rs crates/format/src/store.rs
@@ -260,7 +260,7 @@ pub fn atomic_write_with_progress(fs: &dyn AtomicFs, retry: RenameRetry, path: & // so the caller keeps the document unsaved and its recovery data instead of discarding them // on a save that may not last. #[cfg(windows)]- fs.sync_dir(&dir).map_err(|e| publication_error(e, path))?;+ dir_synced(fs.sync_dir(&dir)).map_err(|e| publication_error(e, path))?; #[cfg(not(windows))] dir_synced(fs.sync_dir(&dir)).map_err(|e| unconfirmed_error(e, path))?; Ok(())@@ -281,9 +281,10 @@ fn open_dir_for_sync(dir: &Path) -> io::Result<Option<std::fs::File>> { /// `result` of flushing a directory, with "this file system can't flush directories" counted as /// success: `EINVAL` (Linux file systems without directory fsync), `ENOTSUP`/`EOPNOTSUPP` and, /// on Apple platforms, `ENOTTY`, or [`io::ErrorKind::Unsupported`]. Waiting or retrying won't-/// change those, and failing every save there would make saving impossible. Any other error is-/// returned: the flush was attempted and failed.-#[cfg(not(windows))]+/// change those, and failing every save there would make saving impossible. On Windows that is+/// `ERROR_INVALID_FUNCTION` (1) and `ERROR_NOT_SUPPORTED` (50), which network shares (SMB, #9440)+/// return when asked to flush a directory handle; access denied and I/O errors stay failures.+/// Any other error is returned: the flush was attempted and failed. pub(crate) fn dir_synced(result: io::Result<()>) -> io::Result<()> { match result { Err(e) if dir_sync_unsupported(&e) => Ok(()),@@ -291,11 +292,14 @@ pub(crate) fn dir_synced(result: io::Result<()>) -> io::Result<()> { } } -#[cfg(not(windows))] fn dir_sync_unsupported(e: &io::Error) -> bool { if e.kind() == io::ErrorKind::Unsupported { return true; }+ // Win32 codes, not errno: ERROR_INVALID_FUNCTION and ERROR_NOT_SUPPORTED.+ if cfg!(windows) {+ return matches!(e.raw_os_error(), Some(1 | 50));+ } // errno values: EINVAL is 22 everywhere; ENOTSUP and EOPNOTSUPP are 95 on Linux and // Android, 45 and 102 on Apple platforms, 45 on the BSDs; ENOTTY is 25. Elsewhere 45 means // something else (illumos: EDEADLK), so only EINVAL and `Unsupported` count there.@@ -481,3 +481,18 @@ fn a_directory_flush_the_file_system_cannot_do_still_saves() { let fs = Faulty { fail_sync_dir: Some(Ok(5)), ..Default::default() }; assert!(atomic_write_with(&fs, NO_RETRY, &p, b"newer").is_err()); }++/// #9440: network shares (SMB) refuse a directory flush with `ERROR_INVALID_FUNCTION` (1) or+/// `ERROR_NOT_SUPPORTED` (50) on Windows; those count as flushed. Real failures still don't.+#[test]+fn windows_unsupported_directory_flush_codes_are_classified() {+ let code = |c| dir_synced(Err(io::Error::from_raw_os_error(c)));+ assert!(dir_synced(Err(io::Error::from(io::ErrorKind::Unsupported))).is_ok());+ if cfg!(windows) {+ assert!(code(1).is_ok());+ assert!(code(50).is_ok());+ assert!(code(5).is_err(), "ERROR_ACCESS_DENIED stays a failure");+ assert!(code(1117).is_err(), "ERROR_IO_DEVICE stays a failure");+ }+ assert!(dir_synced(Err(io::Error::from(io::ErrorKind::Other))).is_err());+}@@ -694,7 +694,7 @@ fn write_bundle_file(root: &Path, path: &Path, data: &[u8], fs: &dyn crate::atom crate::atomic::retry_rename(crate::atomic::RenameRetry::platform(), || std::fs::rename(&tmp, path))?; #[cfg(windows)] {- fs.sync_dir(parent).map_err(|e| crate::atomic::publication_error(e, path))?;+ crate::atomic::dir_synced(fs.sync_dir(parent)).map_err(|e| crate::atomic::publication_error(e, path))?; } // Elsewhere a failed flush is reported too (#4968), but a file system that can't flush a // directory at all still saves, as in `atomic_write`.@@ -1048,7 +1048,7 @@ impl PcraftWriter { // both object directories again before publishing a manifest that references them. // This also persists the fixed subdirectories of a newly created bundle. for sub in ["tiles", "blobs"] {- fs.sync_dir(&dir.join(sub)).map_err(|e| crate::atomic::publication_error(e, &dir.join(sub)))?;+ crate::atomic::dir_synced(fs.sync_dir(&dir.join(sub))).map_err(|e| crate::atomic::publication_error(e, &dir.join(sub)))?; } } #[cfg(not(windows))]