mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-15 03:37:03 -04:00
On macOS sync_all is F_FULLFSYNC, which SMB and FUSE volumes such as a mounted Cryptomator vault refuse with ENOTSUP or ENOTTY, so every save there failed. Those two errors now fall back to fsync; any other flush error still fails the save.
This commit is contained in:
1 parent
c4bf96b897
commit
c0c40acf22
3 files changed
+71
-2
No files matched your search
Generated
+1
@@ -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",
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<Item = u64>) -> 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<Item = u64>,
|
||||
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<I
|
||||
{
|
||||
let mut f = f;
|
||||
f.write_all(bytes)?;
|
||||
f.sync_all()?;
|
||||
sync_staged(&f, sync_all)?;
|
||||
}
|
||||
std::fs::rename(&tmp, target)
|
||||
})();
|
||||
@@ -943,6 +954,23 @@ fn write_atomically_with(path: &str, bytes: &[u8], suffixes: impl IntoIterator<I
|
||||
result
|
||||
}
|
||||
|
||||
/// Flush the staging file to the disk before it replaces the original. On macOS `sync_all` asks
|
||||
/// for a full flush (`F_FULLFSYNC`), which SMB and FUSE volumes, such as a mounted Cryptomator
|
||||
/// vault, refuse as unsupported: there a plain `fsync` is the strongest flush the volume offers.
|
||||
/// Any other failure still fails the save.
|
||||
fn sync_staged(f: &std::fs::File, sync_all: impl FnOnce(&std::fs::File) -> 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 [
|
||||
|
||||
Reference in new issue
Block a user