mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-28 03:58:45 -06:00
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 <noreply@anthropic.com> Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
Brandon Thomas
parent
c3e9609bd1
commit
3f9cd3b87d
Generated
+2
@@ -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",
|
||||
]
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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::<Result<Vec<_>, 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(())
|
||||
|
||||
@@ -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<String> =
|
||||
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};
|
||||
|
||||
@@ -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<PathBuf> {
|
||||
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<Item = u64>,
|
||||
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)]
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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<Arc<Vec<u8>>, 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<Arc<Vec<u8>>, 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<Arc<Vec<u8>>, 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`],
|
||||
|
||||
@@ -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)]
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -64,6 +64,62 @@ pub fn staging_suffixes() -> impl Iterator<Item = u64> {
|
||||
(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<Item = u64>,
|
||||
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<String> = 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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<I
|
||||
}
|
||||
|
||||
/// [`write_atomically_with`], flushing the staging file with `sync_all` (the standard library's
|
||||
/// in the app, a failing one in tests).
|
||||
/// in the app, a failing one in tests). The writer itself is the platform crate's, shared with
|
||||
/// the tools and the CLI (#2166); the desktop app keeps its staging-name form.
|
||||
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("."));
|
||||
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"))]
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user