BUG?: git-apply sometimes says "corrupt patch" when adding single lines

Hi.

I often use git-gui when staging commits, and I noticed that if the
file I'm working with has a:

\ No newline at end of file

... as the last line of the diff, and I try to "Stage lines for
commit", then git-gui (or really git-apply, as I understand it) says
that I have a "corrupt patch at line <so-and-so>".

I don't really know the git-gui code or the git C code well, but it
seems that I got around the error with this small patch:

diff --git a/builtin/apply.c b/builtin/apply.c
index 12ef9ea..124791f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c

@@ -1335,6 +1335,8 @@ static int parse_fragment(char *line, unsigned long size,
 		 * l10n of "\ No newline..." is at least that long.
 		 */
 		case '\\':
+			if (newlines == oldlines == 1)
+				newlines = oldlines = 0;
 			if (len < 12 || memcmp(line, "\\ ", 2))
 				return -1;
 			break;

Is this a bug? Or is it maybe git-gui that sends something wrong to
git-apply? I don't know... :-/

(I'm using msysGit on Windows, if that matters)

-Tor Arvid-

Tor Arvid Lund wrote:

[...]

Heh.  This code is a roundabout way to say "if (newlines == oldlines)",
rather than "if (newlines == 1 && oldlines == 1)" that might have been
intended.

Unfortunately I do not have any more insight than that.  If you can
give an example reproducing this with git apply from the command
line, that would be very helpful.

Thanks,
Jonathan

On Wed, Aug 18, 2010 at 1:43 AM, Jonathan Nieder [off-list ref] wrote:

Tor Arvid Lund wrote:

quoted

I don't really know the git-gui code or the git C code well, but it
seems that I got around the error with this small patch:

diff --git a/builtin/apply.c b/builtin/apply.c
index 12ef9ea..124791f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c

@@ -1335,6 +1335,8 @@ static int parse_fragment(char *line, unsigned long size,

               * l10n of "\ No newline..." is at least that long.
               */
              case '\\':
+                     if (newlines == oldlines == 1)

[...]

Heh.  This code is a roundabout way to say "if (newlines == oldlines)",
rather than "if (newlines == 1 && oldlines == 1)" that might have been
intended.

Right you are, good sir ;-)

Unfortunately I do not have any more insight than that.  If you can
give an example reproducing this with git apply from the command
line, that would be very helpful.

I'll try to do that tomorrow.. My brain needs sleep before it will function :)

-Tor Arvid-

Am 8/18/2010 1:27, schrieb Tor Arvid Lund:

I often use git-gui when staging commits, and I noticed that if the
file I'm working with has a:

\ No newline at end of file

... as the last line of the diff, and I try to "Stage lines for
commit", then git-gui (or really git-apply, as I understand it) says
that I have a "corrupt patch at line <so-and-so>".

Is this a bug? Or is it maybe git-gui that sends something wrong to
git-apply? I don't know... :-/

This is really a bug in git-gui, not in git-apply: The big while loop in
lib/diff.tcl, function apply_range_or_line, must be taught about "\ No
newline...".

-- Hannes

On Wed, Aug 18, 2010 at 9:29 AM, Johannes Sixt [off-list ref] wrote:

Am 8/18/2010 1:27, schrieb Tor Arvid Lund:

quoted

I often use git-gui when staging commits, and I noticed that if the
file I'm working with has a:

\ No newline at end of file

... as the last line of the diff, and I try to "Stage lines for
commit", then git-gui (or really git-apply, as I understand it) says
that I have a "corrupt patch at line <so-and-so>".

Is this a bug? Or is it maybe git-gui that sends something wrong to
git-apply? I don't know... :-/

This is really a bug in git-gui, not in git-apply: The big while loop in
lib/diff.tcl, function apply_range_or_line, must be taught about "\ No
newline...".

Ok, thanks for tracking it down. I suspected this, since the tests for
git-apply seemed to have example patches for "\ No newline...".
Ultimately, this doesn't scratch me enough that I want to learn (or
guess) how to fix the Tcl stuff...

-Tor Arvid-