git add -p and unresolved conflicts

git add -p and unresolved conflicts

[Erik Faye-Lund]

I'm running git version 1.7.9.msysgit.0.390.g01fca.dirty, and I've
noticed a puzzling behavior with "git add -p" on a conflicted file: it
seems to simply spew the diff and exit the process.

Is this the expected behavior? It doesn't seem very useful to me...

Here's how to reproduce the behavior:

git init /tmp/git-add--p-conflict
cd /tmp/git-add--p-conflict/
echo "foo" > bar.txt
git add bar.txt
git commit -m.
echo "bar" >> bar.txt
git stash save
echo "baz" >> bar.txt
git commit -am.
git stash pop
git add -p bar.txt

Re: git add -p and unresolved conflicts

[Junio C Hamano]

Erik Faye-Lund <kusmabite@gmail.com> writes:

> I'm running git version 1.7.9.msysgit.0.390.g01fca.dirty, and I've
> noticed a puzzling behavior with "git add -p" on a conflicted file: it
> seems to simply spew the diff and exit the process.

Yeah, when I wrote the 'p'atch mode in "add--interactive", I wasn't
interested at all in letting it be used on a conflicted path, so that is
not a designed-in behaviour but merely whatever it happens to do.  So at
least it should not allow the user to pick a path that has conflicts in
it.

What the behaviour for people who *do* want to use the patch mode for
conflicted paths should be is a separate matter.  As I said, I am not
interested in it, so I wouldn't be the best person to design it.

I mildly suspect nobody would come up with a sane behaviour, but what
would I know...

Re: git add -p and unresolved conflicts

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> I mildly suspect nobody would come up with a sane behaviour, but what
> would I know...

Designing a per-hunk behavior would clearly be very tricky.

But a simple way to deal with this would be to show the whole-file
patch, and ask "stage this file (yes/no)?".

My fingers are used to typing

git add -p
y
y
y
git commit

and they would appreciate if it worked in the presence of conflicts.
Most of the time, I have small conflicts, so a whole-file behavior
wouldn't be disturbing, but I like seeing the changes before commiting
them.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: git add -p and unresolved conflicts

[Holger Hellmuth]

On 28.03.2012 17:21, Matthieu Moy wrote:
> Junio C Hamano<gitster@pobox.com>  writes:
>
>> I mildly suspect nobody would come up with a sane behaviour, but what
>> would I know...
>
> Designing a per-hunk behavior would clearly be very tricky.
>

I don't think it's that tricky:

For a conflicted file show any conflicted hunks first(!) and only 
provide the following options for them:
q - quit, do not stage...
d - do not stage this hunk nor any of the remaining hunks in the file
e - manually edit the current hunk
? - print help

and additionally the following three options:
b - choose the base version
< - choose our version
 > - choose their version

< and > correspond to the conflict markers, alternatives would be o and t

The user now has the option to remove the conflicts (through editing or 
choosing one of the versions) or abort adding the file.

The tricky part might be sorting the hunks so that conflicts are first 
(depending on how the code works now), choosing the right version in a 
hunk (the three new options) should be relatively easy.

Re: git add -p and unresolved conflicts

[Junio C Hamano]

Holger Hellmuth <hellmuth@ira.uka.de> writes:

> I don't think it's that tricky:
> ...
> The tricky part might be sorting the hunks so that conflicts are first
> (depending on how the code works now), choosing the right version in a
> hunk (the three new options) should be relatively easy.

Code it and then tell us if it is tricky or not.  Until then, especially
when you have to say "depending on how the code works now", implying you
do not know, why should we even listen to you?

Re: git add -p and unresolved conflicts

[Holger Hellmuth]

On 28.03.2012 21:19, Junio C Hamano wrote:
> Holger Hellmuth<hellmuth@ira.uka.de>  writes:
>
>> I don't think it's that tricky:

I was assuming this "tricky" was about a sensible user interface.

>> ...
>> The tricky part might be sorting the hunks so that conflicts are first
>> (depending on how the code works now), choosing the right version in a
>> hunk (the three new options) should be relatively easy.
>
> Code it and then tell us if it is tricky or not.  Until then, especially
> when you have to say "depending on how the code works now", implying you
> do not know, why should we even listen to you?

