Library on an SMB share: the catalog files are written where macOS can't fully flush them (fixes #1407)

crates/catalog/src/store.rs

@@ -88,10 +88,14 @@ impl Store for FsStore {
             self.appender = None;
         }
         let tmp = self.path(&format!("{name}.tmp"));
-        {
+        let written = (|| {
             let mut f = std::fs::File::create(&tmp)?;
             f.write_all(data)?;
-            f.sync_all()?;
+            sync_file(&f, false)
+        })();
+        if let Err(e) = written {
+            let _ = std::fs::remove_file(&tmp);
+            return Err(e);
         }
         std::fs::rename(&tmp, self.path(name))?;
         self.sync_dir();
@@ -109,7 +113,7 @@ impl Store for FsStore {
             fill(&mut w)?;
             w.flush()?;
             let f = w.inner.into_inner().map_err(|e| e.into_error())?;
-            f.sync_all()?;
+            sync_file(&f, false)?;
             Ok(w.n)
         })();
         let n = match written {
@@ -135,7 +139,7 @@ impl Store for FsStore {
             self.appender = Some((name.to_string(), f));
         }
         let Some((_, f)) = self.appender.as_mut() else { return Err(io::Error::other("appender not open")) };
-        let written = f.write_all(data).and_then(|()| f.sync_data());
+        let written = f.write_all(data).and_then(|()| sync_file(f, true));
         if written.is_err() {
             // The handle may be dead (a network share that reconnected, a removed volume): the next
             // attempt reopens the file. Part of `data` may be in the file; the caller cuts it back
@@ -156,7 +160,7 @@ impl Store for FsStore {
             Err(e) => return Err(e),
         };
         f.set_len(len)?;
-        f.sync_all()
+        sync_file(&f, false)
     }
 
     fn describe(&self) -> String {
@@ -169,6 +173,64 @@ impl Store for FsStore {
     }
 }
 
+/// Flush a file to disk: `sync_all`, or `sync_data` when `data_only`.
+///
+/// On macOS both are `F_FULLFSYNC`, which an SMB share refuses with ENOTSUP (issue #1407): the
+/// volume has no such flush, so the write stands as the share keeps it instead of failing. Any
+/// other error still fails the write.
+fn sync_file(f: &std::fs::File, data_only: bool) -> io::Result<()> {
+    #[cfg(test)]
+    let r = if NO_FULL_SYNC.with(std::cell::Cell::get) {
+        Err(enotsup())
+    } else if data_only {
+        f.sync_data()
+    } else {
+        f.sync_all()
+    };
+    #[cfg(not(test))]
+    let r = if data_only { f.sync_data() } else { f.sync_all() };
+    match r {
+        Err(e) if flush_unsupported(&e) => Ok(()),
+        r => r,
+    }
+}
+
+/// The volume can't flush a file to disk (ENOTSUP on macOS, `Unsupported` elsewhere).
+fn flush_unsupported(e: &io::Error) -> bool {
+    const ENOTSUP: i32 = 45;
+    e.kind() == io::ErrorKind::Unsupported || (cfg!(target_vendor = "apple") && e.raw_os_error() == Some(ENOTSUP))
+}
+
+#[cfg(test)]
+thread_local! {
+    static NO_FULL_SYNC: std::cell::Cell<bool> = const { std::cell::Cell::new(false) };
+}
+
+/// What a volume without a full flush answers (macOS: ENOTSUP, os error 45, from `F_FULLFSYNC`).
+#[cfg(test)]
+fn enotsup() -> io::Error {
+    if cfg!(target_vendor = "apple") { io::Error::from_raw_os_error(45) } else { io::Error::from(io::ErrorKind::Unsupported) }
+}
+
+/// Tests: while the guard lives, every file sync [`FsStore`] makes on the calling thread fails
+/// as on an SMB share mounted by macOS (issue #1407).
+#[cfg(test)]
+pub(crate) fn full_sync_unsupported() -> NoFullSync {
+    NO_FULL_SYNC.with(|c| c.set(true));
+    NoFullSync(())
+}
+
+/// Ends a [`full_sync_unsupported`] when dropped.
+#[cfg(test)]
+pub(crate) struct NoFullSync(());
+
+#[cfg(test)]
+impl Drop for NoFullSync {
+    fn drop(&mut self) {
+        NO_FULL_SYNC.with(|c| c.set(false));
+    }
+}
+
 /// Counts the bytes written through it.
 struct Counter<W> {
     inner: W,

crates/catalog/src/tests_journal.rs

@@ -286,6 +286,35 @@ fn fs_store_roundtrip() {
     let _ = std::fs::remove_dir_all(&dir);
 }
 
+/// A library on an SMB share (issue #1407): macOS can't make the full flush there (`F_FULLFSYNC`
+/// fails with ENOTSUP), and the new library's snapshot, its log and `prefs.json` are still written.
+#[test]
+fn a_volume_without_a_full_flush_still_gets_its_files() {
+    let dir = std::env::temp_dir().join(format!("lc-journal-nofullsync-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    let c = {
+        let _smb = crate::store::full_sync_unsupported();
+        let (mut j, mut c, r) = Journal::open(Box::new(FsStore::open(&dir).unwrap())).unwrap();
+        assert!(r.created);
+        assert!(dir.join(SNAPSHOT).exists(), "the new library's snapshot is written");
+        let id = c.alloc_photo_id();
+        let add = Op::AddPhoto { photo: Box::new(Photo::new(id, Source::File { path: "/a.jpg".into() }, "a.jpg", "JPEG", 1, 1, "t")) };
+        c.apply(add.clone()).unwrap();
+        j.append(&[add]).unwrap();
+        j.snapshot(&c).unwrap();
+        let mut s = FsStore::open(&dir).unwrap();
+        s.write_atomic("prefs.json", b"{}").unwrap();
+        s.truncate(LOG, 0).unwrap();
+        c
+    };
+    let mut names: Vec<String> = std::fs::read_dir(&dir).unwrap().flatten().map(|e| e.file_name().to_string_lossy().to_string()).collect();
+    names.sort();
+    assert_eq!(names, vec![LOG, SNAPSHOT, "prefs.json"], "no temp file left behind");
+    let (_, c2, r) = Journal::open(Box::new(FsStore::open(&dir).unwrap())).unwrap();
+    assert_eq!((r.snapshot_seq, c2.to_snapshot()), (1, c.to_snapshot()));
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
 /// The streamed snapshot is byte-for-byte the format written before streaming (issue #37), so
 /// existing libraries load and older builds can read new snapshots; FsStore and MemStore agree.
 #[test]