composefs-rs: Stream external files when checking out instead of reading them whole into memory

Repo: composefs/composefs-rs

Task: crates/composefs/src/fs.rs (LeafContent::Regular(External..) in the checkout/write path, around the TODO: make this better. At least needs to be EINTR-safe comment) reads the whole object into a Vec<MaybeUninit<u8>> with one read() call before set_file_contents. A large object therefore costs its full size in RAM and a short read or EINTR corrupts the file. Copy it in a bounded-buffer loop that retries on EINTR and handles short reads, writing straight to the destination file (keep the existing mode/ownership/xattr handling), and remove the TODO. Reflink or copy_file_range is optional; only add it if it is a few lines.

Acceptance: a unit test checks out a file larger than the copy buffer (several MiB, plus a zero-length one and one exactly the buffer size) and compares contents; cargo fmt --check, cargo clippy --all-targets and cargo test --workspace --exclude composefs-integration-tests pass (fs-verity tests may fail on a filesystem without verity; say so rather than skipping).

Constraints: code and tests only, no GitHub writes (the coordinator opens the PR); no design decisions; do not touch .github/ workflows; keep the change small and match the surrounding style; commit message in the project's style with a Generated-by: AI trailer, no Signed-off-by.

Generated-by: https://github.com/cgwalters/#llms

Board scheduler: no plan. A triage run is asked whether this is done; it answers here. Delete this comment to ask again.

Not done. Upstream main still reads the whole object into memory with a single read(), and the TODO is still there: crates/composefs/src/fs.rs#L310-L320 (at f5aa0a2, "Bump version to 0.9.4"). set_file_contents still takes a &[u8]. The issue is valid. It shows up as a code path rather than a failing run: the buffer is the object's full size, and one short read or EINTR truncates the checked-out file without an error.

Three PRs have merged in composefs/composefs-rs since this issue was opened on 3 Oct. None of them touches this code: #413 (CI for standalone composefs-boot), #414 (aboot discovery) and #415 (version bump). No open PR does this work either. Open #409 changes the same match arm (ExternalNoVerity becomes ExternalPath) but keeps the TODO and the whole-file read, so whichever of the two lands second needs a small rebase.

What's left: all of it. That means the bounded copy loop that handles EINTR and short reads, writing straight into the tmpfile, plus the test the issue's acceptance asks for and removing the TODO. The fix is small and self-contained. The only real choice is how set_file_contents gets the data (a reader or a write callback instead of &[u8]), and that is an implementation detail, not a design question.

{
  "schema": "agent-board/job-plan/v1",
  "repo": "cgwalters-forge/tracker",
  "item": 317,
  "kind": "implement",
  "runner": "hosted",
  "pool": "openai",
  "brief": "Implement this issue in composefs/composefs-rs. In crates/composefs/src/fs.rs, write_leaf's LeafContent::Regular(External|ExternalNoVerity) arm (the 'TODO: make this better. At least needs to be EINTR-safe' comment) reads the whole object into a Vec<MaybeUninit<u8>> with one read() and passes it to set_file_contents. A large object costs its full size in RAM, and a short read or EINTR leaves the file truncated. Copy the object in a bounded-buffer loop that retries on EINTR and handles short reads, writing straight to the destination file. Keep set_file_contents' O_TMPFILE+linkat path, its OPNOTSUPP fallback, sync_data and the existing mode/ownership/xattr handling, for example by having it take a reader or a write callback rather than a &[u8]. Remove the TODO. Reflink or copy_file_range is optional; add it only if it is a few lines. Acceptance: a unit test checks out a file larger than the copy buffer (several MiB), a zero-length one and one exactly the buffer size, and compares contents. cargo fmt --check, cargo clippy --all-targets and cargo test --workspace --exclude composefs-integration-tests pass; fs-verity tests may fail on a filesystem without verity, so say so rather than skipping them. Open PR composefs/composefs-rs#409 changes the same match arm (ExternalNoVerity becomes ExternalPath and the object id comes from file.repo_object_id()); base on main and keep the change confined to the copy so a rebase stays trivial. Constraints: code and tests only; no design decisions; do not touch .github/ workflows; keep the change small and match the surrounding style; commit message in the project's style with a Generated-by: AI trailer and no Signed-off-by."
}

{"request":"c50ca122-5d45-43ea-89bd-1a588d2c4236","verdict":"not-done"}

Generated by dispatch · ◷

Dispatched the plan in #317 (comment): run.

bounds: Let runs target the composefs-rs and bootc forks, as staged work

❌ dispatch handed back no result because required data was missing.

(Run status from 2026-10-17T11:19:07Z, re-posted by the bot without the agent's own words. The original job-token comment was removed so this issue counts as collaborator-only input, tracker#738.)

Generated-by: https://github.com/cgwalters/#llms

✅ dispatch ended well. Result: #317 (comment)

Needs the operator: the fix already exists as draft cgwalters-forge/composefs-rs#26, which is waiting on your review. Don't dispatch another implement run.

#26 ("fs: Copy repository objects with std::io::copy", one commit, crates/composefs/src/fs.rs +96/−15) came from the duplicate tracker#356. It does what this issue asks:

set_file_contents now takes a Read instead of &[u8]. It streams with std::io::copy, which retries on EINTR and handles short reads, and it uses copy_file_range for File/Take<File>. The O_TMPFILE+linkat path, the OPNOTSUPP fallback and the mode/xattr handling are unchanged.
The External arm copies File::from(object).take(size) and removes the TODO.
A new test copies empty, small, 5 MiB and 16 MiB sparse files, both whole and size-limited. There's no "exactly the buffer size" case. It matters less here because copy_file_range runs without a user-space buffer, and only the fallback path uses io::copy's 8 KiB buffer.
The PR says the devspace run passed fmt, clippy and the fs::tests tests. In the workspace run, 10 fs-verity tests failed with FilesystemNotSupported (XFS runner). It still shows as mergeable with upstream main (now at 3280e06), and upstream #409, which touches the same match arm, is still open.

Upstream main hasn't changed here: the whole-file read() and the TODO are still at fs.rs#L310-L320. So the issue is still valid until #26 lands.

The 17 Oct dispatch from the plan above (run) ended with missing_data. The plan's repo was cgwalters-forge/tracker, so the agent got the tracker checkout and no fs.rs. Any future plan for this has to target cgwalters-forge/composefs-rs, which is the fork in dispatch.toml's bounds.

Decision for you on #26. The PR adds one behaviour change: an object shorter than its recorded size is now an error (the ensure! in write_leaf), where before it silently wrote a truncated file. You can:

(a) approve or /promote as is. This keeps the error, which I recommend, because a silently truncated checkout is the bug this issue describes.
(b) ask in review to drop the ensure! so it's a strict no-behaviour-change copy.
(c) close bootc: composefs edit follow-ups (sealed UKI test, actionable same-digest error, edit log op) #26 and replan.

Once #26 merges upstream, close this issue as completed.

Generated by dispatch · ◷

Question for the operator (from the audit of the needs-human queue): Re-run the failed centos-stream9 job on composefs-rs#432 and merge it; OK for the bot to close the duplicate forge composefs-rs#14 (yes/no)?

The board's Why for this item was: Fork PR cgwalters-forge/composefs-rs#14; awaiting review by cgwalters

Generated-by: https://github.com/cgwalters/#llms

Done: composefs/composefs-rs#432 merged upstream on 26 Oct. It replaces the whole-object read() and its TODO in crates/composefs/src/fs.rs with a std::io::copy stream (EINTR-safe, short-read safe, copy_file_range for files), checks the copied size, and tests empty, small, multi-MiB and sparse files. Fork draft cgwalters-forge/composefs-rs#14 is now a duplicate; closing it is the operator's call. The epic plan tool can only close as not planned; the work itself is complete.

Navigation Menu

Repository navigation

composefs-rs: Stream external files when checking out instead of reading them whole into memory #317

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

composefs-rs: Stream external files when checking out instead of reading them whole into memory #317

Description

Activity

cgwalters-bot commented on Oct 17, 2026

cgwalters-bot commented on Oct 17, 2026

cgwalters-bot commented on Oct 17, 2026

cgwalters-bot commented on Oct 18, 2026

cgwalters-bot commented on Oct 22, 2026

cgwalters-bot commented on Oct 22, 2026

cgwalters-bot commented on Oct 26, 2026

cgwalters-bot commented on Oct 27, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions