Skip to content

fix: patch copied files in place for prefix replacements#2283

Open
TTTPOB wants to merge 6 commits intoconda:mainfrom
TTTPOB:better-reflink-2282
Open

fix: patch copied files in place for prefix replacements#2283
TTTPOB wants to merge 6 commits intoconda:mainfrom
TTTPOB:better-reflink-2282

Conversation

@TTTPOB
Copy link
Copy Markdown

@TTTPOB TTTPOB commented Mar 25, 2026

Summary

  • switch patched-file installs to copy first and then patch the destination in place
  • temporarily add owner-write access before patching copied read-only files, then restore the original permissions afterward
  • preserve the existing mtime/codesign behavior and add focused tests for same-length, resized, and read-only patched files

Closes #2282

Testing

  • CARGO_HOME=/tmp/cargo-container cargo fmt --all --check
  • CARGO_HOME=/tmp/cargo-container CARGO_TARGET_DIR=/tmp/rattler-target cargo test -p rattler test_patch_copied_destination_updates_only_changed_ranges_when_lengths_match --lib -j1
  • CARGO_HOME=/tmp/cargo-container CARGO_TARGET_DIR=/tmp/rattler-target cargo test -p rattler test_patch_copied_destination_rewrites_from_first_difference_when_length_changes --lib -j1
  • CARGO_HOME=/tmp/cargo-container CARGO_TARGET_DIR=/tmp/rattler-target cargo test -p rattler test_patched_read_only_file_restores_permissions --lib -j1
  • CARGO_HOME=/tmp/cargo-container CARGO_TARGET_DIR=/tmp/rattler-target cargo test -p rattler test_patched_file_receives_modification_time --lib -j1
  • CARGO_HOME=/tmp/cargo-container CARGO_TARGET_DIR=/tmp/rattler-target cargo test -p rattler test_unpatched_file_keeps_source_mtime --lib -j1

@TTTPOB TTTPOB changed the title fix: patch copied files in place for prefix replacements feat: patch copied files in place for prefix replacements Mar 25, 2026
@TTTPOB TTTPOB changed the title feat: patch copied files in place for prefix replacements fix: patch copied files in place for prefix replacements Mar 25, 2026
@TTTPOB TTTPOB marked this pull request as ready for review March 25, 2026 16:57
@TTTPOB
Copy link
Copy Markdown
Author

TTTPOB commented Apr 15, 2026

@hunger @wolfv
Gentle ping on this PR when you have a chance.

This fixes an issue in the pixi store install path where files are currently patched before hardlink/reflink is applied. As a result, the link step cannot fully preserve the intended space-saving behavior, so in many real cases the deduplication benefits are lost.

I’d appreciate a review when convenient. Thanks!

Copy link
Copy Markdown
Collaborator

@baszalmstra baszalmstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I dont quite understand is why we need the sink and everything? It looks overengineered. I also dont understand how this now works? Because you still just copy a file to the destination and then seem to patch that? That still doesnt preserve the COW semantics does it?

@TTTPOB
Copy link
Copy Markdown
Author

TTTPOB commented Apr 16, 2026

  1. I think you are right. The reason I used this overengineered abstraction was to preserve the old helpers and tests. I was a bit unsure whether I should just remove those old helpers and tests.

  2. Yes, this PR just copies a file to the destination and then patches it, but Rust fs::copy calls the copy_file_range syscall on Linux, and fclonefileat on macOS (falling back to fcopyfile). The Linux copy_file_range syscall will automatically do a reflink copy if the filesystem supports it, which takes advantage of a CoW semantics.

Here are some links:

Rust fs::copy comments
https://github.com/rust-lang/rust/blob/e8e4541ff19649d95afab52fdde2c2eaa6829965/library/std/src/fs.rs#L2844

For btrfs
torvalds/linux@fce205e

For ZFS
https://github.com/openzfs/zfs/blob/1644e2ffd2640fa3e2c191ceaf048a5fc8399493/module/os/linux/zfs/zpl_file.c#L1239

If this PR makes sense to you, I will simplify the overengineered abstraction and clean up those tests and helpers.

@baszalmstra
Copy link
Copy Markdown
Collaborator

We also have a crate specifically to do reflinks, also on windows, I would prefer to use that instead.

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.

Feature request: use reflink-then-prefix-replace for patched files to preserve CoW sharing

2 participants