Can't apply patch that removes the last empy line of a file

[luisdavim]

I generated some diff files using https://pkg.go.dev/github.com/rogpeppe/go-internal/diff and then tried to apply them with this library and got unexpected EOF.

This is the diff in question:

diff values.yaml values.yaml
--- values.yaml
+++ values.yaml
@@ -42,8 +42,8 @@
     # cpu: 1000m
     # memory: 1Gi
     requests:
-      cpu: 100m
-      memory: 64Mi
+      cpu: 150m
+      memory: 120Mi
   priorityClassName: ""
   annotations:
     prometheus.io/port: "8080"
@@ -233,7 +233,6 @@
       #    hosts:
       #      - flux-webhook.example.com

-
   nodeSelector: {}
   affinity: {}
   tolerations: []
@@ -328,4 +327,3 @@

 distro:
   openshift: false
-

[luisdavim]

It seems like the issue is with the part of the diff that removes the empty line at the end of the file.

If I use:

diff values.yaml values.yaml
--- values.yaml
+++ values.yaml
@@ -42,8 +42,8 @@
     # cpu: 1000m
     # memory: 1Gi
     requests:
-      cpu: 100m
-      memory: 64Mi
+      cpu: 150m
+      memory: 120Mi
   priorityClassName: ""
   annotations:
     prometheus.io/port: "8080"
@@ -233,7 +233,6 @@
       #    hosts:
       #      - flux-webhook.example.com

-
   nodeSelector: {}
   affinity: {}
   tolerations: []

It applies without any issues...
So this might also be an issue with diffs generaged using git and not necessarely just for diffs generated with: https://pkg.go.dev/github.com/rogpeppe/go-internal/diff

If you want to see the code that I'm using, it's available here: https://github.com/luisdavim/helmquilt

[bluekeyes]

Thanks for the report. Is it possible to also share or link to the original file to which you are trying to apply this patch? That will be helpful to understand exactly what is happening.

My first thought is that this is similar to #57 and is a result of there being no standard or reference implementation for unified diffs. When using patches not generated by Git, this library (and as far as I know, Git itself) is written with the assumption of GNU diff output. This has caused problems when applying patches from BSD diff and I imagine the github.com/rogpeppe/go-internal/diff package could also have some ways in which it diverges from how GNU diff generates patches, especially where final newlines are concerned.

Another thing to check: does Git accept and apply this patch or does Git also produce an error? If Git accepts it, then this library should as well. If Git rejects it, then it may still be possible to support here, but Git's behavior is what I'm trying to match.

[luisdavim]

Hi, thanks for your reply.

This is the file the patch is for https://github.com/fluxcd-community/helm-charts/blob/main/charts%2Fflux2%2Fvalues.yaml

[bluekeyes]

After trying this locally, I believe the unexpected EOF error correctly indicates a problem with the patch. The upstream file does not contain an empty line at the end, so attempting to apply a patch that removes that final line should fail. I think a bug is that go-gitdiff should report this as a gitdiff.Conflict instead of a plain error, but returning an error is correct.

I think the Dos2Unix function in helmquilt is responsible for adding the final line to the local copy that's missing from the upstream file:

		_, _ = writer.WriteString("\n")
		if err == io.EOF {
			break
		}

When reading the last line, ReadLine() returns a nil error. On the next pass through the loop, it returns io.EOF and an empty buffer, but the code writes another \n to the output even though there was no new content. Swapping the error check and the WriteString called seemed to fix it for me locally.

[luisdavim]

Oh, thanks for this. Kind of embarrassed I didn't notice it before, but I really appreciate you taking the time to look at my code and finding this issue.