--diff output does not `git apply` cleanly when the diff context includes EOF

[benkuhn]

Thanks for making an awesome tool! I ran across this issue while trying to make a script that asks before applying changes. (Obviously I could reapply the changes by running black again so it's not too important, but being able to cache the diff so the apply is faster would be nice, and avoid a race condition if someone edits a file in the middle)

Steps to reproduce issue:

# setup
git init black-repro && cd black-repro && echo "x = '1'" > foo.py

black --diff . | git apply
# error: patch failed: foo.py:1
# error: foo.py: patch does not apply

The problem is that the diff includes an extra newline at the end compared to what git expects:

black --diff . | sed s/1,2/1,1/g | grep -Ev '^ $' | git apply
# works

[zsol]

Interesting, does it work with GNU patch?

[benkuhn]

Yeah, it does, good idea! Totally forgot about GNU patch. That's a good workaround. Thanks!

[ambv]

@benkuhn Does GNU patch work? If yes then we need to understand what the failure mode here is. If this is really our diff's fault, I see no problem fixing this.

[benkuhn]

Sorry for not being clear! GNU patch does successfully apply the Black patch.

Here is the output of git diff:

diff --git a/foo.py b/foo.py
index 3263258..4dd6b40 100644
--- a/foo.py
+++ b/foo.py
@@ -1 +1 @@
-x = '1'
+x = "1"

And here is the output of black --diff . (note the extra line at the end with one space character):

--- foo.py	2018-09-29 15:17:59.864001 +0000
+++ foo.py	2018-09-29 15:18:11.125531 +0000
@@ -1,2 +1,2 @@
-x = '1'
+x = "1"
 

The error message for black --diff . | git apply is:

error: patch failed: foo.py:1
error: foo.py: patch does not apply

The failure mode is that the Black patch declares that it is patching two lines, and git apply believes the file is only one line long:

• If I edit foo.py to contain two final newlines, the black patch is successfully applied by git apply.
• If I apply the black patch with GNU patch, the resulting foo.py contains two final newlines (where the original contained one).

[benkuhn]

Here's a repro where the patch does not apply with either tool:

code % git init black-repro && cd black-repro && echo "x = 1\n" > foo.py && git add . && git commit -m 'initial commit' # create a file with an extra trailing newline
[master] black-repro % black -q --diff .
--- foo.py      2018-09-29 15:30:55.113636 +0000
+++ foo.py      2018-09-29 15:32:25.608989 +0000
@@ -1,3 +1,2 @@
 x = 1
 
-
[master] black-repro % black -q --diff . | git apply
error: patch failed: foo.py:1
error: foo.py: patch does not apply
[master] black-repro % black -q --diff . | patch    
patching file foo.py
Reversed (or previously applied) patch detected!  Assume -R? [n] n
Apply anyway? [n] y
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file foo.py.rej

I think GNU patch detects it as "previously applied" because the patch shows the file going from 3 lines long to 2 lines long, but patch is already ignoring the trailing newline so sees the file as 2 lines long.

[akien-mga]

I have a similar issue, though it could also be considered separate, so please advise if I'd better open a new issue about it.

I also have a situation where black --diff involving changes at the EOF generate a patch that cannot be applied with git apply nor GNU patch.
In my case it seems to be because the diff includes too many lines before the actual changes.

Input file:

#!/usr/bin/env python

Import("env")

if "RD_GLSL" in env["BUILDERS"]:
    env.RD_GLSL("ssao.glsl")
    env.RD_GLSL("ssao_minify.glsl")
    env.RD_GLSL("ssao_blur.glsl")
    env.RD_GLSL("roughness_limiter.glsl")
    env.RD_GLSL("screen_space_reflection.glsl")
    env.RD_GLSL("screen_space_reflection_filter.glsl")
    env.RD_GLSL("screen_space_reflection_scale.glsl")
    env.RD_GLSL("specular_merge.glsl")
    
    

(has trailing whitespace on the last two lines)

$ black -q --diff SCsub
--- SCsub       2020-04-02 07:09:58.106534 +0000
+++ SCsub       2020-04-02 07:11:32.927345 +0000
@@ -9,8 +9,6 @@
     env.RD_GLSL("roughness_limiter.glsl")
     env.RD_GLSL("screen_space_reflection.glsl")
     env.RD_GLSL("screen_space_reflection_filter.glsl")
     env.RD_GLSL("screen_space_reflection_scale.glsl")
     env.RD_GLSL("specular_merge.glsl")
-    
-    
 

$ black -q --diff SCsub | git apply
error: patch failed: SCsub:9
error: SCsub: patch does not apply

$ black -q --diff SCsub | patch
patching file SCsub
Hunk #1 FAILED at 9.
1 out of 1 hunk FAILED -- saving rejects to file SCsub.rej

black-diff.zip

On the other hand, git itself would generate this valid diff when doing the changes:

diff --git a/SCsub b/SCsub
index c112b93..1e70b36 100644
--- a/SCsub
+++ b/SCsub
@@ -11,5 +11,3 @@ if "RD_GLSL" in env["BUILDERS"]:
     env.RD_GLSL("screen_space_reflection_filter.glsl")
     env.RD_GLSL("screen_space_reflection_scale.glsl")
     env.RD_GLSL("specular_merge.glsl")
-    
-    

I first thought that the problem might be the extra newline at the end of the black diff, but removing it doesn't make the diff apply. I had to remove the extra two lines of context and amend the line @@ line accordingly for it to apply.

[akien-mga]

I first thought that the problem might be the extra newline at the end of the black diff, but removing it doesn't make the diff apply. I had to remove the extra two lines of context and amend the line @@ line accordingly for it to apply.

Nevermind, the problem is exactly as described by @benkuhn.

The 5 lines of context come from

black/black.py

Line 3882 in 9ed2542

difflib.unified_diff(a_lines, b_lines, fromfile=a_name, tofile=b_name, n=5)

but they're not problematic. It's indeed the extra newline which is problematic.

Here's the (git) diff applied on my generated black --diff that would make it functional:

diff --git a/diff-black b/diff-black
index 055e76d..af081b5 100644
--- a/diff-black
+++ b/diff-black
@@ -1,6 +1,6 @@
 --- SCsub      2020-04-02 07:33:18.592786 +0000
 +++ SCsub      2020-04-02 07:35:24.294130 +0000
-@@ -9,8 +9,6 @@
+@@ -9,7 +9,5 @@
      env.RD_GLSL("roughness_limiter.glsl")
      env.RD_GLSL("screen_space_reflection.glsl")
      env.RD_GLSL("screen_space_reflection_filter.glsl")
@@ -8,4 +8,3 @@
      env.RD_GLSL("specular_merge.glsl")
 -    
 -    
-

[akien-mga]

Found a potential fix, I'll make a PR.