Skip to content

Make unified and patch interoperate with diff(1), patch(1) and git - #16

Merged
hellerve merged 1 commit into
masterfrom
claude/unified-interop
Nov 1, 2026
Merged

hellerve merged 1 commit into
masterfrom
claude/unified-interop

Conversation

@carpentry-agent

Copy link
Copy Markdown

unified and patch now read and write the unified format the way GNU diff, GNU patch and git do, in both directions. Everything is in diff.carp, in pure Carp.

Renderer

  1. When a range has a count of 0, the header now names the line before it, as GNU diff, git and difflib do. Inserting X after line 3 at ctx 0 renders @@ -3,0 +4,1 @@ (master printed -4,0). Deleting line 4 renders @@ -4,1 +3,0 @@. An empty old side is -0,0 and an empty new side is +0,0 (master printed -1,0 and +1,0). Both counts are still always printed, and insertions still come before deletions inside a hunk.
  2. The last hunk now stops after ctx lines of trailing context. Master kept up to 2*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 is Nothing, including a header without its closing @@ (master accepted that). A hunk with old count 0 now goes after line a instead of before it. The out-of-range and out-of-order checks are unchanged.
4. patch now uses exactly b old lines and d new lines per hunk: a space line counts on both sides, - on the old side, + on the new side, and \ No newline at end of file counts on neither. A hunk is Nothing if 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 / -b on a b c gave Just "a\nc". The following are skipped, as GNU patch and git also skip them:

  • lines before the first hunk (diff --git, index, ---/+++, free text);
  • lines after a hunk's counts are used up and before the next @@.

Because of this, real diff -u and git diff output now applies; master returned Nothing for all of it. A ---/+++ pair after a hunk starts a second file and gives Nothing, because patch applies to one document. Inside a hunk, --- x is still the deletion of a line -- x.

The doc string of patch now says it takes single-file diffs from unified, diff -u or git diff, and lists what it rejects.

Consequences

  • A patch written by the old unified with 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.
  • Two existing assertions change from Just "a\nb" to Nothing: a stray é or 0xBB line inside a hunk body is now malformed, as it is for GNU patch and git. Both assertions still check that patch does 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 file is 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)

  • Suite: 114 passed, 0 failed. Run against master, the new suite fails 15 assertions: the 5 new renderer assertions, 8 of the 12 new applier assertions, and the 2 changed ones. The other 4 new assertions pass on master and guard the new code: ---/+++ inside a hunk, the no-newline marker, a second file, and the ctx-0 round trip.
  • Library output applied by GNU patch (300 random pairs over 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.
  • Output of GNU diff -U0/-U1/-U3 (with file headers) and git diff --no-index -U0/-U3, applied by Diff.patch on the same pairs written as newline-terminated files: 1495/1495 correct. Master returned Nothing for all 1495. With the file headers stripped, master gave a wrong document for 145 of 299 diff -U0 patches and 144 of 299 git diff -U0 patches.
  • When neither file ends in a newline, 277 of 299 per tool are correct. All 22 misses have an empty original: "" splits into one empty line, so the result gains a final newline. This is the newline limitation above.
  • 15 mutants of the new code, all killed, each by the listed tests:
    • old count read one too high, or new count one too low: 16 tests each, including every patch round-trips … row.
    • truncation check removed: truncated hunk.
    • a context, + or - line ignores its count: too many context lines, too many + lines and too many - lines respectively.
    • \ marker rejected: skips no-newline markers.
    • zero-count hunk placed before line a: zero-count hunk after the line it names, appends a zero-count hunk at the end and round-trips unified-with-context 0.
    • second-file check removed: second file.
    • ---/+++ skipped inside a hunk: --- and +++ inside a hunk.
    • stray body line skipped: the 2 changed does not abort rows.
    • omitted count read as 0: count-omitted hunk header and 4 others.
    • closing @@ not checked: header without its closing @@.
    • renderer zero-count start fixed on the old side only: zero-count new range and +0,0.
    • renderer trailing context back to 2*ctx: ends the last hunk after ctx context lines.
  • patch old (unified-with-context (line-diff old new) ctx) = Just new held for 1200 random document pairs at ctx 0/1/3. The documents mixed in empty lines and lines like --x, ++y and z.

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

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.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Build & Tests

  • carp -x tests/diff.carp on the head (compat core, armhf): 114 passed, 0 failed. The new test file against master's diff.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-memory probe balances to 0. It runs 200 rounds of patch on headered, malformed, second-file and truncated input, plus unified-with-context at 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/-U3 and git diff --no-index -U0/-U1/-U3 output, headers included, on newline-terminated files: 238/238 for each of the six. Master returns Nothing for all of them.
  • Library output → tools. unified-with-context of Diff.diff over the files' lines, applied by GNU patch and by git apply (--unidiff-zero at 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 new for 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 diff patches (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-num without the Int.str round trip (diff.carp:339): @@ -02 +02 @@ applies. On LP64, @@ -4294967298 +4294967298 @@\n-b\n+B wraps to line 2 and gives Just "a\nB".
  • No non-negative check in parse-hunk-header (diff.carp:357): @@ -1,x +1,x @@\n-a\n+A becomes a no-op, Just "a\nb".
  • No out-of-order check (diff.carp:426): @@ -3 … followed by @@ -1 … on a b c gives Just "a\nb\nC\nA\nb\nc".
  • Deleted lines not compared (diff.carp:404): @@ -2 +2 @@\n-x\n+B on a b gives Just "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. a format-patch signature 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 -U3 patches and 22 of 238 -U1 patches come back Nothing, 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") is Just "A\n\nb\n" on master and Nothing here.
  • I tried an 11-line fix: inside a hunk, read "" as an empty context line, except the last element, which split-by leaves 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 still Nothing, 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 apply rejects 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-zero both 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@@\n gives @@ -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 produce a \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.

@hellerve
hellerve marked this pull request as ready for review November 1, 2026 12:40
@hellerve
hellerve merged commit ca1cfac into master Nov 1, 2026
2 checks passed
@hellerve
hellerve deleted the claude/unified-interop branch November 1, 2026 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant