Am 28.10.24 um 19:39 schrieb Elijah Newren: > On Mon, Oct 28, 2024 at 11:11 AM Taylor Blau <me@xxxxxxxxxxxx> wrote: >> >> On Mon, Oct 28, 2024 at 05:57:54PM +0100, Peregi Tamás wrote: >>> Hi all, >>> >>> I might've found an inconsistency in how git-apply treats corrupt patches >>> (representing empty context lines with completely empty lines instead of >>> lines containing a single space - usually a result of a "trim trailing >>> whitespace" editor setting) based on whether the patch file uses >>> Windows-style line endings (CRLF) or Unix-style line endings (LF only). > > I believe this behavior was caused by: > > $ git log -1 b507b465f7831612b9d9fc643e3e5218b64e5bfa > commit b507b465f7831612b9d9fc643e3e5218b64e5bfa > Author: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> > Date: Thu Oct 19 19:26:08 2006 -0700 > > git-apply: prepare for upcoming GNU diff -u format change. > > The latest GNU diff from CVS emits an empty line to express > an empty context line, instead of more traditional "single > white space followed by a newline". Do not get broken by it. > > Signed-off-by: Linus Torvalds <torvalds@xxxxxxxx> > Signed-off-by: Junio C Hamano <junkio@xxxxxxx> > > That code special-cased a line containing '\n' but not a line > containing only '\r\n'. > > As to whether that's correct, personally I'd rather only special case > workaround important existing clients. Back in 2006, working with GNU > diff was incredibly important, and I'd say is still important today. > I can see Peregi's comment that this make line ending slightly > inconsistent, but I feel like the blank-line handling is a workaround > for an existing client we want to interoperate with and absent a > similar important client with mis-behaving '\r\n'-only lines, I > wouldn't be interested in adding support for it. But that's just my > off-the-cuff feeling and I don't feel strongly about it. Further, all > but one of my contributions above were mere header changes, so if > others have other opinions, they should probably be weighted more > heavily than mine on this topic. What would the patch(1) do? The first column is the exit code of the subsequent command (0: success, 1: one or more rejected lines, 2: failure): 0 patch -p1 --dry-run original-unix.txt <corrupt-unix.patch 0 patch -p1 --dry-run original-unix.txt <intact-unix.patch 0 patch -p1 --dry-run original-win.txt <intact-win.patch 0 patch --ignore-whitespace -p1 --dry-run original-unix.txt <corrupt-unix.patch 0 patch --ignore-whitespace -p1 --dry-run original-unix.txt <intact-unix.patch 0 patch --ignore-whitespace -p1 --dry-run original-unix.txt <intact-win.patch 0 patch --ignore-whitespace -p1 --dry-run original-win.txt <corrupt-unix.patch 0 patch --ignore-whitespace -p1 --dry-run original-win.txt <intact-unix.patch 0 patch --ignore-whitespace -p1 --dry-run original-win.txt <intact-win.patch 1 patch -p1 --dry-run original-unix.txt <intact-win.patch 1 patch -p1 --dry-run original-win.txt <corrupt-unix.patch 1 patch -p1 --dry-run original-win.txt <intact-unix.patch 2 patch -p1 --dry-run original-unix.txt <corrupt-win.patch 2 patch -p1 --dry-run original-win.txt <corrupt-win.patch 2 patch --ignore-whitespace -p1 --dry-run original-unix.txt <corrupt-win.patch 2 patch --ignore-whitespace -p1 --dry-run original-win.txt <corrupt-win.patch So basically the same as git apply? What does current GNU diff do? diff -u <(printf 'a\n\n') <(printf 'b\n\n') | tail -1 | od -a 0000000 sp nl 0000002 diff -u --suppress-blank-empty <(printf 'a\n\n') <(printf 'b\n\n') | tail -1 | od -a 0000000 nl 0000001 diff -u <(printf 'a\r\n\r\n') <(printf 'b\r\n\r\n') | tail -1 | od -a 0000000 sp cr nl 0000003 diff -u --suppress-blank-empty <(printf 'a\r\n\r\n') <(printf 'b\r\n\r\n') | tail -1 | od -a 0000000 sp cr nl 0000003 So it omits the space only if --suppress-blank-empty is given and the blank line ends with \n, not with \r\n. Anyway, I agree with Elijah: The targeted support for GNU diff's eccentricity is fine, and we're in good company with that. We could remove it, since it doesn't seem to be the default (anymore?), but I don't see much of a benefit. We could add support for blank context lines that end in CRLF if there's a notable source of that kind of that deviation from the original format. René
