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

benkuhn · 2018-09-21T17:29:21Z

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

The text was updated successfully, but these errors were encountered:

zsol · 2018-09-21T19:22:44Z

Interesting, does it work with GNU patch?

benkuhn · 2018-09-21T21:00:15Z

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

ambv · 2018-09-25T15:52:08Z

@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 · 2018-09-29T15:28:12Z

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 · 2018-09-29T15:34:23Z

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 · 2020-04-02T07:14:27Z

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 · 2020-04-02T07:35:02Z

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 · 2020-04-02T07:46:02Z

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

`split("\n")` includes a final empty element `""` if the final line ends with `\n` (as it should for POSIX-compliant text files), which then became an extra `"\n"`. `splitlines()` solves that, but there's a caveat, as it will split on other types of line breaks too (like `\r`), which may not be desired. Fixes psf#526.

`split("\n")` includes a final empty element `""` if the final line ends with `\n` (as it should for POSIX-compliant text files), which then became an extra `"\n"`. `splitlines()` solves that, but there's a caveat, as it will split on other types of line breaks too (like `\r`), which may not be desired. Fixes #526.

JelleZijlstra added the C: configuration CLI and configuration label May 5, 2019

akien-mga mentioned this issue Apr 2, 2020

Re-Added screen space reflection. godotengine/godot#37512

Merged

akien-mga mentioned this issue Apr 2, 2020

Fix --diff output when encountering EOF #1328

Merged

JelleZijlstra closed this as completed in #1328 Apr 5, 2020

harenbrs mentioned this issue Apr 9, 2020

Merge from upstream harenbrs/bleck#1

Merged

ichard26 mentioned this issue Sep 2, 2020

"--diff" option doesn't add a newline at the end of the file using 20.8b1 #1662

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

benkuhn commented Sep 21, 2018

zsol commented Sep 21, 2018

benkuhn commented Sep 21, 2018 •

edited

Loading

ambv commented Sep 25, 2018

benkuhn commented Sep 29, 2018

benkuhn commented Sep 29, 2018 •

edited

Loading

akien-mga commented Apr 2, 2020 •

edited

Loading

akien-mga commented Apr 2, 2020 •

edited

Loading

akien-mga commented Apr 2, 2020

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

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

Comments

benkuhn commented Sep 21, 2018

zsol commented Sep 21, 2018

benkuhn commented Sep 21, 2018 • edited Loading

ambv commented Sep 25, 2018

benkuhn commented Sep 29, 2018

benkuhn commented Sep 29, 2018 • edited Loading

akien-mga commented Apr 2, 2020 • edited Loading

akien-mga commented Apr 2, 2020 • edited Loading

akien-mga commented Apr 2, 2020

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

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

benkuhn commented Sep 21, 2018 •

edited

Loading

benkuhn commented Sep 29, 2018 •

edited

Loading

akien-mga commented Apr 2, 2020 •

edited

Loading

akien-mga commented Apr 2, 2020 •

edited

Loading