Re: [PATCH v3 1/4] git-cherry-pick: add allow-empty option

[Neil Horman <nhorman@tuxdriver.com>]

On Tue, Apr 10, 2012 at 02:09:17PM -0700, Junio C Hamano wrote:
> Neil Horman <nhorman@tuxdriver.com> writes:
> 
> > ...  What do you say to my proposal regarding the splitting of the device
> > dependent on how we were executed?  It seems we can't use a single advice string
> > in this case, as no matter which we choose there is a use case in which it fails
> > to make sense.
> 
> When your cherry-pick got --allow-empty, it should pass --allow-empty to
> its inner invocation of commit if it is picking an originally empty
> commit, and this advice will not trigger because commit will happily
> commit the no-change change.
> 
> When your cherry-pick got --allow-empty but not --keep-unnecessary-commit,
> its inner invocation of commit must not pass --allow-empty if it is _not_
> picking an originally empty commit.  Then the inner commit will fail if it
> auto resolves to no change, and the user sees the advice.  The current
> advice text is appropriate for this case.
> 
> When your cherry-pick did not get either of these flags, its inner
> invocation of commit must not pass --allow-empty.  The user sees the
> advice when the auto resolved result matches HEAD from the commit invoked
> by cherry-pick.  The current advice text is fine for this case, as we say
> "possibly", not "we definitely know it was due to conflict resolution".
> 
> Or your cherry-pick may have failed due to a conflict, regardless of the
> options like --allow-empty or --keep-unnecessary-commit given to it, and
> the user may have run commit after resolving the conflict.  The current
> advice text is fine for this case, too, as we say "possibly", and it
> indeed is what just happened.
> 
> So I do not think you need to change anything with respect to the advice
> message.
> 
> Am I missing some other cases?
> 
No, you covered all the cases, but I disagree with your assertion that the advice
is correct (or at least optimal) in any of these cases. If a cherry-pick without
any options is preformed and the commit is empty (regardless of the reason), the
advice given is that git commit --allow-empty should be used.  With the addition
of these new options, thats not true any longer.  Instead of using git commit
--allow-empty, you can use git cherry-pick --allow-empty.

Given your description above though, I'm ok rolling this back.  Regardless of my
disagreement, git commit --allow-empty still works just as well after the
addition of these options, so while I still think its awkward to give git commit
advice on the result of a git cherry-pick operation,  its not any more
problematic than the issues you've pointed out with my change.  I'll roll this
back in my next version and will look for a way to provide better advice in the
future.

Regards
Neil

>