Repository navigation
Make unified and patch interoperate with diff(1), patch(1) and git - #16
Conversation
unified-with-context now names the line before a zero-count range, as GNU diff, git and difflib do (`@@ -3,0 +4,1 @@`, `-0,0`, `+0,0`), and ends the last hunk after ctx lines of context instead of up to 2*ctx. GNU patch applied the old ctx-0 output to the wrong place without error. patch now parses the whole hunk header (`@@ -a[,b] +c[,d] @@ section`), puts a hunk whose old count is 0 after line a, and consumes exactly b old and d new lines per hunk. A truncated hunk, a line that overruns a count, a malformed header or a second file's `---`/`+++` header gives Nothing. Lines before the first hunk and after a hunk's counts are skipped, so real `diff -u` and `git diff` output applies. Counting keeps `--- x` inside a hunk a deletion of `-- x`. A stray line inside a hunk body is now Nothing, as it is for GNU patch and git.
There was a problem hiding this comment.
Build & Tests
carp -x tests/diff.carpon the head (compat core, armhf): 114 passed, 0 failed. The new test file against master'sdiff.carp: 99 passed, 15 failed, and the 15 are exactly the ones the body lists.- CI is green on ubuntu and macOS, including angler and carp-fmt.
- A
--log-memoryprobe balances to 0. It runs 200 rounds ofpatchon headered, malformed, second-file and truncated input, plusunified-with-contextat ctx 0 and 3. A held allocation used as a control reads +9.
Findings
I built my own corpus with GNU diff 3.8, GNU patch 2.7.6 and git 2.39.5 as oracles. It has 300 pairs per seed, with lines drawn from a–e, "", " ", -- x, --- v, +++ u, @@ w, \ q, -, + and a few others.
- Tool output →
Diff.patch.diff -U0/-U1/-U3andgit diff --no-index -U0/-U1/-U3output, headers included, on newline-terminated files: 238/238 for each of the six. Master returnsNothingfor all of them. - Library output → tools.
unified-with-contextofDiff.diffover the files' lines, applied by GNU patch and bygit apply(--unidiff-zeroat ctx 0): 249/249 at ctx 0, 1 and 3 with both tools. On master, GNU patch put 92 of the 249 ctx-0 patches in the wrong place and still exited 0. - Hunk boundaries. I compared pairs where the library and GNU diff choose the same edit script: 854–861 of 963 pairs per ctx, ctx 0–3, including gaps of 0 to 2·ctx+2 equal lines between two changes. Every hunk header matches GNU diff's. On master, 106–485 per ctx differ.
- Round trip.
patch old (unified-with-context (line-diff old new) ctx)=Just newfor 12,000/12,000: 1,200 pairs at ctx 0/1/2/3/5, with and without a final newline. - No regressions except item 2. I checked per case on the library→tools runs and on 9,282 header-stripped
diff/git diffpatches (newline-terminated, no final newline, and mixed). No case master gets right is wrong on the head. - LP64. An aarch64 build of the same C prints byte-identical output on every case file, including
@@ -4294967298 …headers. - The body's "master printed" values check out:
@@ -1,6 +1,6 @@with 5 trailing lines,Just "a\nc"for the truncated hunk, and-1,0/+1,0.
None of the following blocks a merge.
1. Several of the applier's new rules have no test. I wrote 18 mutants of the new code, none of them on the body's list. 15 survive the suite. Some are harmless: the sign check duplicates the non-negative check, and a few only let odd headers like @@@ -1 +1 @@ through. These five give a silently wrong answer:
parse-numwithout theInt.strround trip (diff.carp:339):@@ -02 +02 @@applies. On LP64,@@ -4294967298 +4294967298 @@\n-b\n+Bwraps to line 2 and givesJust "a\nB".- No non-negative check in
parse-hunk-header(diff.carp:357):@@ -1,x +1,x @@\n-a\n+Abecomes a no-op,Just "a\nb". - No out-of-order check (
diff.carp:426):@@ -3 …followed by@@ -1 …ona b cgivesJust "a\nb\nC\nA\nb\nc". - Deleted lines not compared (
diff.carp:404):@@ -2 +2 @@\n-x\n+Bona bgivesJust "a\nB". - The body's rule that lines after a hunk's counts are used up get skipped (
diff.carp:440): making them fail instead goes unnoticed, e.g. aformat-patchsignature after the last hunk.
The out-of-order and deletion checks predate this PR, but it rewrites both lines. Each of these rows passes on the head and fails on its mutant. The last two also pin the zero-count range check (without it, unsafe-nth aborts) and the old count on context lines:
(assert-equal test &(the (Maybe String) (Maybe.Nothing)) &(patch "a\nb" "@@ -02 +02 @@\n-b\n+B") "patch returns Nothing on a zero-padded hunk number")
(assert-equal test &(the (Maybe String) (Maybe.Nothing)) &(patch "a\nb" "@@ -1,x +1,x @@\n-a\n+A") "patch returns Nothing on a non-numeric hunk count")
(assert-equal test &(the (Maybe String) (Maybe.Nothing)) &(patch "a\nb\nc" "@@ -3 +3 @@\n-c\n+C\n@@ -1 +1 @@\n-a\n+A") "patch returns Nothing on out-of-order hunks")
(assert-equal test &(the (Maybe String) (Maybe.Nothing)) &(patch "a\nb" "@@ -2 +2 @@\n-x\n+B") "patch returns Nothing when a deleted line does not match")
(assert-equal test &(Maybe.Just @"A\nb") &(patch "a\nb" "@@ -1 +1 @@\n-a\n+A\n-- \n2.39.5\n") "patch skips lines after a hunk's counts are used up")
(assert-equal test &(the (Maybe String) (Maybe.Nothing)) &(patch "a\nb" "@@ -3,0 +4 @@\n+X") "patch returns Nothing on a zero-count hunk past the end")
(assert-equal test &(the (Maybe String) (Maybe.Nothing)) &(patch "a\nb" "@@ -1,1 +1,2 @@\n a\n b") "patch returns Nothing on a hunk with too many old context lines")
With all seven rows the suite is 121/0 on the head. Still unpinned: the bounds checks in old-line? and in the +++ lookahead, each of which guards an unsafe-nth past the end.
2. An empty line inside a hunk ends the hunk (diff.carp:416). diff -u --suppress-blank-empty, git's diff.suppressBlankEmpty, and editors or mail clients that strip trailing whitespace all write an empty context line as an empty line, not a single space. GNU patch and git apply read it as an empty context line. The head returns Nothing:
- 48 of 238
-U3patches and 22 of 238-U1patches come backNothing, and GNU patch and git apply apply every one of them correctly. - It fails closed, but it is a small regression for header-less patches whose only empty context line is the last one. Master got those right by accident (8 cases).
(patch "a\n\nb\n" "@@ -1,2 +1,2 @@\n-a\n+A\n\n")isJust "A\n\nb\n"on master andNothinghere. - I tried an 11-line fix: inside a hunk, read
""as an empty context line, except the last element, whichsplit-byleaves after the final newline. With it, the patches for all 495 pairs of a larger corpus apply (GNU at ctx 0/1/3, git at 3), a truncated hunk is stillNothing, and the suite stays at 114/0.
3. line-diff output still trips git. The renderer now matches GNU diff for Diff.diff over a file's lines. The documented (unified (line-diff old new)) on newline-terminated text is how a file is normally read, and there the empty string after the final newline counts as a line and shows up in hunks:
(unified (line-diff "a\nb\nc\n" "a\nb\nC\n"))is@@ -1,4 +1,4 @@\n a\n b\n+C\n-c\n \n.git applyrejects it ("patch does not apply") and GNU patch needs fuzz 2.- On my corpus, git apply rejects 121/249 such patches at ctx 1 and 173/249 at ctx 3 (master: 152 and 211). GNU patch needs fuzz on 90 and 126.
- At ctx 0 nothing catches it. GNU patch and
git apply --unidiff-zeroboth put 15/249 of these patches in the wrong place and exit 0 (master: 99 for GNU patch, the same 15 for git). For example,e\n→a \n\n@@\ngives@@ -1,1 +1,1 @@\n+a \n-e\n@@ -2,0 +3,2 @@\n+@@\n+\n. Line 2 is the phantom line, so both tools append at the end and producea \n@@\n\n.
The head is better than master in every cell, so this is not a regression. But the title's git interop holds for Diff.diff over lines, not for line-diff on files, and the body doesn't say which of the two the 897/897 used. The follow-up would be to drop that trailing element and write \ No newline at end of file. It is the same split-model gap as item 4.
4. A changed final newline comes back as Just with the wrong ending. The body says the marker is skipped. In my corpus, 259 pairs have files that differ in their final newline (diff -U0/-U3 and git diff, with or without headers). For 249 of them the head returns Just with the wrong ending, not Nothing, and the other 10 come out right. Another 8 wrong results are the empty-original case the body already names. With headers, master returned Nothing for these; without headers, it also returned a wrong Just. The patch docstring (diff.carp:371) still calls it "a safe applier" that returns Nothing when a patch does not apply. One sentence there about the marker would keep it accurate.
5. Patches written by master at ctx 0. As the body says, a zero-count range written by master's unified now lands one line later. I applied master's ctx-0 output for my 249 pairs (line-diff, newline-terminated) with the head:
- 104 land in the wrong place, silently, as
Just; - 21 return
Nothing; - 124 come out right.
Master's ctx-1 and ctx-3 output still applies 249/249. This is worth a line in the release notes for anyone who stored patches.
Verdict: merge
For Diff.diff output and for real diff -u and git diff files, the renderer and applier now agree with GNU diff, GNU patch and git on every corpus I ran, and nothing regresses against master outside item 2. The rows in item 1 are cheap to add. Items 2–5 are follow-ups or notes.
unifiedandpatchnow read and write the unified format the way GNU diff, GNU patch and git do, in both directions. Everything is indiff.carp, in pure Carp.Renderer
@@ -3,0 +4,1 @@(master printed-4,0). Deleting line 4 renders@@ -4,1 +3,0 @@. An empty old side is-0,0and an empty new side is+0,0(master printed-1,0and+1,0). Both counts are still always printed, and insertions still come before deletions inside a hunk.ctxlines of trailing context. Master kept up to2*ctx:[a b c d e f]→[X b c d e f]at ctx 3 is now@@ -1,4 +1,4 @@with 3 trailing lines (master:@@ -1,6 +1,6 @@with 5).Applier
3. The hunk header is parsed in full:
@@ -a[,b] +c[,d] @@, optionally followed by section text. An omitted count means 1. Numbers must be plain non-negative decimals, and anything else isNothing, including a header without its closing@@(master accepted that). A hunk with old count 0 now goes after lineainstead of before it. The out-of-range and out-of-order checks are unchanged.4.
patchnow uses exactlybold lines anddnew lines per hunk: a space line counts on both sides,-on the old side,+on the new side, and\ No newline at end of filecounts on neither. A hunk isNothingif it ends early (end of text, the next@@, or any other line) or if a line overruns a count. Master half-applied a truncated hunk:@@ -1,3 +1,3 @@/a/-bona b cgaveJust "a\nc". The following are skipped, as GNU patch and git also skip them:diff --git,index,---/+++, free text);@@.Because of this, real
diff -uandgit diffoutput now applies; master returnedNothingfor all of it. A---/+++pair after a hunk starts a second file and givesNothing, becausepatchapplies to one document. Inside a hunk,--- xis still the deletion of a line-- x.The doc string of
patchnow says it takes single-file diffs fromunified,diff -uorgit diff, and lists what it rejects.Consequences
unifiedwith a zero-count range is now read the standard way, i.e. placed one line later. Such ranges only occur at ctx 0 or with an empty side.Just "a\nb"toNothing: a strayéor0xBBline inside a hunk body is now malformed, as it is for GNU patch and git. Both assertions still check thatpatchdoes not abort on those bytes, which is what Fix process abort in Diff.patch on a single multi-byte character line #13 added them for.\ No newline at end of fileis still skipped, as on master, so a patch that only adds or removes the final newline does not do so.Verification (Carp compat core, GNU patch 2.7.6, GNU diff 3.8, git 2.39.5)
---/+++inside a hunk, the no-newline marker, a second file, and the ctx-0 round trip.abcd, lengths 0–20): ctx 0/1/3 gives 897/897 correct. On master, GNU patch applied 112 of the 299 ctx-0 patches to the wrong place, with exit code 0.diff -U0/-U1/-U3(with file headers) andgit diff --no-index -U0/-U3, applied byDiff.patchon the same pairs written as newline-terminated files: 1495/1495 correct. Master returnedNothingfor all 1495. With the file headers stripped, master gave a wrong document for 145 of 299diff -U0patches and 144 of 299git diff -U0patches.""splits into one empty line, so the result gains a final newline. This is the newline limitation above.patch round-trips …row.truncated hunk.+or-line ignores its count:too many context lines,too many + linesandtoo many - linesrespectively.\marker rejected:skips no-newline markers.a:zero-count hunk after the line it names,appends a zero-count hunk at the endandround-trips unified-with-context 0.second file.---/+++skipped inside a hunk:--- and +++ inside a hunk.does not abortrows.count-omitted hunk headerand 4 others.@@not checked:header without its closing @@.zero-count new rangeand+0,0.2*ctx:ends the last hunk after ctx context lines.patch old (unified-with-context (line-diff old new) ctx) = Just newheld for 1200 random document pairs at ctx 0/1/3. The documents mixed in empty lines and lines like--x,++yandz.Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.