apply fails to handle diffs with files without newline or patches that remove newline at end of the file

[fzakaria]

I encountered this issue via aspect-build/rules_lint#487

tl;dr; If a file is missing a newline at the end or the patch removes the newline at the end, go-gitdiff fails applying the patch with the error:

conflict: fragment line does not match src line

I'll push a reproduction example through a failed test, where git succeeds.

> git apply gitdiff/testdata/apply/file_text_modify.patch
> echo $?
0

> go test ./...
--- FAIL: TestApplyFile (0.00s)
    --- FAIL: TestApplyFile/textModify (0.00s)
        apply_test.go:190: unexpected error applying: at 4: fragment 1 at 5: conflict: fragment line does not match src line
FAIL
FAIL    github.com/bluekeyes/go-gitdiff/gitdiff 0.321s
FAIL

The offending check can be found

go-gitdiff/gitdiff/apply_text.go

Line 128 in fffa3cc

func applyTextLine(dst io.Writer, line Line, preimage [][]byte, i int64) (err error) {

[fzakaria]

Investigating more; seems to be that diff -u can produce the "No newline" statement multiple times.

> diff -u  gitdiff/testdata/apply/file_text.src gitdiff/testdata/apply/file_text_modify.out
--- gitdiff/testdata/apply/file_text.src        2025-03-04 14:16:41
+++ gitdiff/testdata/apply/file_text_modify.out 2025-03-04 13:35:04
@@ -1,4 +1,4 @@
-this is line 1
+the first line is different
 this is line 2
 this is line 3
 this is line 4
\ No newline at end of file
@@ -17,10 +17,10 @@
 this is line 17
 this is line 18
 this is line 19
+this line offsets all the line numbers!
 this is line 20
 this is line 21
-this is line 22
-this is line 23
+until here, now we're back on track!
 this is line 24
 this is line 25
 this is line 26
\ No newline at end of file
@@ -53,10 +53,10 @@
 this is line 53
 this is line 54
 this is line 55
-this is line 56
-this is line 57
-this is line 58
-this is line 59
+once upon a time, a line
+  in a text
+    file
+  changed
 this is line 60
 this is line 61
 this is line 62
\ No newline at end of file
@@ -130,8 +130,8 @@
 this is line 130
 this is line 131
 this is line 132
-this is line 133
-this is line 134
+this line was bad and has been removed
+this line was REDACTED and has been REDACTED
 this is line 135
 this is line 136
 this is line 137
\ No newline at end of file
@@ -161,12 +161,7 @@
 this is line 161
 this is line 162
 this is line 163
-this is line 164
-this is line 165
-this is line 166
-this is line 167
-this is line 168
-this is line 169
+the number on the remaining lines is 5 ahead of their actual position in the file
 this is line 170
 this is line 171
 this is line 172
\ No newline at end of file
@@ -197,4 +192,4 @@
 this is line 197
 this is line 198
 this is line 199
-this is line 200
+this is line 200
\ No newline at end of file

[fzakaria]

Okay even more discovery!
The ability for multiple No newline at end of file seems to be a part of Apple BSD diff :(

I did the same with GNU diff and found:

> /opt/homebrew/bin/diff -u  gitdiff/testdata/apply/file_text.src gitdiff/testdata/apply/file_text_modify.out
--- gitdiff/testdata/apply/file_text.src        2025-03-04 15:11:22.172538791 -0800
+++ gitdiff/testdata/apply/file_text_modify.out 2025-03-04 13:35:04.387135108 -0800
@@ -1,4 +1,4 @@
-this is line 1
+the first line is different
 this is line 2
 this is line 3
 this is line 4
@@ -17,10 +17,10 @@
 this is line 17
 this is line 18
 this is line 19
+this line offsets all the line numbers!
 this is line 20
 this is line 21
-this is line 22
-this is line 23
+until here, now we're back on track!
 this is line 24
 this is line 25
 this is line 26
@@ -53,10 +53,10 @@
 this is line 53
 this is line 54
 this is line 55
-this is line 56
-this is line 57
-this is line 58
-this is line 59
+once upon a time, a line
+  in a text
+    file
+  changed
 this is line 60
 this is line 61
 this is line 62
@@ -130,8 +130,8 @@
 this is line 130
 this is line 131
 this is line 132
-this is line 133
-this is line 134
+this line was bad and has been removed
+this line was REDACTED and has been REDACTED
 this is line 135
 this is line 136
 this is line 137
@@ -161,12 +161,7 @@
 this is line 161
 this is line 162
 this is line 163
-this is line 164
-this is line 165
-this is line 166
-this is line 167
-this is line 168
-this is line 169
+the number on the remaining lines is 5 ahead of their actual position in the file
 this is line 170
 this is line 171
 this is line 172
@@ -197,4 +192,4 @@
 this is line 197
 this is line 198
 this is line 199
-this is line 200
+this is line 200
\ No newline at end of file

Git seems to fail on that diff style as well.

[bluekeyes]

Thanks for reporting this. If I understand your investigation, this is specifically a problem with the output of the BSD diff tool, which includes the \ No newline at end of file marker after every fragment instead of only after the last one? I'm not familiar with the rules_lint project you mentioned, but it sounds like it might be running the version of the diff tool that exists on the local machine and then trying to apply the result as a patch.

If Git supports this type of patches, I should fix this library to also accept them. If Git fails on this style of patch, I'm less inclined to add support here, but could at least see how hard it might be. I'm reasonably confident that this works correctly with Git-style patches as there were existing tests for adding a final newline and changing content without changing the final newline. You were correct that there was no test for removing the final newline, but that seems to work as expected in the current version.

[fzakaria]

@bluekeyes it's kind of hard to replicate because the "algorithm" for producing the hunks is a bit tricky.

I think git is failing to apply this same patch which I sent an email to the mailing list.
https://lore.kernel.org/git/CACCo2jkKraSnGUeRqJSuis9EfiubxEbm1dT2WZk7gfk9xjQ1rg@mail.gmail.com/T/#u

It's not clear to me if it's a "bug" in Git or something purposeful.

Given the prevalence of MacOS for developers (Although I daily drive NixOS :P) seems like a foot gun given that patch works with it.

Anyways, this caused me a lot of hair pulling trying to debug; thought I'd record the info here.

[bluekeyes]

It looks like the rules_lint issue was fixed by consistently using a GNU diff binary instead of the system binary. That probably makes the most sense for that project, but I think clients who care about supporting BSD diff can also fix it by post-processing the gitdiff.File objects after parsing, but before applying. This is untested but illustrates the idea:

files, _, err := gitdiff.Parse(input)
if err != nil {
	return err
}

for _, file := range files {
	// For all but the last fragment, add a final newline to any lines that do not have one
	// Only the last fragment in a file can end without a newline
	for i := 0; i < len(files.TextFragments)-1; i++ {
		lines := files.TextFragments[i].Lines
		for n, line := range lines {
			if line.NoEOL() {
				lines[n].Line += "\n"
			}
		}
	}
}

Since Git currently behaves the same way as this library, I'm not going to make any changes. But if the Git developers determine this is a bug and update a future version of Git, I can also update the behavior here to match.