As it is perl I'm already looking at it. I'll try to find out if my 
meagre programming skills can do something here.

Re: git add -p and unresolved conflicts

[Jeff King]

On Wed, Mar 28, 2012 at 09:52:55PM +0200, Holger Hellmuth wrote:

> On 28.03.2012 21:19, Junio C Hamano wrote:
> >Holger Hellmuth<hellmuth@ira.uka.de>  writes:
> >
> >>I don't think it's that tricky:
> 
> I was assuming this "tricky" was about a sensible user interface.

I think maybe the discussion elsewhere in this thread has given you a
taste of the trickiness, but if you want to read more, here is a similar
discussion from the same time last year (ahh, Spring, when a young man's
thoughts turn to partial staging of conflicted hunks):

  http://thread.gmane.org/gmane.comp.version-control.git/168403/focus=168842

-Peff

Re: git add -p and unresolved conflicts

[Holger Hellmuth]

On 29.03.2012 08:08, Jeff King wrote:
> On Wed, Mar 28, 2012 at 09:52:55PM +0200, Holger Hellmuth wrote:
>
>> On 28.03.2012 21:19, Junio C Hamano wrote:
>>> Holger Hellmuth<hellmuth@ira.uka.de>   writes:
>>>
>>>> I don't think it's that tricky:
>>
>> I was assuming this "tricky" was about a sensible user interface.
>
> I think maybe the discussion elsewhere in this thread has given you a
> taste of the trickiness, but if you want to read more, here is a similar
> discussion from the same time last year (ahh, Spring, when a young man's
> thoughts turn to partial staging of conflicted hunks):

It already dawned on me that to add the proposed conflict resolution 
would also mean changing the working copy of the file which none of the 
other options needs. Which would change the work from (assumed) surgical 
changes to completely new routines and special cases and....

Re: git add -p and unresolved conflicts

[Matthieu Moy]

Holger Hellmuth <hellmuth@ira.uka.de> writes:

> and additionally the following three options:
> b - choose the base version
> < - choose our version
> > - choose their version

What does it mean to "choose" in this context? In general, it means "put
this into the index", but if the file is in conflicted state, you have 3
different versions of the file in the index.

And you miss the most usefull (to me at least): "choose the version in
the worktree".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: git add -p and unresolved conflicts

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> And you miss the most usefull (to me at least): "choose the version in
> the worktree".

I tend to think that "add -p" whose sole purpose is to make partial commit
is fundamentally incompatible with "am", "merge" and (mostly) "rebase",
which you do not want to record partial commit.

When "rebase -i" or "cherry-pick" that is used for the purpose of
replaying only a part of a larger commit, letting "add -p" show the
difference between "HEAD" and the working tree version, and letting the
user pick hunks, would make a lot of sense, though.

For such an application, however, the user could first "reset $path" and
use "add -p" normally.

Re: git add -p and unresolved conflicts

[Holger Hellmuth]

On 28.03.2012 21:38, Matthieu Moy wrote:
> Holger Hellmuth<hellmuth@ira.uka.de>  writes:
>
>> and additionally the following three options:
>> b - choose the base version
>> <  - choose our version
>>> - choose their version
>
> What does it mean to "choose" in this context? In general, it means "put
> this into the index", but if the file is in conflicted state, you have 3
> different versions of the file in the index.

The only ways to exit with the proposed reduced options is to either 
quit adding this file (which is comparable to Eriks option 4) or 
resolving all conflicts in the file thereby allowing to add this file to 
the index.

> And you miss the most usefull (to me at least): "choose the version in
> the worktree".
>

But the conflicted chunks are of the form "<<<< our ... |||||||||| 
theirs >>>>>>" in your work tree. So there are two cases:

a) You have removed the markers thereby removing the conflict -> this 
means the chunk will not be offered to you as a conflicting chunk

b) You haven't removed the markers. Then there is a choice between base, 
our and their version.
If you had edited one of the versions between the conflict markers in 
your working tree without removing the markers (which wouldn't be a 
sensible thing) then it depends on whether the chunk in the working tree 
or the versions in the index are used as a basis for the choosing, but I 
suspect using the working tree version is easier to do.

Re: git add -p and unresolved conflicts

[Junio C Hamano]

Holger Hellmuth <hellmuth@ira.uka.de> writes:

