Skip to content

fix: preserve ledgers when formatter writes fail - #1054

Open
ryanduguid wants to merge 2 commits into
beancount:masterfrom
ryanduguid:fix/atomic-format-output
Open

ryanduguid wants to merge 2 commits into
beancount:masterfrom
ryanduguid:fix/atomic-format-output

Conversation

@ryanduguid

@ryanduguid ryanduguid commented Sep 21, 2026 •

Copy link
Copy Markdown

Fixes #1051.

bean-format --in-place and bean-format input -o input can truncate the ledger before a failing write completes. A regression that writes 10 characters and then raises an error reproduces the data loss in both paths.

Write regular-file outputs to a temporary file in the destination directory, close it, then replace the destination. Write, close and replacement failures preserve the original and clean up the temporary file. Open input files only while reading so replacing the same path also works on Windows. Standard input/output and device outputs remain supported; in-place standard input is rejected.

Tests cover both output paths, partial-write and close failures, failed replacement, multiple files, standard streams, read-only inputs, symlinks and POSIX permission bits.

Validation on Windows, Python 3.12.10:

  • pytest beancount -q -o faulthandler_timeout=30: 1,118 passed, 24 skipped, 2 expected failures.
  • meson test -C build-test: one native test passed.
  • mypy .: no issues in 159 source files, with a warning that current Mypy no longer supports the configured Python 3.9 target.
  • Pinned Ruff lint and format checks, applicable pre-commit checks on the two changed files, and git diff --check pass, except the pinned Mypy hook. That hook fails parsing the installed Click dependency under the Python 3.9 configuration; the same failure occurs on the unmodified upstream formatter.

The two POSIX permission tests were skipped locally. The full OS/Python CI matrix and repository-wide pre-commit run remain unverified. Atomic replacement follows symlinks and preserves permission bits, but other hard links retain the old contents and other file metadata may change, as documented in the CLI help. Replacing a file also needs write access to its directory for the temporary file; a writable ledger in a read-only directory now fails before the ledger is changed. This does not claim durability across power loss.

A new output file has nothing to preserve, so write it directly instead of
through a 0600 temporary file. Flush and fsync replacement files before the
rename.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bean-format --in-place leaves a zero-byte ledger when the write fails, and bean-check accepts it

1 participant