From 3f9cd3b87dc495c3b9e74290951d191a47ea9d1c Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Thu, 15 Oct 2026 14:48:09 +0800 Subject: [PATCH] M1: every PDF leaves the engine checked, and every writer uses one staging-file writer (#2166) (#2182) New files from Combine, Extract, Split and Create, signed, timestamped and LTV-embedded files and the OCR batch output reopen with their pages before anyone writes them, as saves already did. The desktop app, the automation tools (with or without --root) and the CLI's edit, combine, extract and split write through pdfcraft_platform::staging::write_atomically, so a failed write or --out naming the input never leaves a half-written file. Special files such as /dev/stdout are still written as given. Claude-Session: https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz Co-authored-by: Claude Fable 5.1 Co-authored-by: Brandon Thomas --- Cargo.lock | 2 + ROADMAP.md | 1 + apps/pdfcraft-cli/Cargo.toml | 1 + apps/pdfcraft-cli/src/main.rs | 17 +++-- apps/pdfcraft-cli/tests/edit.rs | 14 +++++ crates/automation/src/lib.rs | 40 +++++------- crates/automation/tests/automation.rs | 18 ++++++ crates/engine/src/lib.rs | 26 ++++++-- crates/engine/src/ocr.rs | 5 +- crates/platform/Cargo.toml | 4 ++ crates/platform/src/staging.rs | 89 +++++++++++++++++++++++++++ crates/ui-egui/src/editing.rs | 41 ++---------- parity/acrobat-features.toml | 4 +- 13 files changed, 189 insertions(+), 73 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3d8d45707..07ee42e7d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3960,6 +3960,7 @@ dependencies = [ "pdfcraft-automation", "pdfcraft-cos", "pdfcraft-engine", + "pdfcraft-platform", "pdfcraft-render", "rustix 1.1.5", "serde_json", @@ -4186,6 +4187,7 @@ version = "0.8.0" dependencies = [ "async-channel", "futures-lite", + "rustix 1.1.5", "zbus", ] diff --git a/ROADMAP.md b/ROADMAP.md index 3b1c39a14..6348ed7e4 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -157,6 +157,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log - **2026-10-13 (G14, Vietnamese localization):** Added Vietnamese (`vi`, Tiếng Việt) as an interface and date language with a 2,591-entry catalog (`plural_none`) covering 100% of the current localizable key union (plus `theme`/`Appearance`). Locale tags `vi` / `vi-VN` / `vi_VN.UTF-8` resolve; calendar months use numeric `Tháng 1`…`12` / `tháng 1`…`12`. Native-speaker community review recorded. The twelve-language localization mean moves from ≈59% to ≈68%; weighted readiness ≈49% → ≈50%. Status stays `partial`: engine/OS details, document contents, filenames, CLI/MCP and external help stay outside the catalog by design. +- **2026-10-14 (M1, #2166):** No PDF leaves the engine unchecked and no writer truncates: new files from Combine, Extract, Split and Create, signed, timestamped and LTV-embedded files and the OCR batch output reopen with their pages before anyone writes them, as saves already did; the desktop app, the automation tools (with or without `--root`) and the CLI's edit, combine, extract and split all write through the one staging-file writer in `pdfcraft-platform`, so a failed write or `--out` naming the input never leaves a half-written file. Special files such as `/dev/stdout` are still written as given. - **2026-10-14 (M3 #2080 candidate):** Existing per-tab page history gains explicit search jumps, rail buttons and control-channel commands. Continuous scrolling and automatic search refresh stay outside history; page-level restoration and estimates unchanged. - **2026-10-13 (M2, page-unit transform precision):** Preserve small transform coefficients in page marks, added content and print output. Regression cases cover accepted page units of 4096 and 75000; existing page-unit interpretation and estimates are unchanged. diff --git a/apps/pdfcraft-cli/Cargo.toml b/apps/pdfcraft-cli/Cargo.toml index abe2657fb..d7b52fbd6 100644 --- a/apps/pdfcraft-cli/Cargo.toml +++ b/apps/pdfcraft-cli/Cargo.toml @@ -19,6 +19,7 @@ pdfcraft-engine = { workspace = true } pdfcraft-audit = { workspace = true } pdfcraft-cos = { workspace = true } pdfcraft-automation = { workspace = true } +pdfcraft-platform = { workspace = true } base64 = "0.22" [target.'cfg(unix)'.dependencies] diff --git a/apps/pdfcraft-cli/src/main.rs b/apps/pdfcraft-cli/src/main.rs index d94c85447..3ec005344 100644 --- a/apps/pdfcraft-cli/src/main.rs +++ b/apps/pdfcraft-cli/src/main.rs @@ -498,7 +498,16 @@ fn edit(args: &[String]) -> Result<(), CliError> { session.apply(id, e).map_err(|e| e.to_string())?; } let bytes = if full { session.save_full_bytes(id) } else { session.save_bytes(id) }.map_err(|e| e.to_string())?; - std::fs::write(out, bytes.as_slice()).map_err(|e| format!("{out}: {e}").into()) + // As the app's Save: the file must reopen with its pages before it replaces anything (#1698). + session.check_save(id, &bytes).map_err(|e| e.to_string())?; + write_pdf(Path::new(out), &bytes) +} + +/// Write a PDF a command produced, through a staging file beside the target (#2166): a write +/// that fails, or `--out` naming the input, never leaves a truncated file where a good one was. +fn write_pdf(out: &Path, bytes: &[u8]) -> Result<(), CliError> { + use pdfcraft_platform::staging::{StagingName, write_atomically}; + write_atomically(out, bytes, StagingName::SuffixThenTag).map_err(|e| format!("{}: {e}", out.display()).into()) } fn file_stem(path: &str) -> String { @@ -514,7 +523,7 @@ fn combine(args: &[String]) -> Result<(), CliError> { } let sources = inputs.iter().map(|p| Ok((file_stem(p), read(p)?))).collect::, String>>()?; let bytes = pdfcraft_engine::Session::new().combine(&sources).map_err(|e| e.to_string())?; - std::fs::write(out, bytes.as_slice()).map_err(|e| format!("{out}: {e}").into()) + write_pdf(Path::new(out), &bytes) } fn extract(args: &[String]) -> Result<(), CliError> { @@ -525,7 +534,7 @@ fn extract(args: &[String]) -> Result<(), CliError> { let mut session = pdfcraft_engine::Session::new(); let id = session.open(path, None, read(path)?, flag(args, "--password")).map_err(|e| e.to_string())?; let bytes = session.extract(id, &pages).map_err(|e| e.to_string())?; - std::fs::write(out, bytes.as_slice()).map_err(|e| format!("{out}: {e}").into()) + write_pdf(Path::new(out), &bytes) } fn split(args: &[String]) -> Result<(), CliError> { @@ -547,7 +556,7 @@ fn split(args: &[String]) -> Result<(), CliError> { std::fs::create_dir_all(&dir).map_err(|e| format!("split: --out-dir {}: {e}", dir.display()))?; for (a, b, bytes) in parts { let name = dir.join(if a == b { format!("{stem}-p{a}.pdf") } else { format!("{stem}-p{a}-{b}.pdf") }); - std::fs::write(&name, bytes.as_slice()).map_err(|e| format!("{}: {e}", name.display()))?; + write_pdf(&name, &bytes)?; stdout_line(format_args!("{}", name.display()))?; } Ok(()) diff --git a/apps/pdfcraft-cli/tests/edit.rs b/apps/pdfcraft-cli/tests/edit.rs index c9a5e7e2b..2864dcfb3 100644 --- a/apps/pdfcraft-cli/tests/edit.rs +++ b/apps/pdfcraft-cli/tests/edit.rs @@ -92,6 +92,20 @@ fn the_documented_options_still_edit_and_save() { } } +/// `--out` naming the input replaces it through a staging file (#2166): the result is the +/// edited, reopenable file, and nothing is left beside it. +#[test] +fn editing_in_place_leaves_a_valid_file_and_no_staging_file() { + let input = tmp("in-place", "doc.pdf"); + std::fs::write(&input, fixture()).unwrap(); + let out = Command::new(BIN).args(["edit", input.to_str().unwrap(), "--out", input.to_str().unwrap(), "--rotate", "1:90"]).output().unwrap(); + assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr)); + assert_eq!(first_page_rotation(&input), 90); + let names: Vec = + std::fs::read_dir(input.parent().unwrap()).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect(); + assert_eq!(names, ["doc.pdf"], "no staging file beside the result"); +} + mod one_based_arguments { use super::*; use std::sync::atomic::{AtomicU64, Ordering}; diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index f003cc12d..751c1801d 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -33,7 +33,7 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use pdfcraft_engine::{CombineSource, DocId, Document, Edit, Session, commands}; -use pdfcraft_platform::staging::{StagingName, create_staging, staging_suffixes}; +use pdfcraft_platform::staging::{StagingName, staging_suffixes}; use pdfcraft_render::{PageRenderer, PageText, RenderConfig, RenderRequest, RequestKind}; use serde_json::{Value, json}; @@ -146,14 +146,16 @@ impl Automation { /// Save `bytes` that a caller writes on the session's behalf, such as `pdfcraft-cli run` /// saving a rendered page. With a root, it is confined like a tool's own output (a relative - /// path resolves inside it) and written atomically. Without one, the path is written as - /// given, so `/dev/stdout` and the like still work. Returns where the file went. + /// path resolves inside it). Written atomically either way (#2166), except to something + /// that is not a regular file, such as `/dev/stdout` or a pipe, which is written as given. + /// Returns where the file went. pub fn write_output(&self, path: &str, bytes: &[u8]) -> Result { let target = self.resolve(path, true)?; - if self.root.is_some() { - write_atomic(&target, bytes)?; - } else { + let special = self.root.is_none() && std::fs::metadata(&target).is_ok_and(|m| !m.is_file() && !m.is_dir()); + if special { std::fs::write(&target, bytes).map_err(|e| failed(format!("{path}: {e}")))?; + } else { + write_atomic(&target, bytes)?; } Ok(target) } @@ -2440,32 +2442,20 @@ fn write_atomic_synced( suffixes: impl IntoIterator, sync: impl FnOnce(&std::fs::File) -> std::io::Result<()>, ) -> Result<()> { - use std::io::Write; // A folder can't be replaced, and staging beside it could land outside the root: "." names // the root itself, whose parent isn't ours to write in. if path.is_dir() { return Err(failed(format!("{}: is a folder, not a file", path.display()))); } let dir = path.parent().filter(|p| !p.as_os_str().is_empty()).unwrap_or(Path::new(".")); - let name = path.file_name().ok_or_else(|| failed(format!("{}: not a file path", path.display())))?; + path.file_name().ok_or_else(|| failed(format!("{}: not a file path", path.display())))?; std::fs::create_dir_all(dir).map_err(|e| failed(format!("{}: {e}", dir.display())))?; - let (tmp, file) = - create_staging(dir, &name.to_string_lossy(), StagingName::SuffixThenTag, suffixes).map_err(|e| failed(format!("{}: {e}", dir.display())))?; - // Closed at the end of the block, before the rename. - let written = { - let mut file = file; - // Surface delayed writeback failures before replacing the destination or marking - // a document clean. The handle is closed before rename (required on Windows). - file.write_all(bytes).and_then(|()| sync(&file)) - }; - if let Err(e) = written { - let _ = std::fs::remove_file(&tmp); - return Err(failed(format!("{}: {e}", tmp.display()))); - } - std::fs::rename(&tmp, path).map_err(|e| { - let _ = std::fs::remove_file(&tmp); - failed(format!("{}: {e}", path.display())) - }) + // The writer itself is the platform crate's, shared with the desktop app and the CLI + // (#2166): the staging file is written, flushed (delayed writeback failures surface before + // the destination is replaced or a document marked clean), closed and renamed; a failure at + // any step removes it and leaves the destination as it was. + pdfcraft_platform::staging::write_atomically_with(path, bytes, StagingName::SuffixThenTag, suffixes, sync) + .map_err(|e| failed(format!("{}: {e}", path.display()))) } #[cfg(test)] diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index e7600f572..a7db10e58 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -6276,3 +6276,21 @@ fn doc_print_as_image() { other => panic!("{other:?}"), } } + +/// Without a root, `write_output` still writes through a staging file (#2166): the folder is +/// made, nothing is left beside the file, and a folder at the path is refused. +#[test] +fn a_root_less_write_output_is_atomic_too() { + let dir = std::env::temp_dir().join(format!("pdfcraft-automation-rootless-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let free = Automation::new(); + let target = dir.join("new").join("page.png"); + let written = free.write_output(&target.to_string_lossy(), &[1, 2, 3]).unwrap(); + assert_eq!(written, target); + assert_eq!(std::fs::read(&target).unwrap(), [1, 2, 3]); + let names: Vec<_> = std::fs::read_dir(dir.join("new")).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect(); + assert_eq!(names, ["page.png"], "no staging file left beside it"); + let e = free.write_output(&dir.join("new").to_string_lossy(), &[4]).unwrap_err(); + assert!(matches!(&e, ToolError::Failed(m) if m.contains("is a folder")), "{e:?}"); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 6af859416..1058a8e46 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -4352,7 +4352,7 @@ impl Session { let mut cuts = Vec::new(); let mut used = 0usize; for p in 0..n { - let size = self.write_new(&pdfcraft_organize::extract_pages(src, &[p])?)?.len(); + let size = self.write_unchecked(&pdfcraft_organize::extract_pages(src, &[p])?)?.len(); if used > 0 && used + size > max_bytes { cuts.push(p); used = 0; @@ -4450,7 +4450,18 @@ impl Session { doc.editor.as_ref().map(|e| &e.cos).ok_or_else(|| EditError::ReadOnly(doc.read_only_reason.clone().unwrap_or_default())) } + /// A new file (Combine, Extract, Split, Create) as bytes, checked like a save before anyone + /// writes it (#1698, #2166): it reopens, and shows the pages it was built from. fn write_new(&self, doc: &pdfcraft_cos::Document) -> Result>, EditError> { + let bytes = self.write_unchecked(doc)?; + let expected = pdfcraft_organize::page_count(doc)?; + pdfcraft_cos::Document::open(bytes.clone()).map_err(|e| EditError::Reopen(e.to_string()))?; + pdfcraft_render::check_pages(bytes.clone(), None, expected).map_err(|e| EditError::Reopen(e.to_string()))?; + Ok(bytes) + } + + /// [`Self::write_new`] without the check, for bytes nobody writes: measuring a part's size. + fn write_unchecked(&self, doc: &pdfcraft_cos::Document) -> Result>, EditError> { let opts = SaveOptions { mod_date: self.now().map(pdfcraft_cos::pdf_date), ..SaveOptions::default() }; write_full(doc, &opts).map(Arc::new).map_err(|e| EditError::Write(e.to_string())) } @@ -4606,7 +4617,10 @@ impl Session { if opts.date.is_empty() { opts.date = self.signing_date(); } - Ok(Arc::new(pdfcraft_sign::sign_with_timestamp(&editor.cos, id, &opts, tsa)?)) + let signed = Arc::new(pdfcraft_sign::sign_with_timestamp(&editor.cos, id, &opts, tsa)?); + // Checked like a save before anyone writes it (#2166). + self.check_save(doc, &signed)?; + Ok(signed) } /// Append a standalone document timestamp (RFC 3161, `/ETSI.RFC3161`) covering the file's @@ -4615,7 +4629,9 @@ impl Session { let d = self.get(doc).ok_or(EditError::NoDocument)?; let editor = d.editor.as_ref().ok_or_else(|| EditError::ReadOnly(d.read_only_reason.clone().unwrap_or_default()))?; let date = if date.is_empty() { self.signing_date() } else { date }; - Ok(Arc::new(pdfcraft_sign::timestamp_document(&editor.cos, tsa, &date)?)) + let stamped = Arc::new(pdfcraft_sign::timestamp_document(&editor.cos, tsa, &date)?); + self.check_save(doc, &stamped)?; + Ok(stamped) } /// Embed revocation evidence into the catalog's `/DSS` with `/VRI` entries per signature @@ -4623,7 +4639,9 @@ impl Session { pub fn embed_ltv(&self, doc: DocId, evidence: &pdfcraft_sign::dss::Evidence) -> Result>, EditError> { let d = self.get(doc).ok_or(EditError::NoDocument)?; let editor = d.editor.as_ref().ok_or_else(|| EditError::ReadOnly(d.read_only_reason.clone().unwrap_or_default()))?; - Ok(Arc::new(pdfcraft_sign::dss::embed(&editor.cos, evidence)?)) + let embedded = Arc::new(pdfcraft_sign::dss::embed(&editor.cos, evidence)?); + self.check_save(doc, &embedded)?; + Ok(embedded) } /// Record that the signed file `bytes` was saved (to `path`): like [`Session::mark_saved`], diff --git a/crates/engine/src/ocr.rs b/crates/engine/src/ocr.rs index 43a4d7a0a..dcd4142d7 100644 --- a/crates/engine/src/ocr.rs +++ b/crates/engine/src/ocr.rs @@ -248,7 +248,10 @@ pub fn recognize_file( if s.apply_ocr(id, &pages).map_err(|e| e.to_string())? == 0 { return Ok(FileResult { bytes, pages }); } - Ok(FileResult { bytes: s.save_bytes(id).map_err(|e| e.to_string())?, pages }) + // Checked like a save before the caller writes it (#2166). + let bytes = s.save_bytes(id).map_err(|e| e.to_string())?; + s.check_save(id, &bytes).map_err(|e| e.to_string())?; + Ok(FileResult { bytes, pages }) } #[cfg(test)] diff --git a/crates/platform/Cargo.toml b/crates/platform/Cargo.toml index cbae93fca..a552a43f8 100644 --- a/crates/platform/Cargo.toml +++ b/crates/platform/Cargo.toml @@ -10,6 +10,10 @@ workspace = true [dependencies] +[target.'cfg(target_os = "macos")'.dependencies] +# `write_atomically`: a volume that refuses F_FULLFSYNC gets a plain fsync. +rustix = { version = "1", features = ["fs"] } + [target.'cfg(target_os = "linux")'.dependencies] # Already in the dependency graph through the accessibility integration. The portal watcher # uses the async API so dropping it can cancel an idle subscription without polling. diff --git a/crates/platform/src/staging.rs b/crates/platform/src/staging.rs index 641726117..f7200efb9 100644 --- a/crates/platform/src/staging.rs +++ b/crates/platform/src/staging.rs @@ -64,6 +64,62 @@ pub fn staging_suffixes() -> impl Iterator { (0u64..).map(move |i| state.hash_one(i)) } +/// Write `bytes` to `path` through a staging file beside it (see [`create_staging`]; `form` +/// names it as the caller always has), flushed to the disk and then renamed over the target: a +/// crash, a full disk or a refused flush never leaves a half-written file where a good one was, +/// and the target is untouched until the new file is complete. The target's folder must exist; +/// a folder at the target is refused by the rename. Flushing is the staging file's own +/// (`sync_all`); parent-directory durability after a power loss is not promised. +pub fn write_atomically(path: &Path, bytes: &[u8], form: StagingName) -> std::io::Result<()> { + write_atomically_with(path, bytes, form, staging_suffixes(), std::fs::File::sync_all) +} + +/// [`write_atomically`] with the staging-name suffixes and the flush given, so tests can plant +/// names and refuse flushes. A failure at any step removes the staging file and leaves the +/// target as it was. +pub fn write_atomically_with( + path: &Path, + bytes: &[u8], + form: StagingName, + suffixes: impl IntoIterator, + sync_all: impl FnOnce(&std::fs::File) -> std::io::Result<()>, +) -> std::io::Result<()> { + use std::io::Write as _; + let dir = path.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(Path::new(".")); + let name = path.file_name().and_then(|n| n.to_str()).unwrap_or("save"); + let (tmp, file) = create_staging(dir, name, form, suffixes)?; + let result = (|| { + // Closed at the end of the block, before the rename (required on Windows). + { + let mut file = file; + file.write_all(bytes)?; + sync_staged(&file, sync_all)?; + } + std::fs::rename(&tmp, path) + })(); + if result.is_err() { + let _ = std::fs::remove_file(&tmp); + } + result +} + +/// Flush the staging file to the disk before it replaces the target. 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 write. +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(test)] mod tests { use super::*; @@ -126,4 +182,37 @@ mod tests { assert!(!name(STAGING_ATTEMPTS as u64 + 1).exists(), "a 17th name was tried"); let _ = std::fs::remove_dir_all(&dir); } + + /// `write_atomically`: the target holds the new bytes only once they are complete; a refused + /// flush leaves the old file and removes the staging file; a missing folder writes nothing. + #[test] + fn an_atomic_write_replaces_the_target_only_when_complete() { + let dir = test_dir("write"); + let target = dir.join("out.pdf"); + std::fs::write(&target, b"OLD").unwrap(); + let names = |dir: &Path| { + let mut v: Vec = std::fs::read_dir(dir).unwrap().map(|e| file_name(&e.unwrap().path())).collect(); + v.sort(); + v + }; + // The staging file is written and flushed before the target changes. + let mut seen_old = false; + write_atomically_with(&target, b"NEW", StagingName::SuffixThenTag, [7], |f| { + seen_old = std::fs::read(&target).unwrap() == b"OLD" && f.metadata().unwrap().len() == 3; + Ok(()) + }) + .unwrap(); + assert!(seen_old, "flushed before publication, with the target still the old file"); + assert_eq!(std::fs::read(&target).unwrap(), b"NEW"); + assert_eq!(names(&dir), ["out.pdf"], "no staging file is left"); + // A refused flush keeps the target and cleans up. + let e = write_atomically_with(&target, b"NEWER", StagingName::TagThenSuffix, [8], |_| Err(std::io::Error::other("refused"))).unwrap_err(); + assert!(e.to_string().contains("refused")); + assert_eq!(std::fs::read(&target).unwrap(), b"NEW"); + assert_eq!(names(&dir), ["out.pdf"]); + // A folder that does not exist: nothing is written anywhere. + assert!(write_atomically(&dir.join("missing").join("out.pdf"), b"X", StagingName::SuffixThenTag).is_err()); + assert_eq!(names(&dir), ["out.pdf"]); + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index be9691e8f..ddd1af824 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -2,7 +2,7 @@ //! "save changes?" prompt when closing a tab or quitting with unsaved edits. use pdfcraft_engine::Edit; -use pdfcraft_platform::staging::{StagingName, create_staging, staging_suffixes}; +use pdfcraft_platform::staging::{StagingName, staging_suffixes}; use crate::PdfCraftApp; @@ -944,48 +944,15 @@ fn write_atomically_with(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(".")); - let name = target.file_name().and_then(|n| n.to_str()).unwrap_or("save"); - let (tmp, f) = create_staging(dir, name, StagingName::TagThenSuffix, suffixes)?; - let result = (|| { - // Closed at the end of the block, before the rename. - { - let mut f = f; - f.write_all(bytes)?; - sync_staged(&f, sync_all)?; - } - std::fs::rename(&tmp, target) - })(); - if result.is_err() { - let _ = std::fs::remove_file(&tmp); - } - 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) + pdfcraft_platform::staging::write_atomically_with(std::path::Path::new(path), bytes, StagingName::TagThenSuffix, suffixes, sync_all) } #[cfg(not(target_arch = "wasm32"))] diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 8d57c31c0..a25ff43fa 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -346,8 +346,8 @@ milestone = "M1" status = "shipped" commands = [] tools = [] -evidence = ["crates/cos/tests/encryption.rs::qpdf_agrees", "crates/cos/tests/writer.rs::qpdf_accepts_object_stream_output"] -notes = "qpdf is installed on the Linux CI runner and PDFCRAFT_ORACLES_REQUIRED makes its absence a failure, so these run on every CI run rather than skipping; on a machine without qpdf they skip with a notice (crates/oracle)" +evidence = ["crates/cos/tests/encryption.rs::qpdf_agrees", "crates/cos/tests/writer.rs::qpdf_accepts_object_stream_output", "crates/platform/src/staging.rs::an_atomic_write_replaces_the_target_only_when_complete", "apps/pdfcraft-cli/tests/edit.rs::editing_in_place_leaves_a_valid_file_and_no_staging_file", "crates/automation/tests/automation.rs::a_root_less_write_output_is_atomic_too"] +notes = "qpdf is installed on the Linux CI runner and PDFCRAFT_ORACLES_REQUIRED makes its absence a failure, so these run on every CI run rather than skipping; on a machine without qpdf they skip with a notice (crates/oracle). Every PDF leaves the engine checked (it reopens with its pages): saves, new files from Combine/Extract/Split/Create, signed, timestamped and LTV-embedded files, OCR batch output; and every writer goes through one staging-file writer (the desktop app, the tools with or without --root, the CLI), so a failed write never leaves a truncated file (#1698, #2166)" [[feature]] id = "core.deterministic-save"