git add --patch bug with split+edit?

Hannu Koivisto· Jan 17, 2009, 01:37 UTC · lore

Greetings,

If I have a hunk that adds three lines, I can edit the hunk and remove the last line but I can't split it in two, stage the first part, edit the second part and remove the last line. An example:

mkdir gittest cd gittest git init echo "baz\nbaz" > baz git add baz git commit -m baz baz rm baz echo "sur\nbaz\nbaz\njee\njee" > baz git add --patch

Now say 's RET y RET e RET' and remove the second "+jee" line using your editor. The output for me looks like this:

--8<-----------------------------------------------------------------

diff --git a/baz b/baz
index 1f55335..48a5f83 100644
--- a/baz
+++ b/baz
@@ -1,2 +1,5 @@
+sur
 baz
 baz
+jee
+jee
Stage this hunk [y/n/a/d/s/e/?]? s
Split into 2 hunks.
@@ -1,2 +1,3 @@
+sur
 baz
 baz
Stage this hunk [y/n/a/d/j/J/e/?]? y
@@ -1,2 +2,4 @@
 baz
 baz
+jee
+jee
Stage this hunk [y/n/a/d/K/e/?]? e
Waiting for Emacs...
error: patch failed: baz:1
error: baz: patch does not apply
Your edited hunk does not apply. Edit again (saying "no" discards!) [y/n]?
--8<-----------------------------------------------------------------

What I also didn't expect is that if I answer 'n' to that last
question, I get...

@@ -1,2 +1,3 @@
+sur
 baz
 baz
Stage this hunk [y/n/a/d/j/J/e/?]?

...which is the first part of the splitted hunk that I already
staged.  If I answer 'd', git status and git diff indicate that
"+sur" is nevertheless staged.

Now, if instead of splitting the hunk and editing it, I edit the
entire...

@@ -1,2 +1,5 @@
+sur
 baz
 baz
+jee
+jee

...hunk and remove the last "+jee" line, I get no error.

I'm using git 1.6.1 on Linux.

-- 
Hannu

Jeff King· Jan 17, 2009, 02:33 UTC · re: Hannu Koivisto · lore

Re: git add --patch bug with split+edit?

On Sat, Jan 17, 2009 at 03:37:43AM +0200, Hannu Koivisto wrote:

Show 9 quoted lines

> echo "sur\nbaz\nbaz\njee\njee" > baz
> git add --patch
> 
> Now say 's RET y RET e RET' and remove the second "+jee" line using
> your editor.  The output for me looks like this:
> [...]
> error: patch failed: baz:1
> error: baz: patch does not apply
> Your edited hunk does not apply. Edit again (saying "no" discards!) [y/n]?

Actually, you do not even need to change the patch at all for this to fail. The hunk that you edit looks like this:

@@ -1,2 +2,4 @@
 baz
 baz
+jee
+jee

which doesn't make sense. I think the hunk header should actually be:

  @@ -1,2 +1,4 @@

But I don't think that is the problem, since git-apply should be
recounting the hunk information (and in a simple test, it doesn't fix
it).

Hm. OK, I see. The "does this diff apply" check feeds _both_ parts of
the split patch to git-apply. But of course the second part will never
correctly apply, because its context overlaps with the first part, but
doesn't take it into account.

Doing the check with _just_ the edited patch would work. But that
doesn't take into account that your edited patch will potentially fail
to apply in the long run, depending on whether or not you accept the
other half of the split patch. And we can't know that yet, because the
user may not have told us (they could have skipped the first half, and
then come back to it later after the edit step).

So in general, I think splitting and editing the same hunk is inherently
dangerous and is going to lead to these sorts of problems. And because
editing provides a superset of the functionality, I think you should
just edit and either allow the first part of the hunk to be applied or
not depending on your preference.

But maybe there is some better way of resolving the conflict. I don't
see one, but I'm tired and didn't think too hard on it. :)

-Peff