Atomic writes: convert bun init and test a write that fails

Closed
RO
1mo ago
+1402-289

Stacked on #39689, which adds File::write_file_atomically and converts the package.json and snapshot writers. This PR holds what that branch leaves out: one more writer, and tests in which the write itself fails.

Problem

  • bun init on a project with a package.json writes the new text over the old bytes and truncates afterwards (init_command.rs, 'write_package_json). A write that stops early leaves the start of the new file in front of the rest of the old one: package.json failed to write due to error EFBIG, file changed.
  • #39689 tests writers that succeed. Nothing tests a write that fails, which is what the atomic write is for.

Fix

  • bun init writes package.json with write_file_atomically, after it closes the handle it read the file with. The Windows-only save and restore of that handle's position around the read is deleted: it only served the write through the same handle.
  • A failed .snap write at the end of a serial bun test run sets the exit code, like a failed inline snapshot write. It used to leave main as an internal error. The --parallel worker (test/parallel/runner.rs, worker_flush_aggregates) still drops both results and exits 0. That needs a worker to coordinator message and is left out here on purpose. It is reported separately.
  • Verified: withFileSizeLimit(blocks, cmd) in test/harness.ts runs a command under ulimit -f. New tests in bun-pm-pkg, bun-pm-version, init and new-snapshot.test.ts check that the old file is intact and no .tmp file is left, after EFBIG and after a process.exit() under --update-snapshots. All fail on 1.4 canary. On #39689 alone the init test and the exit code assertion fail.

Background

  • Bun ignores SIGXFSZ, so a write past RLIMIT_FSIZE returns EFBIG. A limit of 0 fails every write at once, which exposes a truncate-then-write writer. A limit of one block makes Linux cut the write short, which exposes a write-then-truncate writer. The tests only check that the file is intact, so they also pass on macOS, where the whole write fails.
  • write_file_atomically in #39689 returns the write error without trying the in-place fallback, so these tests hold with its fallback in place.
  • bun create still writes in place. It rewrites the package.json it has just copied.
Notes

The first version of this PR also made write_file_atomically keep the owner of the old file. #39689 took that change (35bb9d3), together with a fix for its two-writer test, so this PR was rebuilt on top of it as one commit without the file.rs part. The review notes on the earlier commits (drained pipes, one-line comments, the deleted Windows seek pair, the --parallel exclusion) are all folded into the current commit.

no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/test/snapshot-tests/new-snapshot.test.ts, test/cli/install/bun-pm-version.test.ts, test/cli/install/bun-pm-pkg.test.ts, test/cli/init/init.test.ts