> But the conflicted chunks are of the form "<<<< our ... ||||||||||
> theirs >>>>>>" in your work tree. So there are two cases:
>
> a) You have removed the markers thereby removing the conflict -> this
> means the chunk will not be offered to you as a conflicting chunk
>
> b) You haven't removed the markers. Then there is a choice between
> base, our and their version.
> If you had edited one of the versions between the conflict markers in
> your working tree without removing the markers (which wouldn't be a
> sensible thing) then it depends on whether the chunk in the working
> tree or the versions in the index are used as a basis for the
> choosing, but I suspect using the working tree version is easier to
> do.

I think Matthieu is not thinking about the "resolve conflicted merge (or
rebase, am) and make a single commit" use case, for which your "pick one
of the sides" makes sense.  His is more about "I like some change in that
unrelated commit---while I have no intention to replay that change as a
whole, I want to use "cherry-pick --no-commit" and pick only the bits out
of the change that are useful for what I am doing" use case.

In such a context, after resolving conflicts in the working tree, "add -p"
would be a good tool to pick hunks in the diff between the HEAD and the
working tree.

I tend to think that use case, while valid, may be a minority case and
giving a version of "add -p" to make that mode too easy to use by mistake
during a true conflict resolution is inviting trouble for newbies.  As I
said, "reset $path" followed by regular "add -p $path" would be a usable
workaround for Matthieu's workflow.

Re: git add -p and unresolved conflicts

[Holger Hellmuth]

Am 28.03.2012 22:50, schrieb Junio C Hamano:
> I think Matthieu is not thinking about the "resolve conflicted merge (or
> rebase, am) and make a single commit" use case, for which your "pick one
> of the sides" makes sense.  His is more about "I like some change in that
> unrelated commit---while I have no intention to replay that change as a
> whole, I want to use "cherry-pick --no-commit" and pick only the bits out
> of the change that are useful for what I am doing" use case.
>
> In such a context, after resolving conflicts in the working tree, "add -p"
> would be a good tool to pick hunks in the diff between the HEAD and the
> working tree.

Ok. Something like this (which is not really that different from Eriks 
example, so I might still be missing something) ?

git init .
echo "foo" > bar.txt
git add bar.txt
git commit -m.
git checkout -b new
echo "bar" >> bar.txt
git commit -am.
git checkout master
git reset --hard
echo "baz" >> bar.txt
git commit -am.
git cherry-pick --no-commit new
git add -p bar.txt

So he would be interested in keeping his previous working tree version 
(identical to HEAD commit, otherwise cherry-pick fails) for conflicting 
chunks as they (in most cases) don't have anything to do with the chunks 
he is interested in. In this case that is "baz"

Which would always be the "ours" side in any conflicting junks.

That suggests that he could just type '<' respective 'o' whenever a 
conflicting junk comes up as it would be the previous working tree version

Re: git add -p and unresolved conflicts

[Junio C Hamano]

Holger Hellmuth <hellmuth@ira.uka.de> writes:

> Am 28.03.2012 22:50, schrieb Junio C Hamano:
>> I think Matthieu is not thinking about the "resolve conflicted merge (or
>> rebase, am) and make a single commit" use case, for which your "pick one
>> of the sides" makes sense.  His is more about "I like some change in that
>> unrelated commit---while I have no intention to replay that change as a
>> whole, I want to use "cherry-pick --no-commit" and pick only the bits out
>> of the change that are useful for what I am doing" use case.
>>
>> In such a context, after resolving conflicts in the working tree, "add -p"
>> would be a good tool to pick hunks in the diff between the HEAD and the
>> working tree.
>
> Ok. Something like this (which is not really that different from Eriks
> example, so I might still be missing something) ?

After re-reading what I wrote, I find that I misspoke.

There is a crucial step missing.  After running "cherry-pick --no-commit",
you would eyeball the result, and typically edit the file.  The conflict
may not be in the part of the file you are interested in picking, in which
case you may even leave that part as-is.  And then, you would pick what
you want to apply to the index during per-hunk interactive patching
session.

In other words,...

> git init .
> echo "foo" > bar.txt
> git add bar.txt
> git commit -m.
> git checkout -b new
> echo "bar" >> bar.txt
> git commit -am.
> git checkout master
> git reset --hard
> echo "baz" >> bar.txt
> git commit -am.
> git cherry-pick --no-commit new

