(On vacation, pardon GMail web client). As POSIX.1 says https://pubs.opengroup.org/onlinepubs/9799919799/utilities/diff.html#tag_20_34_10_07 """It is implementation-defined whether an empty unaffected line is written as an empty line or a line containing a single <space> character.""", it might be unfair to call it "GNU's eccentricity". But I found that the POSIX text is fairly clear that the OP's CRLF-only line does not even qualify as an context line that is totally empty. So perhaps there is nothing to see here, and not just we and patch(1), but GNU diff is doing the right thing? 2024年10月29日(火) 7:07 René Scharfe <l.s.r@xxxxxx>: > > 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é > >
