fix: detect already-applied diffs via post-image matching - #20
Merged
Conversation
The reverse round-trip heuristic in is_diff_applied_with_config misclassified deletion-only patches as already applied: reversing a deletion yields an insertion diff, which applies to the *un*patched content just as well (duplicating the still-present lines) and round-trips cleanly back to the input. Combined with fuzzy per-line matching this caused rattler-build to skip valid patches (prefix-dev/rattler-build#2693). Instead, consider a diff already applied only when every hunk's post-image (context + inserted lines) occurs contiguously in the base image, compared strictly (whitespace/case normalization per config, but no similarity threshold and no fuzz). This mirrors GNU patch's 'previously applied' detection and has neither the forward-apply nor the reverse-round-trip failure mode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 task
This was referenced Jul 28, 2026
Merged
wolfv
added a commit
that referenced
this pull request
Jul 30, 2026
A hunk that only deletes lines, with its deletions at the hunk's edge (e.g. leading context and no trailing context), has a post-image that consists solely of its context lines - which the *un*patched file also contains contiguously. The post-image search from #20 therefore reported such hunks as already applied, and apply_bytes_reporting silently skipped them: exit code 0, no diagnostic, no change. This is how php-feedstock's 0001-win-iconv-compat.patch broke under rattler-build 0.70+: the hunk emptying ext/iconv/php_iconv.def (one EXPORTS context line followed by deletions to EOF) vanished while the other hunks applied, leaving stale .def exports and an LNK2001 on _libiconv_version. For a hunk with no insertions, now additionally require that its pre-image (context plus deleted lines) is absent from the file before declaring it already applied. Hunks with insertions are unaffected: their post-image contains lines the unpatched file doesn't have.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
is_diff_applied_with_configused a reverse round-trip heuristic: apply the reversed diff, check the result differs, re-apply forward, check it round-trips back. This is unsound for deletion-heavy diffs: reversing a deletion yields an insertion diff, which applies to the unpatched content just as well — it simply duplicates the still-present lines — and the forward diff deletes them again, round-tripping perfectly. The check therefore reported deletion-only patches as "already applied" on files they were never applied to.This was compounded by the per-line fuzzy comparison (Levenshtein similarity > 0.8, whitespace ignored), which let e.g.
add_compile_options(/MT)match the context lineadd_compile_options(/utf-8)at 0.85 similarity, so the reversed insertion found a landing spot even where the context didn't really match.Downstream this caused rattler-build to skip valid patches with a bogus "appears to be already applied" warning: prefix-dev/rattler-build#2693 (libddwaf in conda-forge staged-recipes), and previously conda-forge/intel-level-zero-npu-feedstock#7.
Fix
Detect "already applied" the way GNU patch does: a diff counts as already applied only if every hunk's post-image (its context lines plus its inserted lines, contiguous and in order) is actually found in the base image. Lines are compared strictly — whitespace/case normalization per the config still applies, but no similarity threshold and no fuzz. The search starts at the hunk's header position and scans outward, like the pre-image search.
apply_bytes_reportingfalls through to a forward apply.Verified against the real-world reproducer from the issue (libddwaf 1.30.1 tarball +
win-rt.patch): previously misreported asAlreadyApplied; now reportsFailed, matching GNUpatch(that patch's trailing context references a line that only exists in newer upstream, so it genuinely does not apply).Tests
test_is_diff_applied_pure_deletion: deletion-only patch →Appliedon pristine content,AlreadyAppliedon patched content.test_deletion_patch_with_stale_context_is_not_already_applied: distilled libddwaf case →Failed, neverAlreadyApplied.🤖 Generated with Claude Code