diff --git a/Cargo.lock b/Cargo.lock index 298258ca..bb5d87f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3978,6 +3978,7 @@ dependencies = [ "png 0.18.1", "pollster 1.0.1", "rfd", + "rustix 1.1.5", "serde", "serde_json", "skrifa 0.44.0", diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 8ee4e68e..5400829c 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -48,6 +48,9 @@ skrifa = "0.44" objc2 = "0.6" objc2-app-kit = { version = "0.3", default-features = false, features = ["std", "NSEvent", "NSScreen"] } objc2-foundation = { version = "0.3", default-features = false, features = ["std", "NSArray", "NSGeometry"] } +# A plain fsync where a volume refuses the full flush `sync_all` asks for (SMB, FUSE); already in +# the tree through pdfcraft-cli. +rustix = { version = "1", features = ["fs"] } [target.'cfg(windows)'.dependencies] # Auto interface language: the Windows display language, without starting a process. diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index b3b02a38..9b1b35b1 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -923,6 +923,17 @@ pub fn write_atomically(path: &str, bytes: &[u8]) -> std::io::Result<()> { /// [`write_atomically`], trying the staging names that `suffixes` give. fn write_atomically_with(path: &str, bytes: &[u8], suffixes: impl IntoIterator) -> std::io::Result<()> { + write_atomically_syncing(path, bytes, suffixes, std::fs::File::sync_all) +} + +/// [`write_atomically_with`], flushing the staging file with `sync_all` (the standard library's +/// in the app, a failing one in tests). +fn write_atomically_syncing( + path: &str, + bytes: &[u8], + suffixes: impl IntoIterator, + sync_all: impl FnOnce(&std::fs::File) -> std::io::Result<()>, +) -> std::io::Result<()> { use std::io::Write; let target = std::path::Path::new(path); let dir = target.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(std::path::Path::new(".")); @@ -933,7 +944,7 @@ fn write_atomically_with(path: &str, bytes: &[u8], suffixes: impl IntoIterator std::io::Result<()>) -> std::io::Result<()> { + #[cfg(target_os = "macos")] + { + use rustix::io::Errno; + match sync_all(f) { + Err(e) if matches!(Errno::from_io_error(&e), Some(Errno::NOTSUP | Errno::NOTTY)) => Ok(rustix::fs::fsync(f)?), + other => other, + } + } + #[cfg(not(target_os = "macos"))] + sync_all(f) +} + #[cfg(not(target_arch = "wasm32"))] fn short_name(path: &str) -> String { std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| path.to_string()) @@ -1028,7 +1056,7 @@ fn bookmark_at<'a>(items: &'a [pdfcraft_render::OutlineItem], path: &[usize]) -> #[cfg(test)] mod tests { - use super::{SaveTarget, is_mail_temp_copy, write_atomically, write_atomically_with}; + use super::{SaveTarget, is_mail_temp_copy, write_atomically, write_atomically_syncing, write_atomically_with}; use pdfcraft_platform::staging::{STAGING_ATTEMPTS, staging_suffixes}; use std::path::{Path, PathBuf}; @@ -1185,6 +1213,43 @@ mod tests { std::fs::set_permissions(&target, perms).unwrap(); let _ = std::fs::remove_dir_all(&dir); } + + /// On macOS `sync_all` asks for a full flush, which SMB and FUSE volumes (a Cryptomator vault, + /// #981) refuse as unsupported: ENOTSUP (45) or ENOTTY (25). The save still goes through. + #[cfg(target_os = "macos")] + #[test] + fn a_volume_without_full_flush_still_saves() { + for errno in [45, 25] { + let dir = staging_dir(&format!("nofullsync-{errno}")); + let target = dir.join("out.pdf"); + std::fs::write(&target, "OLD").unwrap(); + let asked = std::cell::Cell::new(false); + let refuse = |_: &std::fs::File| { + asked.set(true); + Err(std::io::Error::from_raw_os_error(errno)) + }; + write_atomically_syncing(&target.to_string_lossy(), b"NEW", [7], refuse).unwrap(); + assert!(asked.get(), "the full flush was tried"); + assert_eq!(read(&target), "NEW"); + assert_eq!(listing(&dir), ["out.pdf"], "no staging file is left behind"); + let _ = std::fs::remove_dir_all(&dir); + } + } + + /// Only "not supported" is let through: any other flush failure fails the save and keeps the + /// original. + #[test] + fn a_failed_flush_keeps_the_original() { + let dir = staging_dir("syncfails"); + let target = dir.join("out.pdf"); + std::fs::write(&target, "OLD").unwrap(); + let e = write_atomically_syncing(&target.to_string_lossy(), b"NEW", [7], |_| Err(std::io::Error::other("disk gone"))).unwrap_err(); + assert_eq!(e.to_string(), "disk gone"); + assert_eq!(read(&target), "OLD"); + assert_eq!(listing(&dir), ["out.pdf"], "the staging file was removed"); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn outlook_attachment_copies_are_recognised_on_either_separator_and_in_any_case() { for p in [