Here, there *will* be "edit bar.txt" before "add -p".  Which makes...

> git add -p bar.txt
>
> So he would be interested in keeping his previous working tree version
> (identical to HEAD commit, otherwise cherry-pick fails)...

...this part of your analysys (and your conclusion that he can take '<')
invalid.

Now I am not Matthieu, so he may have a different use case in mind when he
said "I want to take the version from my working tree", but the sequence
"cherry-pick --no-commit ; edit ; add ; commit" is a natural way to pick
only a part relevant to your current work from unrelated commit, and I
would understand why "Taking the version from my working tree" can be
useful in such a scenario.

In short, these two are totally different workflows and call for two
different semantics.

Re: git add -p and unresolved conflicts

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> There is a crucial step missing.  After running "cherry-pick --no-commit",
> you would eyeball the result, and typically edit the file.  The conflict
> may not be in the part of the file you are interested in picking, in which
> case you may even leave that part as-is.  And then, you would pick what
> you want to apply to the index during per-hunk interactive patching
> session.

Strictly speaking, you do not even have to edit the file.  If the conflict
does not overlap the part of the change you are interested in, a
hypothetical version of "add -p" that is oblivious to merge conflicts
(i.e. running "reset $path" before starting "add -p") could help you apply
the changes the picked commit makes relative to its parents to your
current state.

Re: git add -p and unresolved conflicts

[Holger Hellmuth]

Am 29.03.2012 01:01, schrieb Junio C Hamano:
> There is a crucial step missing.  After running "cherry-pick --no-commit",
> you would eyeball the result, and typically edit the file.  The conflict
> may not be in the part of the file you are interested in picking, in which
> case you may even leave that part as-is.  And then, you would pick what
> you want to apply to the index during per-hunk interactive patching
> session.

But that changes nothing. To make it clear: We have a file with chunks 
A, B and C. Only B is conflicting, but the user wants to add A or an 
edited A´ . And maybe also a new edit D.

When he calls "git add -p", he gets presented with B. By picking the 
"ours" option (aka "<") he gets the version in HEAD, resolving the 
conflict and thereby allowing the add. Next he will be presented with A 
(or A´) where he can type in "y" and C where he can type in "n".

Result: chunk A is added, C not, and B doesn't change from HEAD. Note he 
had the option to use ">" instead if it was B he wanted to cherry-pick.

The same happens if he further edited somewhere else to produce chunk D. 
Again, after resolving the conflict of B, he is presented with A,B and D 
and can add these chunks with "y" or not with "n".

The only case I haven't talked about now is if he edits B, but that is 
the trivial case because he would normally remove the conflict markers 
as well and git add -p would work conflict-free after the edit.

> In other words,...
>
>> git init .
>> echo "foo">  bar.txt
>> git add bar.txt
>> git commit -m.
>> git checkout -b new
>> echo "bar">>  bar.txt
>> git commit -am.
>> git checkout master
>> git reset --hard
>> echo "baz">>  bar.txt
>> git commit -am.
>> git cherry-pick --no-commit new
>
> Here, there *will* be "edit bar.txt" before "add -p".

Re: git add -p and unresolved conflicts

[Matthieu Moy]

Holger Hellmuth <hellmuth@ira.uka.de> writes:

>> And you miss the most usefull (to me at least): "choose the version in
>> the worktree".
>>
>
> But the conflicted chunks are of the form "<<<< our ... ||||||||||
> theirs >>>>>>" in your work tree. So there are two cases:
>
> a) You have removed the markers thereby removing the conflict -> this
> means the chunk will not be offered to you as a conflicting chunk

If you have removed the markers, then the file is still marked as
conflicted in the index, and the user may still want to see the combined
diff.

My use-case is actually quite simple: I see "git add -p" both as a way
to make partial commits, and as a way to manually validate what I'm
about to commit. I review the diff, validate them with "y", and if I see
something wrong, I quit "git add -p", fix the issue, and next "git add
-p" won't show it again. This flow could work also with conflicts:

$ git add -p
# see an unresolved diff for file foo.txt
n
# see a resolved diff for file bar.txt
y        <-- the "missing" possibility for me
$ edit foo.txt
$ git add -p
# see resolution for file foo.txt
y
$ git commit

