Re: Regression in patch add?

From: Phillip Wood <phillip.wood@talktalk.net>
Date: May 10, 2018, 13:49 UTC
Message-ID: <be321106-2f10-e678-8237-449d2dd30fee@talktalk.net>
In-Reply-To: <CAN0heSq5SyPgoEURRVHupcabVu3jX+tmX+0U-6azrJDDgfZ5Gw@mail.gmail.com>

On 10/05/18 13:17, Martin Ågren wrote:

Show 13 quoted lines

> 
> On 10 May 2018 at 12:41, Oliver Joseph Ash <oliverjash@gmail.com> wrote:
>> I just ran into a similar problem: https://stackoverflow.com/questions/50258565/git-editing-hunks-fails-when-file-has-other-hunks
>>
>> I can reproduce on 2.17.0. The issue doesn't occur on 2.16.2, however.
>>
>> Is this a bug?
> 
> I would think so. Thanks for finding this thread. To keep history
> around, it would be nice to have your reproduction recipe on the list,
> not just on stackoverflow. That said, I cannot reproduce on v2.17.0
> using your recipe. I suspect there is something quite interesting going
> on here, considering how trivial your edit is.

Thanks Oliver for posting an example that we can test, that said I can't reproduce it on Linux if the hunk is edited correctly. However if I remove the leading space from the empty line between 'baz' and 'foo' then I get the same error as you. Perhaps your editor is stripping trailing white space? If so that will lead to problems when editing diffs as the leading space is needed for apply to know that it's an empty context line.

For the mailing list the hunk in question looks like

@@ -1,5 +1,5 @@
 foo
-bar
+bar1
 baz

 foo

I've tried using 'git apply --recount --cached' directly and was
surprised to see that it accepts the patch with the broken context line.
In 2.17.0 'add -p' no longer uses the --recount option, instead it
counts the patch it's self but stops counting when it runs out of lines
starting with [- +], this explains the difference from earlier versions.
It seems it's not uncommon for editors to strip the space from empty
context lines so maybe 'add -p' should take that into account when
recounting patches. I'm about to go off line for a couple of weeks so it
will probably be next month before I'm able to put a patch together
(assuming Junio agrees we should support broken hunks)

Best Wishes

Phillip


> As a shot in the dark, does your test involve unusual file systems,
> funny characters in filenames, ..? You are on some sort of Linux, right?
> 
> The first thing to try out might be something like
> 
> $ # create the initial file as before, with "bar"
> $ # git add, git commit ...
> $ # do the "change bar to bar1" everywhere
> $ git diff >test-patch
> $ git reset --hard
> $ # edit the *FIRST* hunk in test.patch like before (bar1 -> bar2)
> $ git apply --check test.patch && echo "ok..."
> $ git apply test.patch
> 
> Does that succeed at all?
> 
> $ git diff
> 
> should now show bar2 in the first hunk and bar1 in the second hunk,
> just like your edited test.patch.
> 
> If that works, it would seem that the problem is with `git add -p`, and
> how it is generating the patches for `git apply`. I have some ideas
> about how to debug from there, but ... How comfortable are you with
> building Git from the sources? Or with temporarily fiddling around with
> your Git installation? (git-add--interactive is a Perl script, so it
> would be possible to edit it in place to emit various debug
> information. That has potential for messing up royally, though.)
> 
> Martin
>