Re: [PATCH 3/6] revert: don't let revert continue a cherry-pick

[Jonathan Nieder <jrnieder@gmail.com>]

Ramkumar Ramachandra wrote:
> Jonathan Nieder wrote:

>> I don't know --- it's not confusing to me.  Could you explain further
>> what harm the current behavior does?  E.g., could it cause me to
>> misunderstand some basic concepts, or could it lead me to run commands
>> that cause me to scratch my head or lose data?
>
> Junio explained this to me in [1].  It's very unnatural for a user to
> want to execute "git cherry-pick --continue" when the previous command
> was a "git revert": it probably means that she forgot about the
> in-progress "git revert".
[...]
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/185355

I don't think that's what Junio said.

Did this actually happen, or is it a theoretical worry?  I think I would
be more likely to run "git cherry-pick <foo>..<bar>" than "git
cherry-pick --continue" if I had forgotten about an in-progress
revert.  The former already errors out with a sensible message.

Or is the problem that I might run:

	git revert foo..bar
	git reset --merge; # conflict --- let's clean this up

	# ah, I remember reverting the patch that conflicted before;
	# let's reuse the resolution.
	git cherry-pick baz
	edit file.c; # another conflict, sigh
	git add file.c
	git cherry-pick --continue; # oops!

?  That seems like a real worry, but the same problem could happen
with cherry-pick used both for the multipick and single-pick, so I
don't think your patch fundamentally addresses it.

In other words, this is a problem caused by the overloading of the
same cherry-pick command for single-pick and multi-pick.  I think it
should be preventable by remembering which action failed when stopping
a sequence and doing only a single-pick resume if
CHERRY_PICK_HEAD/REVERT_HEAD/whatever doesn't match that.

The "oops" is bad since the operator might have been intending to run
some more tests and amend as necessary before continuing the
multi-pick.  It is not _that_ bad, since more typically one would have
already run some tests before running cherry-pick --continue to commit
the resolution.  Still probably worth fixing.

> The problem becomes more serious when the
> sequencer grows more capabilities: a "git merge --continue" to
> continue a "git am" sounds much more absurd.  Ofcourse, we will
> provide a way to continue any sequencer operation in the future: "git
> continue" seems to be a good candidate.

I don't understand why "cherry-pick --continue" resuming a revert
sequence implies that "merge --continue" would have to as well.

All that said, forbidding cherry-pick --continue from resuming a
revert sequence would be fine with me, _as long as the semantics are
clearly spelled out in the commit message and documentation_.  What
happens when there is a mixture of picks and reverts?

Thanks.
Jonathan