Re: [PATCH] revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so

From: Jay Soffian <jaysoffian@gmail.com>
Date: Oct 6, 2011, 17:33 UTC
Message-ID: <CAG+J_Dx0W0oRL-MkoZSGuOfmUNCqmMRUigmgND_0o7kbKppu1Q@mail.gmail.com>
In-Reply-To: <CAG+J_DxqpDAm-qPR-Udkr_b1gc=Y+LoKbsQdmiCi6ztNKz0_Mg@mail.gmail.com>

On Thu, Oct 6, 2011 at 1:28 PM, Jay Soffian <jaysoffian@gmail.com> wrote:

Show 20 quoted lines

> On Thu, Oct 6, 2011 at 1:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> I think do_recursive_merge() would die() when the merge cannot even start
>> (i.e. the local changes after the cherry-pick exits are the ones from the
>> time before such a failed cherry-pick started), but I suspect that the
>> other codepath uses try_merge_command() to drive strategies other than
>> recursive and it does not die() there in such a case. Can you make sure
>> this patch is sufficient in such a case as well?
>
> It's wrong to write out CHERRY_PICK_HEAD if we couldn't start the
> merge, but using cherry-pick with a strategy other than recursive
> seems broken in that case in other ways as well:
>
> $ git cherry-pick --strategy=resolve side
> error: Untracked working tree file 'bar' would be overwritten by merge.
> error: could not apply a77535c9ac... bar
> hint: after resolving the conflicts, mark the corrected paths
> hint: with 'git add <paths>' or 'git rm <paths>'
> hint: and commit the result with 'git commit'
>
> The "hint" advice is completely wrong.

For that matter, I don't see how to distinguish "the merge did not even start" from "the merge had conflicts" when using try_merge_command(). In the former case we do NOT want CHERRY_PICK_HEAD left behind (nor to print the wrong advice above), while in the latter case we do.

j.