Thread (64 messages) flat view 64 messages, 2 authors, 2016-06-15

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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:52:44

Ramkumar Ramachandra wrote:
Jonathan Nieder wrote:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help