From 10699c4d4124c0053069d58c18b4ab431bc5c6e7 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Sat, 4 Jul 2026 10:08:10 +0200 Subject: [PATCH] feat: add is_diff_applied_with_config and apply_bytes_reporting Add primitives to detect whether a diff is already applied to a base image, so callers (e.g. rattler-build) can warn-and-skip instead of silently re-applying a merged-upstream patch. - `is_diff_applied_with_config(base, diff, config) -> bool`: robust reverse round-trip check. - `apply_bytes_reporting(base, diff, config) -> ApplyOutcome`: apply, reporting Applied / AlreadyApplied / Failed. Both are robust under fuzzy matching, where a forward apply of an already-applied diff is unreliable: it may fail (a deleted line no longer matches) or succeed while wrongly re-applying the change (e.g. duplicating an inserted line). The already-applied case is therefore detected via a reverse round-trip, never via a forward no-op. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/apply.rs | 305 ++++++++++++++++++++++++++++++++++++++++++++++++++- src/lib.rs | 5 +- 2 files changed, 307 insertions(+), 3 deletions(-) diff --git a/src/apply.rs b/src/apply.rs index 4477e6a..a1edf3b 100644 --- a/src/apply.rs +++ b/src/apply.rs @@ -363,6 +363,134 @@ pub fn apply_bytes(base_image: &[u8], patch: &Diff<'_, [u8]>) -> ApplyResult, + config: &ApplyConfig, +) -> bool { + // Reverse round-trip: reversing an already-applied diff must produce a + // *different* pre-image that, patched forward again, reproduces the input. + match apply_bytes_with_config(base_image, &diff.reverse(), config) { + Ok((pre, _)) if pre != base_image => apply_bytes_with_config(&pre, diff, config) + .map(|(re, _)| re == base_image) + .unwrap_or(false), + _ => false, + } +} + +/// The outcome of attempting to apply a diff with [`apply_bytes_reporting`]. +/// +/// This distinguishes the three cases a caller usually cares about: the diff +/// was applied and changed the content, the diff appears to be already applied +/// (so applying it would be a no-op), or the diff does not apply and is not +/// already applied. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ApplyOutcome { + /// The diff was applied and modified the base image. Contains the patched + /// image together with the [`ApplyStats`] from the application. + Applied(Vec, ApplyStats), + /// The diff appears to be already applied: `base_image` already reflects the + /// modified side of the diff. Contains the (unchanged) base image. + /// + /// This is detected robustly even under fuzzy matching, where a forward + /// apply of an already-applied diff succeeds as a no-op. + AlreadyApplied(Vec), + /// The diff could not be applied and is not already applied. Contains the + /// [`ApplyError`] from the failed forward application. + Failed(ApplyError), +} + +/// Apply a non-utf8 `Diff` to a base image, reporting whether it was applied, +/// was already applied, or failed. +/// +/// This is a convenience wrapper around [`is_diff_applied_with_config`] and +/// [`apply_bytes_with_config`] that answers "apply this, but if it is already +/// applied tell me so instead of applying it again". It checks for the +/// already-applied case *first* (via a reverse round-trip) and only forward +/// applies when the diff is genuinely not yet applied. This ordering is +/// necessary under fuzzy matching, where a forward apply of an already-applied +/// diff is unreliable — it may fail, or it may succeed while wrongly +/// re-applying the change (e.g. duplicating an inserted line). +/// +/// # Examples +/// +/// ``` +/// use flickzeug::{apply_bytes_reporting, ApplyConfig, ApplyOutcome, Diff}; +/// +/// let patch = "\ +/// --- a/version +/// +++ b/version +/// @@ -1 +1 @@ +/// -3.1 +/// +3.12 +/// "; +/// let diff = Diff::from_bytes(patch.as_bytes()).unwrap(); +/// let config = ApplyConfig::default(); +/// +/// // Fresh content: the diff applies and changes it. +/// assert!(matches!( +/// apply_bytes_reporting(b"3.1\n", &diff, &config), +/// ApplyOutcome::Applied(..) +/// )); +/// // Already-patched content: reported as already applied. +/// assert!(matches!( +/// apply_bytes_reporting(b"3.12\n", &diff, &config), +/// ApplyOutcome::AlreadyApplied(_) +/// )); +/// ``` +pub fn apply_bytes_reporting( + base_image: &[u8], + diff: &Diff<'_, [u8]>, + config: &ApplyConfig, +) -> ApplyOutcome { + // Check for the already-applied case first: a forward apply cannot be + // trusted to detect it (it may fail, or succeed while re-applying the + // change), so we must not forward apply until we know the diff is not + // already applied. + if is_diff_applied_with_config(base_image, diff, config) { + return ApplyOutcome::AlreadyApplied(base_image.to_vec()); + } + + match apply_bytes_with_config(base_image, diff, config) { + Ok((patched, stats)) => ApplyOutcome::Applied(patched, stats), + Err(err) => ApplyOutcome::Failed(err), + } +} + /// Apply a non-utf8 `Diff` to a base image with custom fuzzy matching configuration pub fn apply_bytes_with_config( base_image: &[u8], @@ -819,7 +947,10 @@ where mod test { use std::path::PathBuf; - use crate::{Diff, apply}; + use crate::{ + ApplyConfig, ApplyOutcome, Diff, FuzzyConfig, apply, apply_bytes_reporting, + is_diff_applied_with_config, + }; fn load_files(name: &str) -> (String, String) { let base_folder = PathBuf::from(env!("CARGO_MANIFEST_DIR")) @@ -1040,4 +1171,176 @@ mod test { .join("\n"); insta::assert_snapshot!(relevant_lines); } + + /// rattler-build's configuration: fuzzy matching enabled, which is exactly + /// the case where a naive forward-apply check misclassifies an + /// already-applied diff (the forward apply succeeds as a no-op). + fn fuzzy_config() -> ApplyConfig { + ApplyConfig { + fuzzy_config: FuzzyConfig { + max_fuzz: 2, + ignore_whitespace: true, + ignore_case: false, + }, + ..Default::default() + } + } + + #[test] + fn test_is_diff_applied_line_replacement() { + let patch = "\ +--- a/version ++++ b/version +@@ -1,3 +1,3 @@ + line 1 +-3.1 ++3.12 + line 3 +"; + let diff = Diff::from_bytes(patch.as_bytes()).unwrap(); + let config = fuzzy_config(); + + let pre = b"line 1\n3.1\nline 3\n"; + let post = b"line 1\n3.12\nline 3\n"; + + // Not applied to the pre-image. + assert!(!is_diff_applied_with_config(pre, &diff, &config)); + // Already applied to the post-image. + assert!(is_diff_applied_with_config(post, &diff, &config)); + } + + #[test] + fn test_is_diff_applied_pure_insertion() { + // A hunk with only context + inserted lines (no removed lines). This is + // the case most likely to fool a forward-based check, because inserting + // already-present lines under fuzzy matching can look like a no-op. + let patch = "\ +--- a/list ++++ b/list +@@ -1,3 +1,4 @@ + first + second ++inserted + third +"; + let diff = Diff::from_bytes(patch.as_bytes()).unwrap(); + let config = fuzzy_config(); + + let pre = b"first\nsecond\nthird\n"; + let post = b"first\nsecond\ninserted\nthird\n"; + + assert!(!is_diff_applied_with_config(pre, &diff, &config)); + assert!(is_diff_applied_with_config(post, &diff, &config)); + } + + #[test] + fn test_is_diff_applied_unrelated_content() { + let patch = "\ +--- a/version ++++ b/version +@@ -1 +1 @@ +-3.1 ++3.12 +"; + let diff = Diff::from_bytes(patch.as_bytes()).unwrap(); + let config = fuzzy_config(); + + // Neither the pre- nor the post-image: not applied. + assert!(!is_diff_applied_with_config( + b"something else entirely\n", + &diff, + &config + )); + } + + #[test] + fn test_forward_reapply_is_unreliable_but_classifier_is_correct() { + // Under fuzzy matching a forward re-apply of an already-applied diff is + // an unreliable signal: depending on the hunk shape it either fails or + // succeeds while wrongly re-applying the change. The classifier must be + // correct regardless. + let config = fuzzy_config(); + + // Line replacement: forward re-apply FAILS (the `-3.1` delete line no + // longer matches the already-patched `3.12`). + let replace = "\ +--- a/version ++++ b/version +@@ -1,3 +1,3 @@ + line 1 +-3.1 ++3.12 + line 3 +"; + let replace_diff = Diff::from_bytes(replace.as_bytes()).unwrap(); + let replace_post = b"line 1\n3.12\nline 3\n"; + assert!(crate::apply_bytes_with_config(replace_post, &replace_diff, &config).is_err()); + assert!(is_diff_applied_with_config( + replace_post, + &replace_diff, + &config + )); + + // Pure insertion: forward re-apply SUCCEEDS but wrongly re-inserts the + // already-present line, changing the content. + let insert = "\ +--- a/list ++++ b/list +@@ -1,3 +1,4 @@ + first + second ++inserted + third +"; + let insert_diff = Diff::from_bytes(insert.as_bytes()).unwrap(); + let insert_post = b"first\nsecond\ninserted\nthird\n"; + let (reapplied, _) = + crate::apply_bytes_with_config(insert_post, &insert_diff, &config).unwrap(); + assert_ne!(&reapplied[..], &insert_post[..]); + assert!(is_diff_applied_with_config( + insert_post, + &insert_diff, + &config + )); + } + + #[test] + fn test_apply_bytes_reporting_outcomes() { + let patch = "\ +--- a/version ++++ b/version +@@ -1,3 +1,3 @@ + line 1 +-3.1 ++3.12 + line 3 +"; + let diff = Diff::from_bytes(patch.as_bytes()).unwrap(); + let config = fuzzy_config(); + + let pre = b"line 1\n3.1\nline 3\n"; + let post = b"line 1\n3.12\nline 3\n"; + + // Fresh content: applied and changed. + match apply_bytes_reporting(pre, &diff, &config) { + ApplyOutcome::Applied(content, stats) => { + assert_eq!(content, post); + assert!(stats.has_changes()); + } + other => panic!("expected Applied, got {other:?}"), + } + + // Already-patched content: reported as already applied (not a no-op + // "Applied", and not "Failed"). + match apply_bytes_reporting(post, &diff, &config) { + ApplyOutcome::AlreadyApplied(content) => assert_eq!(content, post), + other => panic!("expected AlreadyApplied, got {other:?}"), + } + + // Unrelated content: cannot apply and is not already applied. + match apply_bytes_reporting(b"totally different\n", &diff, &config) { + ApplyOutcome::Failed(_) => {} + other => panic!("expected Failed, got {other:?}"), + } + } } diff --git a/src/lib.rs b/src/lib.rs index 50da247..c678734 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -229,8 +229,9 @@ mod range; mod utils; pub use apply::{ - ApplyConfig, ApplyError, ApplyResult, ApplyStats, FuzzyConfig, LineEndHandling, apply, - apply_bytes, apply_bytes_with_config, apply_with_config, + ApplyConfig, ApplyError, ApplyOutcome, ApplyResult, ApplyStats, FuzzyConfig, LineEndHandling, + apply, apply_bytes, apply_bytes_reporting, apply_bytes_with_config, apply_with_config, + is_diff_applied_with_config, }; pub use diff::{DiffOptions, create_patch, create_patch_bytes}; pub use line_end::*;