{"thread":{"id":"33989","subject":"[PATCH] cherry-pick: don't barf when there's nothing to do","startedAt":"2013-05-30T13:49:41Z","lastAt":"2013-06-03T23:20:27Z","messageCount":2,"participants":["Felipe Contreras","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"219003","messageId":"1369921781-30667-1-git-send-email-felipe.contreras@gmail.com","threadId":"33989","inReplyTo":null,"subject":"[PATCH] cherry-pick: don't barf when there's nothing to do","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-30T13:49:41Z","receivedAt":"2013-05-30T13:49:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"If the user set --ff, it's expected that if theres's nothing to do we\nfast-forward our current HEAD, which is a no-op.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n sequencer.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex d8f9d30..b9d4b48 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -749,7 +749,7 @@ static void prepare_revs(struct replay_opts *opts)\n \tif (prepare_revision_walk(opts->revs))\n \t\tdie(_(\"revision walk setup failed\"));\n \n-\tif (!opts->revs->commits)\n+\tif (!opts->revs->commits && !opts->allow_ff)\n \t\tdie(_(\"empty commit set passed\"));\n }\n \n-- \n1.8.3.rc3.312.g47657de\n"},{"id":"219314","messageId":"7vtxlebx38.fsf@alter.siamese.dyndns.org","threadId":"33989","inReplyTo":"1369921781-30667-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] cherry-pick: don't barf when there's nothing to do","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-03T23:20:27Z","receivedAt":"2013-06-03T23:20:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> If the user set --ff, it's expected that if theres's nothing to do we\n> fast-forward our current HEAD, which is a no-op.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n\nThis is about \"git cherry-pick topic..master\" which is a misspelt\nform of \"git cherry-pick master..topic\", or alternatively, a script\ncalling \"git cherry-pick ..$branch\" that turns out to be giving an\nempty set, isn't it?\n\nI suspect that it is perfectly fine to turn this die() into a\nwarning or an informational feedback (or be silent when --quiet is\ngiven) and make the command exit with success.  If the user asked\nfor a no-op, the result should be a no-op, but because a no-op is a\nstrange thing to ask for, and is deserved to be warned about.\n\nBut I do not see why we should behave differently between with and\nwithout --ff option.\n\nI also notice this funkiness with \"--ff\":\n\n\tgit.git/master$ git checkout master^\n        git.git/(ed73fe5...)$ git cherry-pick master^0\n        [detached HEAD 32af3ed] Update draft release notes to 1.8.4\n\t 1 file changed, 53 insertions(+)\n\tgit.git/(32af3ed...)$ git checkout master^\n        git.git/(ed73fe5...)$ git cherry-pick --ff master^0\n        git.git/(b2edae0...)$\n\nThe last cherry-pick shows nothing.  The user can notice that\nsomething useful has happened only because git-prompt script is in\nuse (and he is experimenting on a detached HEAD), but otherwise\nthere is no feedback.  I think it should, unless given --quiet, say\nsomething like \"fast-forwarded to ...\" to avoid user confusion.\n\n>  sequencer.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index d8f9d30..b9d4b48 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -749,7 +749,7 @@ static void prepare_revs(struct replay_opts *opts)\n>  \tif (prepare_revision_walk(opts->revs))\n>  \t\tdie(_(\"revision walk setup failed\"));\n>  \n> -\tif (!opts->revs->commits)\n> +\tif (!opts->revs->commits && !opts->allow_ff)\n>  \t\tdie(_(\"empty commit set passed\"));\n>  }\n"}]}