The "'git reset $path' before 'add -p'" workaround is not really handy
there: it requires typing the path, while my workflow with "git add -p"
does not, and it wouldn't show the combined diff, which is usually the
best tool to see if a merge has been resolved properly.

The current behavior is not that bad for me: I see the combined diff in
the output, I just have one extra "git add $path" to type. Being able to
just say "y" instead would be handy, but not fundamentally different.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: git add -p and unresolved conflicts

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> The "'git reset $path' before 'add -p'" workaround is not really handy
> there: it requires typing the path,...

That is exactly why "add -i" was written without first giving "add -p"
shorthand, which spoiled you and at the same time robbed you the other
parts of "add -i".

Re: git add -p and unresolved conflicts

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> The "'git reset $path' before 'add -p'" workaround is not really handy
>> there: it requires typing the path,...
>
> That is exactly why "add -i" was written without first giving "add -p"
> shorthand, which spoiled you and at the same time robbed you the other
> parts of "add -i".

Quite frankly, I never liked the interface of "git add -i". It saves
very little compared to typing "git status" and then doing stuff
directly from the shell. "git add -p" on the other hand just requires
"y" and "n" most of the time, and that really does what I want.

In the case of conflicts, I don't think "git add -i" solves the problem
"view the diff and mark it as resolved" smoothly.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: git add -p and unresolved conflicts

[Erik Faye-Lund]

On Wed, Mar 28, 2012 at 5:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> I'm running git version 1.7.9.msysgit.0.390.g01fca.dirty, and I've
>> noticed a puzzling behavior with "git add -p" on a conflicted file: it
>> seems to simply spew the diff and exit the process.
>
> Yeah, when I wrote the 'p'atch mode in "add--interactive", I wasn't
> interested at all in letting it be used on a conflicted path, so that is
> not a designed-in behaviour but merely whatever it happens to do.

OK, that's pretty much what I suspected ;)

> So at
> least it should not allow the user to pick a path that has conflicts in
> it.

So, the options are:
 1) Keep the current behavior
 2) Abort with an error immediately if one of the picked paths are in
a conflicted state
 3) Abort with an error when first encounter a path that is in a
conflicted state
 4) Warn and skip any paths that are in a conflicted state
 5) Implement "git add -p" for conflicted files (if we can agree on a behavior)

1) and 3) makes little sense to me. 5) only makes sense if we can
agree on what should have happened.

> What the behaviour for people who *do* want to use the patch mode for
> conflicted paths should be is a separate matter.  As I said, I am not
> interested in it, so I wouldn't be the best person to design it.
>
> I mildly suspect nobody would come up with a sane behaviour, but what
> would I know...

Well, I can explain what I expected to happen:

I had stashed away some of my "hacking"-code to rebase a branch to the
current master. I was going to stage parts of the stashed changes as a
commit on top. However after rebasing, "git stash pop" lead to a
conflict. So I manually resolved the conflicts, but only wanted to
stage some of the hunks to be committed.

Now, what is obvious in hindsight, is that I should simply have done
"git reset" after resolving the conflicts manually. When I do "git add
-p", I add hunks to the index. But for a conflicted file, the index
isn't in a well-defined state (or, the contents of the file is in a
not-yet-defined state).

However, as it happened it made me confused (and a little bit
frustrated). I suspect this could confuse other users as well. Perhaps
we should implement 1) or 3) and come up with a helpful error/warning
message? I don't have a very strong opinion on which one is better,
but I suspect 3) is easier.

Re: git add -p and unresolved conflicts

[Junio C Hamano]

Erik Faye-Lund <kusmabite@gmail.com> writes:

> So, the options are:
>  1) Keep the current behavior
>  2) Abort with an error immediately if one of the picked paths are in
> a conflicted state
>  3) Abort with an error when first encounter a path that is in a
> conflicted state
>  4) Warn and skip any paths that are in a conflicted state
>  5) Implement "git add -p" for conflicted files (if we can agree on a behavior)
>
> 1) and 3) makes little sense to me.

I do not think 1, 2, or 3 makes any sense to me if you are proposing to
change anything.  Why did you even bother to list them?

Given that you wrote "(if we can...)" for #5, in the short term, #4 is the
only sensible thing to do, I would think.