From c732bc8d46b339fdb59df5aaba4109c5be4c555c Mon Sep 17 00:00:00 2001 From: Ofer Chen Date: Thu, 6 Aug 2026 23:49:11 +0300 Subject: [PATCH 1/5] fix(engine): give local --append-verify the phase-2 redo model The local-copy executor treated --append-verify as an a-priori prefix comparison: determine_append_mode() compared the destination prefix against the source before transferring and, on mismatch, returned AppendMode::Disabled - silently degrading to an ordinary single-pass whole-file copy. The final bytes were right, so nothing failed, but the whole append -> verify -> retain -> redo model was absent: local --append-verify never appended, never retained a partial, never warned, and never ran a second pass. Upstream's order is the opposite. Pass one is always a pure append: the sender jumps last_match to the destination length and zeroes the block count (match.c:372-391) and the generator emits a sum header with no block sums (generator.c:787). receive_data() then compares whole-file checksums (receiver.c:517-519), which under --append-verify fold in the pre-existing prefix on both sides (match.c:373-386, receiver.c:357-371). A mismatch keeps the appended bytes, because --append implies --inplace (options.c:2400-2411) and receiver.c:1029 takes its `|| inplace` leg for recv_ok == 0, warns (receiver.c:1063-1097), and asks the generator to redo the file with append_mode negated and ignore_times bumped (generator.c:2186-2200) against a session whose whole_file was already forced to 0 (generator.c:2288-2289). Upstream runs that loop locally too - local_server (main.c:1468) forks local_child (main.c:649-655) and do_recv forks again (main.c:1050) so recv_files and generate_files run over a socketpair. execute_transfer is split into execute_transfer_once, which reports a TransferOutcome, and a verify_redo wrapper that supplies the second pass. determine_append_mode now appends regardless and reports verify_failed; the prefix comparison is kept as the predicate because locally it is the whole-file comparison, both sides summing the same appended tail. Measured local pull, 200 KiB source over a 100 KiB zero-filled seed, against rsync 3.4.4: transfers 1 -> 2, Literal 204,800 -> 205,300, Total transferred file size 204,800 -> 409,600, and the retained-update WARNING now reaches stderr under -v and stays silent by default, all matching upstream exactly. Matched data remains over-counted by the pre-existing append accounting defect, which reproduces on plain --append with a matching prefix and is tracked separately. --- .../src/local_copy/executor/file/append.rs | 93 +++++-- .../local_copy/executor/file/copy/dry_run.rs | 6 +- .../file/copy/transfer/execute/mod.rs | 52 ++-- .../executor/file/copy/transfer/mod.rs | 20 +- .../file/copy/transfer/verify_redo.rs | 233 ++++++++++++++++++ .../src/local_copy/tests/execute_append.rs | 136 +++++++++- 6 files changed, 497 insertions(+), 43 deletions(-) create mode 100644 crates/engine/src/local_copy/executor/file/copy/transfer/verify_redo.rs diff --git a/crates/engine/src/local_copy/executor/file/append.rs b/crates/engine/src/local_copy/executor/file/append.rs index 2183046bf5..da12f7305f 100644 --- a/crates/engine/src/local_copy/executor/file/append.rs +++ b/crates/engine/src/local_copy/executor/file/append.rs @@ -14,15 +14,32 @@ pub(crate) enum AppendMode { Disabled, /// Destination is already at least as large as the source; skip the file. Skip, - /// Destination is shorter; append starting from the given offset. - Append(u64), + /// Destination is shorter; append starting from `offset`. + Append { + /// Byte offset the appended tail starts at - the destination's length. + offset: u64, + /// Whether `--append-verify`'s whole-file re-checksum will fail, so the + /// caller must retain the appended result and redo the file in phase 2. + verify_failed: bool, + }, } /// Decides the append strategy for a file based on existing destination size. /// /// Returns `Disabled` when append is off, `Skip` when the destination is -/// already at least as large, or `Append(offset)` when the destination is -/// shorter and the transfer should resume from that offset. +/// already at least as large, or `Append` when the destination is shorter and +/// the transfer should resume from that offset. +/// +/// Under `--append-verify` a mismatching prefix does **not** cancel the append. +/// Upstream appends first and only then compares whole-file checksums: the +/// sender sums the source's first `flength` bytes and the receiver sums the +/// destination's, both followed by the identical appended tail +/// (match.c:372-391, receiver.c:352-379), so the comparison reduces exactly to +/// "do the two prefixes agree". A disagreement is reported through +/// `verify_failed` rather than acted on here, because upstream keeps the +/// appended bytes on disk (receiver.c:1029, reached because `--append` implies +/// `--inplace` - options.c:2400-2411) and redoes the file in phase 2 against +/// that retained partial. // upstream: receiver.c:recv_files() - append mode size comparison pub(crate) fn determine_append_mode( append_allowed: bool, @@ -60,21 +77,23 @@ pub(crate) fn determine_append_mode( return Ok(AppendMode::Skip); } - if append_verify { - let matches = verify_append_prefix(reader, source, destination, existing_len)?; - reader - .seek(SeekFrom::Start(0)) - .map_err(|error| LocalCopyError::io("copy file", source, error))?; - if !matches { - return Ok(AppendMode::Disabled); - } + // Plain `--append` (append_mode == 1) never re-checksums: upstream skips the + // prefix in `sum_update` on both sides (match.c:373-391 runs the CHUNK_SIZE + // loop only when `append_mode == 2`), so the two whole-file sums cover just + // the appended tail and always agree. + let verify_failed = if append_verify { + !verify_append_prefix(reader, source, destination, existing_len)? } else { - reader - .seek(SeekFrom::Start(0)) - .map_err(|error| LocalCopyError::io("copy file", source, error))?; - } + false + }; + reader + .seek(SeekFrom::Start(0)) + .map_err(|error| LocalCopyError::io("copy file", source, error))?; - Ok(AppendMode::Append(existing_len)) + Ok(AppendMode::Append { + offset: existing_len, + verify_failed, + }) } /// Verifies that the existing destination prefix matches the source. @@ -272,7 +291,15 @@ mod tests { .expect("determine"); match result { - AppendMode::Append(offset) => assert_eq!(offset, 6), // "source" is 6 bytes + AppendMode::Append { + offset, + verify_failed, + } => { + assert_eq!(offset, 6); // "source" is 6 bytes + // Without --append-verify upstream never re-checksums the + // prefix, so no redo can be requested. + assert!(!verify_failed); + } AppendMode::Disabled | AppendMode::Skip => panic!("expected Append mode"), } } @@ -299,13 +326,19 @@ mod tests { .expect("determine"); match result { - AppendMode::Append(offset) => assert_eq!(offset, 15), + AppendMode::Append { + offset, + verify_failed, + } => { + assert_eq!(offset, 15); + assert!(!verify_failed); + } AppendMode::Disabled | AppendMode::Skip => panic!("expected Append mode"), } } #[test] - fn determine_append_mode_with_verify_disabled_when_prefix_mismatch() { + fn determine_append_mode_still_appends_when_verify_will_fail() { let temp = tempdir().expect("tempdir"); let source_path = temp.path().join("source.txt"); let dest_path = temp.path().join("dest.txt"); @@ -325,7 +358,25 @@ mod tests { ) .expect("determine"); - assert!(matches!(result, AppendMode::Disabled)); + // Degrading to a plain whole-file copy here is exactly the bug this + // encodes against: upstream appends the tail regardless, keeps the + // result (--append implies --inplace, options.c:2400-2411), and only + // then reports the failed whole-file re-checksum so the generator can + // redo the file against the retained partial (generator.c:2175-2217). + // Cancelling the append would leave nothing to re-delta and would make + // the transfer look like a clean single-pass copy. + match result { + AppendMode::Append { + offset, + verify_failed, + } => { + assert_eq!(offset, 16); + assert!(verify_failed); + } + AppendMode::Disabled | AppendMode::Skip => { + panic!("a failed verification must still append, then redo") + } + } } #[test] diff --git a/crates/engine/src/local_copy/executor/file/copy/dry_run.rs b/crates/engine/src/local_copy/executor/file/copy/dry_run.rs index 590ebc506f..9f34d2f375 100644 --- a/crates/engine/src/local_copy/executor/file/copy/dry_run.rs +++ b/crates/engine/src/local_copy/executor/file/copy/dry_run.rs @@ -160,8 +160,12 @@ pub(super) fn handle_dry_run( )); return Ok(()); } + // A dry run never writes, so the phase-2 redo a failed `--append-verify` + // would trigger has nothing to report: upstream's `--dry-run` receiver takes + // the `discard_receive_data()` leg (receiver.c:797) and never reaches the + // `recv_ok` switch that queues the redo. let append_offset = match append_mode { - AppendMode::Append(offset) => offset, + AppendMode::Append { offset, .. } => offset, AppendMode::Disabled | AppendMode::Skip => 0, }; let bytes_transferred = file_size.saturating_sub(append_offset); diff --git a/crates/engine/src/local_copy/executor/file/copy/transfer/execute/mod.rs b/crates/engine/src/local_copy/executor/file/copy/transfer/execute/mod.rs index 1e5696fabd..95c569bbc2 100644 --- a/crates/engine/src/local_copy/executor/file/copy/transfer/execute/mod.rs +++ b/crates/engine/src/local_copy/executor/file/copy/transfer/execute/mod.rs @@ -1,9 +1,10 @@ //! Main file transfer orchestration. //! -//! Hosts [`execute_transfer`], the central function that drives a single file -//! copy: skip detection, backup, append-mode resume, delta signature, +//! Hosts [`execute_transfer_once`], the central function that drives a single +//! file copy pass: skip detection, backup, append-mode resume, delta signature, //! writer-strategy selection (direct, inplace, temp-file), buffer allocation, -//! data copy, and post-transfer bookkeeping. +//! data copy, and post-transfer bookkeeping. The `verify_redo` module wraps it +//! to supply the second pass an `--append-verify` failure requires. //! //! # Submodules //! @@ -43,10 +44,10 @@ use super::super::super::comparison::build_delta_signature; use super::super::super::compute_backup_path; use super::super::super::guard::remove_incomplete_destination; use super::super::super::preallocate::maybe_preallocate_destination; -use super::TransferFlags; use super::finalize::finalize_guard_and_metadata; use super::open::open_source_file; use super::write_strategy::{open_destination_writer, select_write_strategy}; +use super::{TransferFlags, TransferOutcome}; use skip::try_skip_up_to_date; @@ -57,7 +58,7 @@ use skip::try_skip_up_to_date; /// when a usable basis file exists at the destination. The caller is /// responsible for pre-checks (dry-run, size filters, link processing). #[allow(clippy::too_many_arguments)] -pub(in crate::local_copy) fn execute_transfer( +pub(in crate::local_copy) fn execute_transfer_once( context: &mut CopyContext, source: &Path, destination: &Path, @@ -76,7 +77,7 @@ pub(in crate::local_copy) fn execute_transfer( // rather than a network transfer (`>`). // upstream: generator.c:1039 - itemize(..., ITEM_LOCAL_CHANGE, ...). reference_basis: Option, -) -> Result<(), LocalCopyError> { +) -> Result { #[cfg(not(all(unix, any(feature = "xattr", feature = "acl"))))] let _ = mode; @@ -122,7 +123,7 @@ pub(in crate::local_copy) fn execute_transfer( &flags, mode, )? { - return Ok(()); + return Ok(TransferOutcome::Complete); } } @@ -223,6 +224,8 @@ pub(in crate::local_copy) fn execute_transfer( // instead falls through to the generic read/write loop below, which streams // its `file_size` bytes just like a regular file (upstream sender.c:410-418). if !file_type.is_file() && device_as_file_size.is_none() { + // A placeholder write carries no appended tail and no whole-file + // checksum, so it can never fail verification. return super::special::copy_special_as_regular_file( context, source, @@ -236,7 +239,8 @@ pub(in crate::local_copy) fn execute_transfer( relative, mode, flags, - ); + ) + .map(|()| TransferOutcome::Complete); } // Fast path: macOS clonefile for new whole-file copies. Skipped for @@ -265,7 +269,7 @@ pub(in crate::local_copy) fn execute_transfer( flags, )? { - return Ok(()); + return Ok(TransferOutcome::Complete); } // Fast path: Windows CopyFileExW / ReFS reflink for new whole-file copies. @@ -296,7 +300,7 @@ pub(in crate::local_copy) fn execute_transfer( flags, )? { - return Ok(()); + return Ok(TransferOutcome::Complete); } // Fast path: Linux FICLONE reflink for new whole-file copies on Btrfs, @@ -325,7 +329,7 @@ pub(in crate::local_copy) fn execute_transfer( flags, )? { - return Ok(()); + return Ok(TransferOutcome::Complete); } let mut reader = open_source_file(source, context.open_noatime_enabled()) @@ -355,11 +359,19 @@ pub(in crate::local_copy) fn execute_transfer( Duration::default(), Some(metadata_snapshot), )); - return Ok(()); + return Ok(TransferOutcome::Complete); } + // `verify_failed` is upstream's `recv_ok == 0` for this file, decided before + // the append rather than after it because the local executor can compare the + // two prefixes directly. The append still runs: upstream retains the result + // and re-deltas it in phase 2 (receiver.c:1029, generator.c:2175-2217). + let mut verify_failed = false; let append_offset = match append_mode { - AppendMode::Append(offset) => { + AppendMode::Append { + offset, + verify_failed: failed, + } => { debug_log!( Send, 2, @@ -367,6 +379,7 @@ pub(in crate::local_copy) fn execute_transfer( record_path.display(), offset ); + verify_failed = failed; offset } AppendMode::Disabled | AppendMode::Skip => 0, @@ -467,7 +480,7 @@ pub(in crate::local_copy) fn execute_transfer( mode, flags, )? { - return Ok(()); + return Ok(TransferOutcome::Complete); } let mut writer = open_destination_writer( @@ -778,7 +791,16 @@ pub(in crate::local_copy) fn execute_transfer( preserve_acls, )?; - Ok(()) + // upstream: receiver.c:1015 - `recv_ok = receive_data(...)` compares the + // sender's whole-file checksum against the receiver's. The local executor + // reduces that comparison to the pre-append prefix comparison recorded in + // `verify_failed` (see `determine_append_mode`), because both sides sum the + // same appended tail. + Ok(if verify_failed { + TransferOutcome::VerificationFailed + } else { + TransferOutcome::Complete + }) } /// Finds a fuzzy delta basis for `destination` when the exact destination is diff --git a/crates/engine/src/local_copy/executor/file/copy/transfer/mod.rs b/crates/engine/src/local_copy/executor/file/copy/transfer/mod.rs index 4895007957..6bc37c0786 100644 --- a/crates/engine/src/local_copy/executor/file/copy/transfer/mod.rs +++ b/crates/engine/src/local_copy/executor/file/copy/transfer/mod.rs @@ -17,11 +17,29 @@ mod execute; mod finalize; mod open; mod special; +mod verify_redo; mod write_strategy; -pub(super) use execute::execute_transfer; #[cfg(test)] pub(crate) use open::take_fsync_call_count; +pub(super) use verify_redo::execute_transfer; + +/// Whether a single transfer pass completed or must be redone. +/// +/// Mirrors upstream's `recv_ok` for the two outcomes the local executor can +/// produce: `recv_ok == 1` (committed) and `recv_ok == 0` (the whole-file +/// re-checksum failed, so the update is kept and the file is queued for the +/// phase-2 redo). upstream: receiver.c:1061-1101. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(in crate::local_copy) enum TransferOutcome { + /// The pass committed; nothing further is owed for this file. + Complete, + /// An `--append-verify` pass appended its tail but the whole-file + /// re-checksum disagreed. The appended bytes stay on disk and the caller + /// must rerun the file as an ordinary delta transfer. + /// upstream: receiver.c:1096 `send_msg_int(MSG_REDO, ndx)`. + VerificationFailed, +} /// Boolean flags controlling file transfer behavior. /// diff --git a/crates/engine/src/local_copy/executor/file/copy/transfer/verify_redo.rs b/crates/engine/src/local_copy/executor/file/copy/transfer/verify_redo.rs new file mode 100644 index 0000000000..bbbde98478 --- /dev/null +++ b/crates/engine/src/local_copy/executor/file/copy/transfer/verify_redo.rs @@ -0,0 +1,233 @@ +//! The `--append-verify` append -> verify -> retain -> redo cycle. +//! +//! Upstream gets this for free on a local transfer because "local" is just a +//! client and a server exchanging the ordinary protocol over a socketpair +//! (main.c:1468 sets `local_server`, main.c:649-655 forks `local_child`, and +//! main.c:1050-1132 forks again so `recv_files()` and `generate_files()` run +//! concurrently). The local executor is a separate implementation of that +//! transfer, so it has to reproduce the semantics explicitly: +//! +//! 1. **Append first.** The sender jumps `last_match` to the destination's +//! length and zeroes the block count (match.c:372-391), and the generator +//! writes a sum header with no block sums (generator.c:787), so pass one is +//! always a pure append. +//! 2. **Verify the whole file.** `receive_data()` compares the sender's +//! whole-file checksum with the receiver's (receiver.c:517-519). With +//! `--append-verify` both sides fold the pre-existing prefix into that sum +//! (match.c:373-386, receiver.c:357-371) and then the identical appended +//! tail, so the comparison is exactly "do the two prefixes agree". +//! 3. **Retain the result.** `--append` implies `--inplace` +//! (options.c:2400-2411), so `finish_transfer()` runs even for `recv_ok == 0` +//! (receiver.c:1029) and the appended bytes stay on disk. +//! 4. **Warn and request the redo** (receiver.c:1063-1097). +//! 5. **Redo as an ordinary delta.** The generator re-enters `recv_generator()` +//! with `append_mode` negated and `ignore_times` bumped +//! (generator.c:2186-2200), and `whole_file` was already forced to 0 for the +//! whole session because append mode is active (generator.c:2288-2289), so +//! the retained partial is described as the delta basis +//! (generator.c:1967 -> generate_and_send_sums). + +use std::fs; +use std::path::{Path, PathBuf}; + +use ::metadata::MetadataOptions; + +use crate::local_copy::{CopyContext, LocalCopyError, LocalCopyExecution}; + +use super::execute::execute_transfer_once; +use super::{TransferFlags, TransferOutcome}; + +/// Executes the data transfer for a single regular file, including the +/// `--append-verify` phase-2 redo when the first pass fails verification. +/// +/// The common case is a single call to [`execute_transfer_once`]. Only an +/// `--append-verify` pass whose whole-file re-checksum disagreed runs a second +/// pass, and that pass is an ordinary delta transfer against the partial the +/// first pass left on disk. +#[allow(clippy::too_many_arguments)] +pub(in crate::local_copy) fn execute_transfer( + context: &mut CopyContext, + source: &Path, + destination: &Path, + metadata: &fs::Metadata, + metadata_options: MetadataOptions, + record_path: &Path, + existing_metadata: Option<&fs::Metadata>, + destination_previously_existed: bool, + file_type: fs::FileType, + relative: Option<&Path>, + flags: TransferFlags, + mode: LocalCopyExecution, + copy_source_override: Option, + reference_basis: Option, +) -> Result<(), LocalCopyError> { + let outcome = execute_transfer_once( + context, + source, + destination, + metadata, + metadata_options.clone(), + record_path, + existing_metadata, + destination_previously_existed, + file_type, + relative, + flags, + mode, + copy_source_override.clone(), + reference_basis.clone(), + )?; + + if outcome == TransferOutcome::Complete { + return Ok(()); + } + + warn_verification_failed(context, record_path); + + // The first pass appended in place, so the destination now holds the full + // source length with a wrong prefix. Re-stat it: that partial is the delta + // basis for the redo, exactly as upstream's `generate_and_send_sums()` reads + // the retained file. Losing it between the passes means there is nothing to + // re-delta, so a stat failure is a hard error rather than a fall-back to a + // whole-file copy. + let retained = fs::symlink_metadata(destination).map_err(|error| { + LocalCopyError::io("inspect retained partial", destination.to_path_buf(), error) + })?; + + execute_transfer_once( + context, + source, + destination, + metadata, + metadata_options, + record_path, + Some(&retained), + // The destination existed before this pass: upstream counts + // `stats.created_files++` only on the non-redo leg (receiver.c:778). + true, + file_type, + relative, + redo_flags(flags, context.sparse_enabled()), + mode, + copy_source_override, + reference_basis, + )?; + + Ok(()) +} + +/// The flag set upstream's generator installs around the phase-2 redo. +/// +/// upstream: generator.c:2186-2188 negates `append_mode` and bumps +/// `ignore_times`; generator.c:2288-2289 already forced `whole_file = 0` for the +/// session because append mode is active, which is what overrides the +/// `whole_file = 1` default a local transfer would otherwise carry +/// (main.c:652-653); receiver.c:761,771 negates `sparse_files` alongside +/// `append_mode`, restoring whatever `--sparse` asked for. +fn redo_flags(flags: TransferFlags, sparse_enabled: bool) -> TransferFlags { + TransferFlags { + append_allowed: false, + append_verify: false, + whole_file_enabled: false, + // Without this the quick-check would skip the redo outright: the + // retained partial now has the source's size, and pass one already + // stamped it with the source's mtime. + ignore_times_enabled: true, + use_sparse_writes: sparse_enabled, + ..flags + } +} + +/// Emits upstream's retained-update warning for a failed verification. +/// +/// upstream: receiver.c:1071-1094. The line is gated behind +/// `INFO_GTE(NAME, 1) || stdout_format_has_i` (receiver.c:1072) and carries the +/// transfer-relative name, never an absolute path. +/// +/// `keptstr` is unconditionally `"retained"`. Upstream reaches that string +/// because `--append` implies `--inplace` (options.c:2400-2411), which takes +/// receiver.c:1073-1078 past both the `"discarded"` and the +/// `"put into partial-dir"` leg. The local executor arrives at the same place by +/// a different route: this warning is reachable only when `append_offset > 0`, +/// which pins `select_write_strategy` to `WriteStrategy::Append`, and that +/// strategy writes straight into the destination - so the update is retained +/// there no matter what `--partial-dir` says. +/// +/// `redostr` is `" (will try again)"` because the local executor never replays a +/// batch, and the code is `FWARNING` rather than `FERROR_XFER` because the redo +/// this warning announces has not run yet (upstream's `redoing` is still 0). +/// +/// The line goes to stderr because upstream emits it as `FWARNING` +/// (rsync.h:278, routed by log.c:314-316), matching the other stderr notices +/// this executor writes. +fn warn_verification_failed(context: &CopyContext, record_path: &Path) { + if !logging::info_gte(logging::InfoFlag::Name, 1) && !context.options().is_itemize_active() { + return; + } + eprintln!( + "WARNING: {} failed verification -- update retained (will try again).", + record_path.display() + ); +} + +#[cfg(test)] +mod tests { + use super::*; + + fn flags() -> TransferFlags { + TransferFlags { + append_allowed: true, + append_verify: true, + whole_file_enabled: true, + inplace_enabled: false, + partial_enabled: false, + use_sparse_writes: false, + compress_enabled: false, + size_only_enabled: false, + ignore_times_enabled: false, + checksum_enabled: false, + #[cfg(all(any(unix, windows), feature = "xattr"))] + preserve_xattrs: false, + xattrs_changed: false, + #[cfg(all(any(unix, windows), feature = "acl"))] + preserve_acls: false, + } + } + + #[test] + fn redo_negates_append_and_forces_a_delta_pass() { + // Each of these is load-bearing for the redo to reproduce upstream's + // second pass: still appending would re-append nothing, whole-file + // would re-send the file as pure literal instead of re-deltaing the + // retained partial, and honouring times would skip the file entirely + // because pass one already gave it the source's size and mtime. + let redo = redo_flags(flags(), false); + assert!(!redo.append_allowed); + assert!(!redo.append_verify); + assert!(!redo.whole_file_enabled); + assert!(redo.ignore_times_enabled); + } + + #[test] + fn redo_restores_sparse_writes_from_the_session_setting() { + // upstream: receiver.c:761,771 negates sparse_files alongside + // append_mode, so --sparse suppressed during the append comes back for + // the redo. Without --sparse it must stay off. + assert!(redo_flags(flags(), true).use_sparse_writes); + assert!(!redo_flags(flags(), false).use_sparse_writes); + } + + #[test] + fn redo_preserves_unrelated_flags() { + // The redo negates only what upstream negates; everything else is the + // session's setting and must survive. + let mut base = flags(); + base.compress_enabled = true; + base.checksum_enabled = true; + base.inplace_enabled = true; + let redo = redo_flags(base, false); + assert!(redo.compress_enabled); + assert!(redo.checksum_enabled); + assert!(redo.inplace_enabled); + } +} diff --git a/crates/engine/src/local_copy/tests/execute_append.rs b/crates/engine/src/local_copy/tests/execute_append.rs index cc6db1f22c..870bfd324e 100644 --- a/crates/engine/src/local_copy/tests/execute_append.rs +++ b/crates/engine/src/local_copy/tests/execute_append.rs @@ -323,7 +323,7 @@ fn append_verify_succeeds_when_prefix_matches() { } #[test] -fn append_verify_retransfers_when_prefix_mismatch() { +fn append_verify_appends_then_redoes_when_prefix_mismatch() { let temp = tempdir().expect("tempdir"); let source = temp.path().join("source.txt"); let destination = temp.path().join("dest.txt"); @@ -345,8 +345,15 @@ fn append_verify_retransfers_when_prefix_mismatch() { ) .expect("copy succeeds"); - // File should be re-transferred completely due to mismatch - assert_eq!(summary.files_copied(), 1); + // Two passes, not one. A failed --append-verify is not a licence to fall + // back to a single whole-file copy: upstream appends the tail, keeps it + // (--append implies --inplace, options.c:2400-2411, so receiver.c:1029 + // commits even for recv_ok == 0), warns, and redoes the file in phase 2 + // against that retained partial (generator.c:2175-2217). Each pass counts + // as a transfer, which is how the redo is observable at all - collapsing to + // one means the retain-and-redo model is missing. + // MEASURED against rsync 3.4.4: `Number of regular files transferred: 2`. + assert_eq!(summary.files_copied(), 2); assert_eq!( fs::read(&destination).expect("read dest"), b"correct source content plus more" @@ -412,14 +419,133 @@ fn append_verify_detects_corruption_in_middle() { ) .expect("copy succeeds"); - // File should be re-transferred due to corruption detection - assert_eq!(summary.files_copied(), 1); + // Corruption inside the existing prefix takes the same append -> verify -> + // retain -> redo route as any other mismatch, so it is two passes. + // MEASURED against rsync 3.4.4 on this exact fixture: 2 transfers, + // `Literal data: 30 bytes`, and the retained-update WARNING on stderr. + assert_eq!(summary.files_copied(), 2); assert_eq!( fs::read(&destination).expect("read dest"), b"0123456789ABCDEFGHIJ" ); } +#[test] +fn append_verify_failure_retains_the_appended_partial_for_the_redo() { + // The load-bearing property of the whole cycle: after the verification + // fails, the bytes the append wrote must still be on disk, because they are + // the delta basis the second pass re-deltas against. This is what upstream + // buys with `--append` implying `--inplace` (options.c:2400-2411), which + // sends receiver.c:1029 down the `|| inplace` leg for recv_ok == 0. + // + // The check is indirect but exact: the source's second half is byte-equal to + // the destination's second half after pass one, so if the partial were + // discarded the redo would have nothing to match and the final file could + // only be reconstructed as pure literal. Asserting the two-pass count plus + // byte-correctness pins both halves of that. + let temp = tempdir().expect("tempdir"); + let source = temp.path().join("source.bin"); + let destination = temp.path().join("dest.bin"); + + // A prefix long enough to span several delta blocks, so the redo has real + // matchable content in the tail the append contributed. + let source_bytes: Vec = (0..40_000u32).map(|i| (i % 251) as u8).collect(); + let seed = vec![0u8; 20_000]; + fs::write(&source, &source_bytes).expect("write source"); + fs::write(&destination, &seed).expect("write mismatching partial"); + + let operands = vec![ + source.into_os_string(), + destination.clone().into_os_string(), + ]; + let plan = LocalCopyPlan::from_operands(&operands).expect("plan"); + + let summary = plan + .execute_with_options( + LocalCopyExecution::Apply, + LocalCopyOptions::default().append_verify(true), + ) + .expect("copy succeeds"); + + assert_eq!( + summary.files_copied(), + 2, + "one append pass plus one redo pass" + ); + assert_eq!( + fs::read(&destination).expect("read dest"), + source_bytes, + "the redo must reconstruct the file exactly" + ); +} + +#[test] +fn append_verify_success_stays_a_single_pass() { + // The redo must be reachable only through a real verification failure. A + // matching prefix means the whole-file sums agree (receiver.c:517-519), so + // upstream never sends MSG_REDO and the file is transferred once. Without + // this guard a redo that fired unconditionally would still produce correct + // bytes and go unnoticed. + let temp = tempdir().expect("tempdir"); + let source = temp.path().join("source.bin"); + let destination = temp.path().join("dest.bin"); + + let source_bytes: Vec = (0..40_000u32).map(|i| (i % 251) as u8).collect(); + fs::write(&source, &source_bytes).expect("write source"); + fs::write(&destination, &source_bytes[..20_000]).expect("write matching partial"); + + let operands = vec![ + source.into_os_string(), + destination.clone().into_os_string(), + ]; + let plan = LocalCopyPlan::from_operands(&operands).expect("plan"); + + let summary = plan + .execute_with_options( + LocalCopyExecution::Apply, + LocalCopyOptions::default().append_verify(true), + ) + .expect("copy succeeds"); + + assert_eq!(summary.files_copied(), 1, "a clean append must not redo"); + assert_eq!(fs::read(&destination).expect("read dest"), source_bytes); +} + +#[test] +fn plain_append_never_redoes_even_with_a_mismatching_prefix() { + // upstream: match.c:373-386 folds the pre-existing prefix into the + // whole-file sum only when `append_mode == 2` (--append-verify). Plain + // --append leaves both sides summing just the appended tail, so the sums + // always agree, no redo is ever requested, and the destination is left + // knowingly wrong. Pinning this keeps the redo from leaking onto --append. + let temp = tempdir().expect("tempdir"); + let source = temp.path().join("source.txt"); + let destination = temp.path().join("dest.txt"); + + fs::write(&source, b"0123456789ABCDEFGHIJ").expect("write source"); + fs::write(&destination, b"01234XXX89").expect("write mismatching partial"); + + let operands = vec![ + source.into_os_string(), + destination.clone().into_os_string(), + ]; + let plan = LocalCopyPlan::from_operands(&operands).expect("plan"); + + let summary = plan + .execute_with_options( + LocalCopyExecution::Apply, + LocalCopyOptions::default().append(true), + ) + .expect("copy succeeds"); + + assert_eq!(summary.files_copied(), 1, "--append must stay single-pass"); + assert_eq!( + fs::read(&destination).expect("read dest"), + b"01234XXX89ABCDEFGHIJ", + "--append keeps the wrong prefix - that is the documented trade-off" + ); +} + #[test] fn append_without_verify_blindly_appends() { let temp = tempdir().expect("tempdir"); From fba393aa5830e19ac313f423c9dfc28c72267c15 Mon Sep 17 00:00:00 2001 From: Ofer Chen Date: Thu, 6 Aug 2026 23:52:30 +0300 Subject: [PATCH 2/5] test(engine): retarget the append-verify byte-count pin at the redo execute_with_append_verify_rewrites_on_mismatch asserted 6 literal bytes for a 6-byte source over a 3-byte mismatching seed - the count you get only if the append never happens and the file is copied whole in one pass. Measured on that exact fixture, rsync 3.4.4 reports 2 transfers and 9 literal bytes: 3 appended, then all 6 re-sent as literal by the redo, because the 6-byte basis is a single short block that cannot match. Matched data is pinned at its current 3 rather than upstream's 0. Append mode never calls matched() (match.c:389-390 zeroes the block count and skips the hash loop) so the pre-existing prefix contributes nothing upstream, while the local summary derives matched as file_size - literal_bytes. That accounting defect is pre-existing and tracked separately; pinning it here makes the assertion fail loudly and name the upstream answer once it is fixed. --- .../engine/src/local_copy/tests/bandwidth.rs | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/local_copy/tests/bandwidth.rs b/crates/engine/src/local_copy/tests/bandwidth.rs index ccaa881afb..09d45bafa9 100644 --- a/crates/engine/src/local_copy/tests/bandwidth.rs +++ b/crates/engine/src/local_copy/tests/bandwidth.rs @@ -75,7 +75,7 @@ fn execute_with_append_appends_missing_bytes() { } #[test] -fn execute_with_append_verify_rewrites_on_mismatch() { +fn execute_with_append_verify_appends_then_redoes_on_mismatch() { let temp = tempdir().expect("tempdir"); let source = temp.path().join("source.txt"); let destination = temp.path().join("dest.txt"); @@ -96,8 +96,27 @@ fn execute_with_append_verify_rewrites_on_mismatch() { .expect("append verify succeeds"); assert_eq!(fs::read(&destination).expect("read dest"), b"abcdef"); - assert_eq!(summary.bytes_copied(), 6); - assert_eq!(summary.matched_bytes(), 0); + + // MEASURED against rsync 3.4.4 on this exact fixture + // (`-a --append-verify --ignore-times --stats`, source "abcdef", seed + // "abx"): 2 transfers, `Literal data: 9 bytes`, `Matched data: 0 bytes`. + // + // Nine literal bytes, not six: pass one appends the 3-byte tail, then the + // failed whole-file re-checksum retains it and the redo re-sends all 6 + // bytes as literal, because the 6-byte basis is one short block that does + // not match. Asserting 6 would be asserting that the append never happened. + assert_eq!(summary.bytes_copied(), 9); + + // KNOWN DIVERGENCE, tracked separately: upstream reports 0 matched bytes + // here. Append mode never calls `matched()` (upstream match.c:389-390 sets + // `last_match = s->flength; s->count = 0;` and skips the hash loop), so the + // pre-existing prefix contributes nothing to `stats.matched_data`. oc + // derives matched as `file_size - literal_bytes` in + // local_copy::plan::summary, so pass one's untouched 3-byte prefix falls + // out as "matched". This is pinned at the current value deliberately: when + // the accounting is fixed this assertion fails and points straight at the + // upstream answer above. + assert_eq!(summary.matched_bytes(), 3); } #[test] From 4b6aa377b574be12a4bdb0caf4ae794506a3f882 Mon Sep 17 00:00:00 2001 From: Ofer Chen Date: Thu, 6 Aug 2026 23:54:35 +0300 Subject: [PATCH 3/5] docs(engine): correct two upstream line citations for the redo path Both were re-read against rsync-3.4.4 rather than trusted. The whole-file checksum comparison is receiver.c:518-519, not 517 - 517 is the DEBUG_GTE(DELTASUM,2) "got file_sum" trace just above it. The leg that makes a dry run skip the recv_ok switch entirely is the `if (!do_xfers)` block at receiver.c:805-810; receiver.c:797 is the unrelated read-batch "Skipping batched update" path. --- crates/engine/src/local_copy/executor/file/copy/dry_run.rs | 4 ++-- .../src/local_copy/executor/file/copy/transfer/verify_redo.rs | 2 +- crates/engine/src/local_copy/tests/execute_append.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/local_copy/executor/file/copy/dry_run.rs b/crates/engine/src/local_copy/executor/file/copy/dry_run.rs index 9f34d2f375..0c05523b2f 100644 --- a/crates/engine/src/local_copy/executor/file/copy/dry_run.rs +++ b/crates/engine/src/local_copy/executor/file/copy/dry_run.rs @@ -161,8 +161,8 @@ pub(super) fn handle_dry_run( return Ok(()); } // A dry run never writes, so the phase-2 redo a failed `--append-verify` - // would trigger has nothing to report: upstream's `--dry-run` receiver takes - // the `discard_receive_data()` leg (receiver.c:797) and never reaches the + // would trigger has nothing to report: upstream's `if (!do_xfers)` leg + // (receiver.c:805-810) logs the item and `continue`s, never reaching the // `recv_ok` switch that queues the redo. let append_offset = match append_mode { AppendMode::Append { offset, .. } => offset, diff --git a/crates/engine/src/local_copy/executor/file/copy/transfer/verify_redo.rs b/crates/engine/src/local_copy/executor/file/copy/transfer/verify_redo.rs index bbbde98478..23066f24ff 100644 --- a/crates/engine/src/local_copy/executor/file/copy/transfer/verify_redo.rs +++ b/crates/engine/src/local_copy/executor/file/copy/transfer/verify_redo.rs @@ -12,7 +12,7 @@ //! writes a sum header with no block sums (generator.c:787), so pass one is //! always a pure append. //! 2. **Verify the whole file.** `receive_data()` compares the sender's -//! whole-file checksum with the receiver's (receiver.c:517-519). With +//! whole-file checksum with the receiver's (receiver.c:518-519). With //! `--append-verify` both sides fold the pre-existing prefix into that sum //! (match.c:373-386, receiver.c:357-371) and then the identical appended //! tail, so the comparison is exactly "do the two prefixes agree". diff --git a/crates/engine/src/local_copy/tests/execute_append.rs b/crates/engine/src/local_copy/tests/execute_append.rs index 870bfd324e..20a9a59e1c 100644 --- a/crates/engine/src/local_copy/tests/execute_append.rs +++ b/crates/engine/src/local_copy/tests/execute_append.rs @@ -482,7 +482,7 @@ fn append_verify_failure_retains_the_appended_partial_for_the_redo() { #[test] fn append_verify_success_stays_a_single_pass() { // The redo must be reachable only through a real verification failure. A - // matching prefix means the whole-file sums agree (receiver.c:517-519), so + // matching prefix means the whole-file sums agree (receiver.c:518-519), so // upstream never sends MSG_REDO and the file is transferred once. Without // this guard a redo that fired unconditionally would still produce correct // bytes and go unnoticed. From 2a5540d8ea550d61a8f8b8c780febddf8e679f25 Mon Sep 17 00:00:00 2001 From: Ofer Chen Date: Fri, 7 Aug 2026 00:14:40 +0300 Subject: [PATCH 4/5] fix(engine): accumulate matched bytes instead of deriving them The local summary derived `matched = file_size - literal_bytes`, which silently assumes every byte that was not literal came from a block match. Append mode falsifies that: the pre-existing prefix is neither literal nor matched, so the derivation reported the whole skipped prefix as matched data. Upstream never derives this figure. `stats.matched_data` grows in exactly one place, `matched()` at match.c:121, reached only through `hash_search()`. A whole-file transfer has `s->count == 0` so the hash loop never runs, and append mode zeroes the count outright (match.c:389-390 `last_match = s->flength; s->count = 0;`). Both therefore report zero matched bytes however little of the file was literal. So report it the way upstream produces it. `FileCopyOutcome` carries a matched-byte count, the delta loop accumulates it at the two points where it emits a matched block, and every path that never consults a signature - whole-file, sparse whole-file, append, the clone/reflink fast paths, and special-file placeholders - reports MATCHED_NONE. This leaves the ordinary delta case numerically identical, because there every byte really is either literal or matched, and corrects append without special-casing the statistic. MEASURED against rsync 3.4.4, `-a --append --ignore-times --stats`, source "abcdef" over a matching "abc": upstream Literal 3 / Matched 0, oc was Literal 3 / Matched 3 and is now Literal 3 / Matched 0. Two tests had encoded the derivation and now assert the upstream values. --- crates/engine/src/local_copy/context.rs | 43 ++++++++++++++----- .../local_copy/context_impl/delta_transfer.rs | 14 +++++- .../src/local_copy/context_impl/transfer.rs | 10 ++--- .../file/copy/transfer/execute/clonefile.rs | 4 +- .../file/copy/transfer/execute/ficlone.rs | 4 +- .../file/copy/transfer/execute/iouring.rs | 4 +- .../file/copy/transfer/execute/mod.rs | 9 ++-- .../file/copy/transfer/execute/wincopy.rs | 4 +- .../executor/file/copy/transfer/special.rs | 6 ++- crates/engine/src/local_copy/mod.rs | 2 +- crates/engine/src/local_copy/plan/summary.rs | 25 ++++++++--- .../engine/src/local_copy/tests/bandwidth.rs | 30 ++++++++----- .../src/local_copy/tests/execute_append.rs | 21 +++++++++ 13 files changed, 132 insertions(+), 44 deletions(-) diff --git a/crates/engine/src/local_copy/context.rs b/crates/engine/src/local_copy/context.rs index b08f63ca60..b607423001 100644 --- a/crates/engine/src/local_copy/context.rs +++ b/crates/engine/src/local_copy/context.rs @@ -401,14 +401,27 @@ impl<'a> FinalizeMetadataParams<'a> { #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] pub(crate) struct FileCopyOutcome { literal_bytes: u64, + matched_bytes: u64, compressed_bytes: Option, } +/// Matched-byte count for copy paths that never match a basis block. +/// +/// A whole-file copy, a sparse whole-file copy, and an append all reach the +/// destination without consulting a signature, so nothing is "matched" even +/// though most of the resulting file may not be literal either. upstream: +/// match_sums() only reaches `matched()` (match.c:121) via `hash_search()`, +/// which is skipped when `s->count == 0` - the whole-file case - and append +/// mode zeroes `s->count` outright (match.c:389-390). +pub(crate) const MATCHED_NONE: u64 = 0; + impl FileCopyOutcome { - /// Creates a new outcome with the given literal and optional compressed byte counts. - const fn new(literal_bytes: u64, compressed_bytes: Option) -> Self { + /// Creates a new outcome with the given literal, matched, and optional + /// compressed byte counts. + const fn new(literal_bytes: u64, matched_bytes: u64, compressed_bytes: Option) -> Self { Self { literal_bytes, + matched_bytes, compressed_bytes, } } @@ -418,6 +431,16 @@ impl FileCopyOutcome { self.literal_bytes } + /// Returns the number of bytes supplied by block matches against the basis. + /// + /// Reported by the copy paths that actually emit matches, never inferred + /// from the file size. upstream: match.c:121 - `stats.matched_data` grows + /// only inside `matched()`, so a whole-file copy and an append both leave + /// it at zero even though most of their bytes were not literal. + pub(crate) const fn matched_bytes(self) -> u64 { + self.matched_bytes + } + /// Returns the compressed byte count, if compression was used. pub(crate) const fn compressed_bytes(self) -> Option { self.compressed_bytes @@ -997,21 +1020,21 @@ mod tests { #[test] fn file_copy_outcome_new_stores_values() { - let outcome = FileCopyOutcome::new(1000, Some(500)); + let outcome = FileCopyOutcome::new(1000, 0, Some(500)); assert_eq!(outcome.literal_bytes(), 1000); assert_eq!(outcome.compressed_bytes(), Some(500)); } #[test] fn file_copy_outcome_new_without_compression() { - let outcome = FileCopyOutcome::new(2000, None); + let outcome = FileCopyOutcome::new(2000, 0, None); assert_eq!(outcome.literal_bytes(), 2000); assert!(outcome.compressed_bytes().is_none()); } #[test] fn file_copy_outcome_zero_bytes() { - let outcome = FileCopyOutcome::new(0, Some(0)); + let outcome = FileCopyOutcome::new(0, 0, Some(0)); assert_eq!(outcome.literal_bytes(), 0); assert_eq!(outcome.compressed_bytes(), Some(0)); } @@ -1025,7 +1048,7 @@ mod tests { #[test] fn file_copy_outcome_clone() { - let outcome = FileCopyOutcome::new(100, Some(50)); + let outcome = FileCopyOutcome::new(100, 0, Some(50)); let cloned = outcome; assert_eq!(cloned.literal_bytes(), 100); assert_eq!(cloned.compressed_bytes(), Some(50)); @@ -1033,7 +1056,7 @@ mod tests { #[test] fn file_copy_outcome_debug_format() { - let outcome = FileCopyOutcome::new(100, None); + let outcome = FileCopyOutcome::new(100, 0, None); let debug = format!("{outcome:?}"); assert!(debug.contains("FileCopyOutcome")); assert!(debug.contains("100")); @@ -1041,9 +1064,9 @@ mod tests { #[test] fn file_copy_outcome_eq() { - let a = FileCopyOutcome::new(100, Some(50)); - let b = FileCopyOutcome::new(100, Some(50)); - let c = FileCopyOutcome::new(100, Some(60)); + let a = FileCopyOutcome::new(100, 0, Some(50)); + let b = FileCopyOutcome::new(100, 0, Some(50)); + let c = FileCopyOutcome::new(100, 0, Some(60)); assert_eq!(a, b); assert_ne!(a, c); } diff --git a/crates/engine/src/local_copy/context_impl/delta_transfer.rs b/crates/engine/src/local_copy/context_impl/delta_transfer.rs index 35d30eef71..f37ab4a914 100644 --- a/crates/engine/src/local_copy/context_impl/delta_transfer.rs +++ b/crates/engine/src/local_copy/context_impl/delta_transfer.rs @@ -119,6 +119,14 @@ impl<'a> CopyContext<'a> { // match_report()). let mut probe = ProbeCounters::default(); let mut file_matches = 0u64; + // Accumulated where matches are produced, never derived from + // `total_size - literal_bytes`. upstream: match.c:121 `matched()` is the + // only place `stats.matched_data` grows, so a byte counts as matched + // exactly when a block match emitted it - bytes that are neither + // literal nor matched (the prefix an append skips over) count as + // neither. upstream: match.c:389-390 sets `last_match = s->flength; + // s->count = 0;` so append mode never reaches `matched()` at all. + let mut matched_bytes = 0u64; let mut sparse_state = SparseWriteState::default(); sparse_state.set_preallocated_len(preallocated_len); let mut window: VecDeque = VecDeque::with_capacity(index.block_length()); @@ -281,6 +289,7 @@ impl<'a> CopyContext<'a> { } total_bytes = total_bytes.saturating_add(block_len as u64); + matched_bytes = matched_bytes.saturating_add(block_len as u64); let progressed = initial_bytes.saturating_add(total_bytes); self.notify_progress(relative, Some(total_size), progressed, start.elapsed()); window.clear(); @@ -395,6 +404,7 @@ impl<'a> CopyContext<'a> { } total_bytes = total_bytes.saturating_add(block_len as u64); + matched_bytes = matched_bytes.saturating_add(block_len as u64); let progressed = initial_bytes.saturating_add(total_bytes); self.notify_progress(relative, Some(total_size), progressed, start.elapsed()); window.clear(); @@ -477,9 +487,9 @@ impl<'a> CopyContext<'a> { let delta = compressed_total.saturating_sub(compressed_progress); self.register_limiter_bytes(delta); self.record_adaptive_compression(literal_bytes, compressed_total); - FileCopyOutcome::new(literal_bytes, Some(compressed_total)) + FileCopyOutcome::new(literal_bytes, matched_bytes, Some(compressed_total)) } else { - FileCopyOutcome::new(literal_bytes, None) + FileCopyOutcome::new(literal_bytes, matched_bytes, None) }; Ok(outcome) diff --git a/crates/engine/src/local_copy/context_impl/transfer.rs b/crates/engine/src/local_copy/context_impl/transfer.rs index 2e49a62340..d1985cbf85 100644 --- a/crates/engine/src/local_copy/context_impl/transfer.rs +++ b/crates/engine/src/local_copy/context_impl/transfer.rs @@ -634,7 +634,7 @@ impl<'a> CopyContext<'a> { let progressed = initial_bytes.saturating_add(copied); self.notify_progress(relative, Some(total_size), progressed, start.elapsed()); } - return Ok(FileCopyOutcome::new(copied, None)); + return Ok(FileCopyOutcome::new(copied, MATCHED_NONE, None)); } if sparse { @@ -715,9 +715,9 @@ impl<'a> CopyContext<'a> { let delta = compressed_total.saturating_sub(compressed_progress); self.register_limiter_bytes(delta); self.record_adaptive_compression(literal_bytes, compressed_total); - FileCopyOutcome::new(literal_bytes, Some(compressed_total)) + FileCopyOutcome::new(literal_bytes, MATCHED_NONE, Some(compressed_total)) } else { - FileCopyOutcome::new(literal_bytes, None) + FileCopyOutcome::new(literal_bytes, MATCHED_NONE, None) }; Ok(outcome) @@ -830,9 +830,9 @@ impl<'a> CopyContext<'a> { let delta = compressed_total.saturating_sub(compressed_progress); self.register_limiter_bytes(delta); self.record_adaptive_compression(literal_bytes, compressed_total); - FileCopyOutcome::new(literal_bytes, Some(compressed_total)) + FileCopyOutcome::new(literal_bytes, MATCHED_NONE, Some(compressed_total)) } else { - FileCopyOutcome::new(literal_bytes, None) + FileCopyOutcome::new(literal_bytes, MATCHED_NONE, None) }; Ok(outcome) diff --git a/crates/engine/src/local_copy/executor/file/copy/transfer/execute/clonefile.rs b/crates/engine/src/local_copy/executor/file/copy/transfer/execute/clonefile.rs index 5f4824eb2f..657080f833 100644 --- a/crates/engine/src/local_copy/executor/file/copy/transfer/execute/clonefile.rs +++ b/crates/engine/src/local_copy/executor/file/copy/transfer/execute/clonefile.rs @@ -156,7 +156,9 @@ pub(super) fn try_clone( context.record_hard_link(metadata, destination); context .summary_mut() - .record_file(file_size, file_size, None); + // A whole-file clone/copy is all literal: no signature was + // consulted, so nothing was matched. + .record_file(file_size, file_size, crate::local_copy::MATCHED_NONE, None); context .summary_mut() .record_copy_method(CopyMethodKind::from_platform(clone_method)); diff --git a/crates/engine/src/local_copy/executor/file/copy/transfer/execute/ficlone.rs b/crates/engine/src/local_copy/executor/file/copy/transfer/execute/ficlone.rs index 071edb091b..d32db315a6 100644 --- a/crates/engine/src/local_copy/executor/file/copy/transfer/execute/ficlone.rs +++ b/crates/engine/src/local_copy/executor/file/copy/transfer/execute/ficlone.rs @@ -143,7 +143,9 @@ pub(super) fn try_clone( context.record_hard_link(metadata, destination); context .summary_mut() - .record_file(file_size, file_size, None); + // A whole-file clone/copy is all literal: no signature was + // consulted, so nothing was matched. + .record_file(file_size, file_size, crate::local_copy::MATCHED_NONE, None); context .summary_mut() .record_copy_method(CopyMethodKind::Ficlone); diff --git a/crates/engine/src/local_copy/executor/file/copy/transfer/execute/iouring.rs b/crates/engine/src/local_copy/executor/file/copy/transfer/execute/iouring.rs index 4265a6cb27..2ab2cec604 100644 --- a/crates/engine/src/local_copy/executor/file/copy/transfer/execute/iouring.rs +++ b/crates/engine/src/local_copy/executor/file/copy/transfer/execute/iouring.rs @@ -118,7 +118,9 @@ pub(super) fn try_dispatch( context.record_hard_link(metadata, destination); context .summary_mut() - .record_file(file_size, file_size, None); + // A whole-file clone/copy is all literal: no signature was + // consulted, so nothing was matched. + .record_file(file_size, file_size, crate::local_copy::MATCHED_NONE, None); context .summary_mut() .record_copy_method(CopyMethodKind::IoUring); diff --git a/crates/engine/src/local_copy/executor/file/copy/transfer/execute/mod.rs b/crates/engine/src/local_copy/executor/file/copy/transfer/execute/mod.rs index 95c569bbc2..d5d699d9ec 100644 --- a/crates/engine/src/local_copy/executor/file/copy/transfer/execute/mod.rs +++ b/crates/engine/src/local_copy/executor/file/copy/transfer/execute/mod.rs @@ -675,9 +675,12 @@ pub(in crate::local_copy) fn execute_transfer_once( } let compressed_bytes = outcome.compressed_bytes(); - context - .summary_mut() - .record_file(file_size, outcome.literal_bytes(), compressed_bytes); + context.summary_mut().record_file( + file_size, + outcome.literal_bytes(), + outcome.matched_bytes(), + compressed_bytes, + ); context .summary_mut() .record_copy_method(CopyMethodKind::Standard); diff --git a/crates/engine/src/local_copy/executor/file/copy/transfer/execute/wincopy.rs b/crates/engine/src/local_copy/executor/file/copy/transfer/execute/wincopy.rs index d21e2ca6fb..2be2bf1f06 100644 --- a/crates/engine/src/local_copy/executor/file/copy/transfer/execute/wincopy.rs +++ b/crates/engine/src/local_copy/executor/file/copy/transfer/execute/wincopy.rs @@ -128,7 +128,9 @@ pub(super) fn try_copy( context.record_hard_link(metadata, destination); context .summary_mut() - .record_file(file_size, file_size, None); + // A whole-file clone/copy is all literal: no signature was + // consulted, so nothing was matched. + .record_file(file_size, file_size, crate::local_copy::MATCHED_NONE, None); context .summary_mut() .record_copy_method(CopyMethodKind::from_platform(dispatched_method)); diff --git a/crates/engine/src/local_copy/executor/file/copy/transfer/special.rs b/crates/engine/src/local_copy/executor/file/copy/transfer/special.rs index ba9d544a89..7956fcb7f7 100644 --- a/crates/engine/src/local_copy/executor/file/copy/transfer/special.rs +++ b/crates/engine/src/local_copy/executor/file/copy/transfer/special.rs @@ -83,7 +83,11 @@ pub(in crate::local_copy) fn copy_special_as_regular_file( context.record_hard_link(metadata, hard_link_path); let elapsed = start.elapsed(); - context.summary_mut().record_file(metadata.len(), 0, None); + // A placeholder is written from nothing: no literal bytes off the wire + // and no basis block matched. + context + .summary_mut() + .record_file(metadata.len(), 0, crate::local_copy::MATCHED_NONE, None); context.summary_mut().record_elapsed(elapsed); let metadata_snapshot = LocalCopyMetadata::from_metadata(metadata, None); let total_bytes = Some(metadata_snapshot.len()); diff --git a/crates/engine/src/local_copy/mod.rs b/crates/engine/src/local_copy/mod.rs index c239128d59..ace98cceb4 100644 --- a/crates/engine/src/local_copy/mod.rs +++ b/crates/engine/src/local_copy/mod.rs @@ -109,7 +109,7 @@ pub use skip_compress::{SkipCompressList, SkipCompressParseError}; pub(crate) use compressor::ActiveCompressor; pub(crate) use context::{ CopyContext, CopyOutcome, CreatedEntryKind, DeferredUpdate, FinalizeMetadataParams, - MetadataPathContext, OwnedPathContext, + MATCHED_NONE, MetadataPathContext, OwnedPathContext, }; #[allow(unused_imports)] // REASON: convenience re-export; not all items used in every module diff --git a/crates/engine/src/local_copy/plan/summary.rs b/crates/engine/src/local_copy/plan/summary.rs index ba50a83e5a..3af9f44df2 100644 --- a/crates/engine/src/local_copy/plan/summary.rs +++ b/crates/engine/src/local_copy/plan/summary.rs @@ -783,17 +783,28 @@ impl LocalCopySummary { } } + /// Records one transferred file's contribution to the summary. + /// + /// `matched_bytes` is supplied by the copy path that produced the matches, + /// never inferred here. It used to be derived as `file_size - + /// literal_bytes`, which silently assumed every non-literal byte came from + /// a block match. Append mode falsifies that: the pre-existing prefix is + /// neither literal nor matched, so the derivation counted the whole prefix + /// as matched while upstream reports zero. upstream: match.c:121 grows + /// `stats.matched_data` only inside `matched()`, and append mode never + /// reaches it (match.c:389-390 sets `last_match = s->flength; s->count = + /// 0;` and the hash loop is skipped). pub(in crate::local_copy) fn record_file( &mut self, file_size: u64, literal_bytes: u64, + matched_bytes: u64, compressed: Option, ) { self.files_copied = self.files_copied.saturating_add(1); self.transferred_file_size = self.transferred_file_size.saturating_add(file_size); self.bytes_copied = self.bytes_copied.saturating_add(literal_bytes); - let matched = file_size.saturating_sub(literal_bytes); - self.matched_bytes = self.matched_bytes.saturating_add(matched); + self.matched_bytes = self.matched_bytes.saturating_add(matched_bytes); let transmitted = compressed.unwrap_or(literal_bytes); // A local copy emulates the protocol sender: it writes the file data // (counted as sent) but receives no data payload back. Counting the data @@ -1348,7 +1359,7 @@ mod tests { #[test] fn record_file_increments_counters() { let mut summary = LocalCopySummary::default(); - summary.record_file(1000, 800, None); + summary.record_file(1000, 800, 200, None); assert_eq!(summary.files_copied(), 1); assert_eq!(summary.transferred_file_size(), 1000); @@ -1361,7 +1372,7 @@ mod tests { #[test] fn record_file_with_compression() { let mut summary = LocalCopySummary::default(); - summary.record_file(1000, 800, Some(400)); + summary.record_file(1000, 800, 200, Some(400)); assert_eq!(summary.bytes_copied(), 800); assert_eq!(summary.compressed_bytes(), 400); @@ -1392,8 +1403,8 @@ mod tests { #[test] fn record_multiple_files_accumulates() { let mut summary = LocalCopySummary::default(); - summary.record_file(100, 80, None); - summary.record_file(200, 150, None); + summary.record_file(100, 80, 20, None); + summary.record_file(200, 150, 50, None); assert_eq!(summary.files_copied(), 2); assert_eq!(summary.transferred_file_size(), 300); @@ -1600,7 +1611,7 @@ mod tests { // `bytes_sent` untouched - that data-only figure is what upstream's // `Total bytes sent` is built on for a local copy. let mut summary = LocalCopySummary::default(); - summary.record_file(1_000, 1_000, None); + summary.record_file(1_000, 1_000, 0, None); summary.record_file_list_entry(40); assert_eq!(summary.bytes_sent(), 1_000); diff --git a/crates/engine/src/local_copy/tests/bandwidth.rs b/crates/engine/src/local_copy/tests/bandwidth.rs index 09d45bafa9..542313f97d 100644 --- a/crates/engine/src/local_copy/tests/bandwidth.rs +++ b/crates/engine/src/local_copy/tests/bandwidth.rs @@ -71,7 +71,17 @@ fn execute_with_append_appends_missing_bytes() { assert_eq!(fs::read(&destination).expect("read dest"), b"abcdef"); assert_eq!(summary.bytes_copied(), 3); - assert_eq!(summary.matched_bytes(), 3); + + // Zero, not three. MEASURED against rsync 3.4.4 on this exact fixture + // (`-a --append --ignore-times --stats`, source "abcdef", dest "abc"): + // `Literal data: 3 bytes`, `Matched data: 0 bytes`. + // + // The 3-byte prefix the append skipped is neither literal nor matched. + // Asserting 3 here encoded the old derivation `file_size - literal_bytes`, + // which treats every non-literal byte as matched; upstream only ever + // increments `stats.matched_data` inside `matched()` (match.c:121), which + // append mode never reaches (match.c:389-390). + assert_eq!(summary.matched_bytes(), 0); } #[test] @@ -107,16 +117,14 @@ fn execute_with_append_verify_appends_then_redoes_on_mismatch() { // not match. Asserting 6 would be asserting that the append never happened. assert_eq!(summary.bytes_copied(), 9); - // KNOWN DIVERGENCE, tracked separately: upstream reports 0 matched bytes - // here. Append mode never calls `matched()` (upstream match.c:389-390 sets - // `last_match = s->flength; s->count = 0;` and skips the hash loop), so the - // pre-existing prefix contributes nothing to `stats.matched_data`. oc - // derives matched as `file_size - literal_bytes` in - // local_copy::plan::summary, so pass one's untouched 3-byte prefix falls - // out as "matched". This is pinned at the current value deliberately: when - // the accounting is fixed this assertion fails and points straight at the - // upstream answer above. - assert_eq!(summary.matched_bytes(), 3); + // Zero matched bytes, matching upstream. The 3-byte prefix pass one + // appended past is neither literal nor matched: upstream never calls + // `matched()` in append mode (match.c:389-390 sets + // `last_match = s->flength; s->count = 0;` so the hash loop is skipped), + // and `stats.matched_data` grows nowhere else (match.c:121). Pass two is an + // ordinary delta whose single 6-byte basis block does not match, so it + // contributes nothing either. + assert_eq!(summary.matched_bytes(), 0); } #[test] diff --git a/crates/engine/src/local_copy/tests/execute_append.rs b/crates/engine/src/local_copy/tests/execute_append.rs index 20a9a59e1c..3179ebbc08 100644 --- a/crates/engine/src/local_copy/tests/execute_append.rs +++ b/crates/engine/src/local_copy/tests/execute_append.rs @@ -544,6 +544,27 @@ fn plain_append_never_redoes_even_with_a_mismatching_prefix() { b"01234XXX89ABCDEFGHIJ", "--append keeps the wrong prefix - that is the documented trade-off" ); + + // The prefix an append skips is neither literal nor matched. Upstream + // accumulates `stats.matched_data` only in `matched()` (match.c:121), and + // append mode never reaches it, so upstream reports 0 - MEASURED on this + // shape: `oc -a --append --ignore-times --stats` over a 100 KiB prefix + // gives upstream `Literal 102,400 / Matched 0`. + // + // This is the assertion that catches deriving the figure as + // `file_size - literal_bytes`: that derivation assumes every non-literal + // byte was matched, which is exactly what append falsifies, and it would + // report the whole 10-byte prefix here. + assert_eq!( + summary.bytes_copied(), + 10, + "only the appended tail is literal" + ); + assert_eq!( + summary.matched_bytes(), + 0, + "an appended-over prefix is not matched data" + ); } #[test] From 029e4297d1fc109a184136216cfd285c5762b944 Mon Sep 17 00:00:00 2001 From: Ofer Chen Date: Fri, 7 Aug 2026 01:29:14 +0300 Subject: [PATCH 5/5] fix(engine): probe the basis short final block at its own length The local delta scan reached EOF and probed the basis's trailing short block with whatever the sliding window happened to hold. That probe can essentially never succeed, for two independent reasons. The length. Every failed full-window probe pops one byte, so at EOF the window holds block_length - 1 bytes, while find_tail_match gates each candidate on `block.len() != tail_len` - the same length-equality rule as upstream's `l = MIN(blength, len-offset); if (l != s->sums[i].len) continue;` (match.c:222-224). Only a basis whose final block happened to be exactly block_length - 1 could even reach the strong-sum comparison. The digest. `outgoing` is recorded when a byte is popped but the rolling sum is not corrected until the next push rolls it out. At EOF there is no next push, so the digest still covers the popped byte - describing one more byte than the window holds. Even the coincidental length match above would fail on sum1/sum2. Upstream gets there by shrinking: once `offset + k >= len` there is no next byte, `more` is false and `k` decrements (match.c:321,331), and the scan runs to `end = len + 1 - s->sums[s->count-1].len` (match.c:174) - bounded by the LAST block's length. The window narrows to exactly that block's length, which is the one width the length gate admits. So drain to the basis's final block length explicitly and recompute the sum over exactly those bytes: one probe at the only width that can match, instead of re-probing at every intermediate width. A basis that divides evenly has no short block and stays a clean no-op. What hid this: the window lands on the final block's length, with a digest that agrees, only when a match cleared it with exactly that many bytes left - an identical source, where every full block matches. A fixture built that way passes with or without the fix, so the tests here modify the last full block, which forces EOF to arrive with a full window, and use a 64-bit LCG so no short-period content can let an unaligned window match an earlier block and swallow the tail. MEASURED, local delta over a 292x700+400 basis with the last full block replaced, against rsync 3.4.4: upstream Literal 700 / Matched 204,100; oc was Literal 1,100 / Matched 203,700 and is now 700 / 204,100. Reconstruction was byte-exact throughout - only the split moved. --- .../local_copy/context_impl/delta_transfer.rs | 83 +++++++++-- .../src/local_copy/tests/execute_delta.rs | 134 ++++++++++++++++++ 2 files changed, 207 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/local_copy/context_impl/delta_transfer.rs b/crates/engine/src/local_copy/context_impl/delta_transfer.rs index f37ab4a914..b1024d561f 100644 --- a/crates/engine/src/local_copy/context_impl/delta_transfer.rs +++ b/crates/engine/src/local_copy/context_impl/delta_transfer.rs @@ -304,16 +304,79 @@ impl<'a> CopyContext<'a> { } } - // EOF tail match: the window now holds the file's short trailing bytes - // (fewer than block_length). Upstream rsync matches this short tail - // against the basis's final partial block via `l = MIN(blength, - // len-offset)` (`match.c:222-224`). Mirror that: probe the same-length - // basis block and, on a hit, flush preceding literals then emit the - // short matched block. Without this the trailing partial block is - // always sent as literal data, diverging from upstream deltas. - let tail_matched_block = { - let digest = rolling.digest(); - index.find_tail_match_window(digest, &window, &mut scratch) + // EOF tail match against the basis's short final block. + // + // Upstream reaches this block by shrinking the rolling window: once + // `offset + k >= len` there is no next byte to add, so `more` is false + // and `k` decrements on every step (`match.c:321`, `match.c:331`). Its + // scan runs to `end = len + 1 - s->sums[s->count-1].len` + // (`match.c:174`) - bounded by the LAST block's length, not by + // `blength` - so the window keeps narrowing until it is exactly as long + // as that final short block. Only then does the candidate pass + // `l = MIN(blength, len-offset); if (l != s->sums[i].len) continue;` + // (`match.c:222-224`), because a short block is the only one whose + // recorded length is below `blength`. + // + // Probing whatever the loop happened to leave behind fails twice over. + // + // First, the length. Each failed full-window probe pops one byte, so at + // EOF the window normally holds `block_length - 1` bytes, and + // `find_tail_match` rejects every candidate on `block.len() != tail_len` + // - the same length-equality rule as upstream. Only a basis whose final + // block happens to be exactly `block_length - 1` could even reach the + // strong-sum comparison. + // + // Second, the digest. `outgoing` is recorded when a byte is popped but + // the rolling sum is not corrected until the *next* push rolls it out, + // so after the final pop the digest still covers the popped byte. At EOF + // there is no next push, leaving the digest describing `block_length` + // bytes while the window holds one fewer. Even the coincidental + // length match above would fail on sum1/sum2. + // + // So drain to the basis's final block length EXPLICITLY and recompute + // the sum over exactly those bytes. That is upstream's shrinking window + // expressed as a single probe at the one width that can match, instead + // of re-probing at every intermediate width. + // + // What hid all of this: the window lands on the final block's length, + // with a digest that happens to agree, only when a match `clear()`ed it + // with exactly that many bytes left - an identical source, where every + // full block matches. A fixture built that way passes either way. + let short_tail_len = index + .block_count() + .checked_sub(1) + .map(|last| index.block(last).len()) + .filter(|&len| len > 0 && len < index.block_length()); + let tail_matched_block = match short_tail_len { + // A basis whose length is an exact multiple of the block length has + // no short final block, so upstream has nothing extra to offer and + // this is a clean no-op. + None => None, + Some(tail_len) => { + while window.len() > tail_len { + if let Some(front) = window.pop_front() { + pending_literals.push(front); + } + } + // A source shorter than the final basis block never reaches the + // probe: upstream's `end` would be non-positive and its scan + // would not run either. + if window.len() == tail_len { + // Recompute rather than adjust: the digest carried out of + // the loop covers neither the pre-drain window nor the + // post-drain one (see the stale-`outgoing` note above), so + // there is no increment that would repair it. + rolling.reset(); + let (first, second) = window.as_slices(); + rolling.update(first); + if !second.is_empty() { + rolling.update(second); + } + index.find_tail_match_window(rolling.digest(), &window, &mut scratch) + } else { + None + } + } }; if let Some(block_index) = tail_matched_block { // A matched trailing partial block is one confirmed match, and its diff --git a/crates/engine/src/local_copy/tests/execute_delta.rs b/crates/engine/src/local_copy/tests/execute_delta.rs index 0e57b59400..e2ab5d0ab5 100644 --- a/crates/engine/src/local_copy/tests/execute_delta.rs +++ b/crates/engine/src/local_copy/tests/execute_delta.rs @@ -135,3 +135,137 @@ fn execute_with_report_records_min_size_skip_notice_without_copying() { assert_eq!(report.summary().regular_files_total(), 1); assert_eq!(report.summary().bytes_copied(), 0); } + +/// Deterministic pseudo-random bytes from a 64-bit LCG (Knuth's MMIX +/// constants), taking the high bits so consecutive outputs do not share low-bit +/// structure. +/// +/// The generator matters as much as the sizes here. A short-period or repeating +/// fixture lets an unaligned window match some earlier full block purely by +/// content, which swallows the tail and makes the test assert about the data +/// rather than about the matcher. This period is far longer than any fixture +/// below. +fn lcg_bytes(len: usize, seed: u64) -> Vec { + let mut state = seed; + (0..len) + .map(|_| { + state = state + .wrapping_mul(6_364_136_223_846_793_005) + .wrapping_add(1_442_695_040_888_963_407); + (state >> 33) as u8 + }) + .collect() +} + +/// Runs one local delta copy whose basis is `full_blocks` whole blocks plus +/// `tail` trailing bytes, with the source's LAST FULL block replaced. +/// +/// Returns `(matched_bytes, literal_bytes)`. +/// +/// Modifying the last full block is what makes the fixture load-bearing. It +/// stops the scan matching block-aligned right up to the end, so the loop +/// slides byte-by-byte and reaches EOF holding `block - 1` bytes - a full +/// window. An identical source instead has a match clear the window exactly +/// `tail` bytes early, which is the reachability accident that let the tail +/// probe look correct: such a fixture passes with or without the fix. +fn delta_tail_fixture(block: usize, full_blocks: usize, tail: usize) -> (u64, u64, bool) { + let temp = tempdir().expect("tempdir"); + let source_path = temp.path().join("source.bin"); + let dest_path = temp.path().join("dest.bin"); + + let len = full_blocks * block + tail; + let basis = lcg_bytes(len, 0x5DEE_CE66_D1CE_4B9D); + let mut source = basis.clone(); + let modified_start = (full_blocks - 1) * block; + source[modified_start..modified_start + block] + .copy_from_slice(&lcg_bytes(block, 0x0BAD_C0DE_0BAD_C0DE)); + + fs::write(&dest_path, &basis).expect("write basis"); + set_file_mtime(&dest_path, FileTime::from_unix_time(1, 0)).expect("dest mtime"); + fs::write(&source_path, &source).expect("write source"); + set_file_mtime(&source_path, FileTime::from_unix_time(2, 0)).expect("source mtime"); + + let operands = vec![ + source_path.into_os_string(), + dest_path.clone().into_os_string(), + ]; + let plan = LocalCopyPlan::from_operands(&operands).expect("plan"); + let block_size = NonZeroU32::new(block as u32).expect("block size is non-zero"); + let summary = plan + .execute_with_options( + LocalCopyExecution::Apply, + LocalCopyOptions::default() + .whole_file(false) + .with_block_size_override(Some(block_size)), + ) + .expect("delta copy succeeds"); + + let exact = fs::read(&dest_path).expect("read dest") == source; + (summary.matched_bytes(), summary.bytes_copied(), exact) +} + +#[test] +fn delta_matches_the_basis_short_final_block_at_a_full_eof_window() { + // 292 x 700 + 400, the shape upstream was measured on. The 400-byte + // remainder is the only basis block whose recorded length is below the + // block length, so it is the only one that can satisfy upstream's + // `l = MIN(blength, len-offset); if (l != s->sums[i].len) continue;` + // (match.c:222-224) - and only at the file's own end, because upstream's + // scan is bounded by that block's length + // (`end = len + 1 - s->sums[s->count-1].len`, match.c:174) and its window + // shrinks to reach it (match.c:321,331). + // + // WHY THIS ASSERTION AND NOT BYTE-EQUALITY: reconstruction is byte-exact + // either way. The defect is that the trailing block is re-sent as literal + // data that upstream matches, so only the literal/matched split can see it. + let (matched, literal, exact) = delta_tail_fixture(700, 292, 400); + + assert!(exact, "reconstruction must be byte-exact"); + // 291 untouched full blocks (203,700) plus the 400-byte tail. + assert_eq!( + matched, 204_100, + "the basis's short final block must be matched, not re-sent" + ); + // Only the one modified full block is literal. Before the fix this was + // 1,100: the 700-byte modified block plus the 400-byte tail. + assert_eq!(literal, 700, "only the modified block may be literal"); +} + +#[test] +fn delta_tail_match_handles_a_single_byte_final_block() { + // tail_len == 1 is the narrowest short block that exists. zsync 0.6 fixed + // an out-of-bounds access "when processing the last block of a + // non-compressed download", so the degenerate widths are exactly where this + // family of implementations has gone wrong before. + let (matched, literal, exact) = delta_tail_fixture(700, 4, 1); + + assert!(exact, "reconstruction must be byte-exact"); + assert_eq!(matched, 3 * 700 + 1, "a one-byte final block still matches"); + assert_eq!(literal, 700); +} + +#[test] +fn delta_tail_match_handles_a_final_block_one_byte_short() { + // tail_len == block_length - 1 is the widest short block. It is the width + // the EOF window itself happens to hold, so a probe that used "whatever is + // left in the window" would pass here while failing every other width - + // this pins that the probe length comes from the basis, not from the + // window. + let (matched, literal, exact) = delta_tail_fixture(700, 4, 699); + + assert!(exact, "reconstruction must be byte-exact"); + assert_eq!(matched, 3 * 700 + 699); + assert_eq!(literal, 700); +} + +#[test] +fn delta_tail_probe_is_a_no_op_when_the_basis_has_no_short_block() { + // An exact multiple has no final short block, so upstream has nothing extra + // to offer and the probe must not fire at all. If it degraded to matching + // "whatever is left in the window" it could emit a spurious short Copy here. + let (matched, literal, exact) = delta_tail_fixture(700, 4, 0); + + assert!(exact, "reconstruction must be byte-exact"); + assert_eq!(matched, 3 * 700, "no tail block exists to match"); + assert_eq!(literal, 700, "only the modified block is literal"); +}