Re: [PATCH] rebase -i: remove CHERRY_PICK_HEAD when cherry-pick failed

[Jonathan Nieder <jrnieder@gmail.com>]

(cc-ing Jay, expert on the CHERRY_PICK_HEAD facility)
Hi all,

Ramkumar Ramachandra wrote:
> Andrew Wong wrote:

>> Instead of having the sequencer catch errors and remove CHERRY_PICK_HEAD
>> for its caller's sake, let its caller do the work. This way, the
>> sequencer doesn't have to check all points of failures where its caller
>> doesn't want CHERRY_PICK_HEAD.
>
> This part makes sense.

Sorry, I think I've missed the point.  Can you explain to me what
problem this is solving, aside from somehow dividing responsibility
for the CHERRY_PICK_HEAD file among different tools?

>From the surrounding thread, it looks like the following sequence of
commands is at stake:

	git checkout -b tmp v1.7.9
	git cherry-pick 6e1c9bb^2^
	git rebase -i --onto 6e1c9bb HEAD^
	git rebase --continue

The pick works fine and is just part of the setup.  The rebase produces

	The previous cherry-pick is now empty, possibly due to conflict resolution.
	If you wish to commit it anyway, use:

	    git commit --allow-empty

	Otherwise, please use 'git reset'

CHERRY_PICK_HEAD points to "run-command: optionally kill children on
exit" to help the user understand how to resolve the conflict.
Normally print_advice() would remove it because the caller has set
GIT_CHERRY_PICK_HELP to indicate that it wants to use some other
mechanism than "git commit" to deal with resolved conflicts.
Unfortunately the GIT_CHERRY_PICK_HELP facility does not give the
caller a way to specify an alternative message for this case, like:

	The previous cherry-pick is now empty, possibly due to conflict resolution.

	When you have resolved this problem run "git rebase --continue".
	If you would prefer to skip this patch, instead run "git rebase --skip".
	To check out the original branch and stop rebasing run "git rebase --abort".

In fact, "git cherry-pick" does not handle this case at all.  It lets
"git commit" notice the lack of change.  "git commit" emits a message
and follows the usual rules for a failed commit, including preserving
CHERRY_PICK_HEAD to help the operator clean up.

Ok.  Now the user (sensibly) ignores the message from cherry-pick and
just runs "git rebase --continue".  The rebase finishes but nobody
feels it's his responsibility to remove the .git/CHERRY_PICK_HEAD file
and it gets left behind.

For symptom relief, your patch makes sense, though I haven't checked
it in detail yet.  The description distracted me --- it would be
better to say "this sequence of commands has this bad consequence;
this patch papers over the problem to make people happier until the
underlying problem can be addressed" instead of pretending the design
was almost sane and we are just fixing the last detail. ;-)

I suspect a more appropriate long-term fix would involve "git
cherry-pick" noticing when a patch has resolved to nothing instead of
leaving it to "git commit" to detect that.

Sensible?
Jonathan