revert: report success when using option --strategy

"git cherry-pick foo" has always reported success with
"Finished one cherry-pick" but "cherry-pick --strategy"
does not print anything. So move the code to write that
message from do_recursive_merge() to do_cherry_pick()
so other strategies can share it.

This patch also refactors the code that prints a message
like "Automatic cherry-pick failed. <help message>". This
code was duplicated in both do_recursive_merge() and
do_pick_commit().

To do that, now do_recursive_merge() returns an int to signal
success or failure. And in case of failure we just return 1
from do_pick_commit() instead of doing "exit(1)" from either
do_recursive_merge() or do_pick_commit().

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/revert.c b/builtin/revert.c
index 8b9d829a73a0862dfdf2c2499b18878489620f16..b082bb425c6e865a3e5d7bae00b9da68e0933517 100644 (file)
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -311,10 +311,9 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from)
        return write_ref_sha1(ref_lock, to, "cherry-pick");
 }
 
-static void do_recursive_merge(struct commit *base, struct commit *next,
-                              const char *base_label, const char *next_label,
-                              unsigned char *head, struct strbuf *msgbuf,
-                              char *defmsg)
+static int do_recursive_merge(struct commit *base, struct commit *next,
+                             const char *base_label, const char *next_label,
+                             unsigned char *head, struct strbuf *msgbuf)
 {
        struct merge_options o;
        struct tree *result, *next_tree, *base_tree, *head_tree;
@@ -357,14 +356,9 @@ static void do_recursive_merge(struct commit *base, struct commit *next,
                                        i++;
                        }
                }
-               write_message(msgbuf, defmsg);
-               fprintf(stderr, "Automatic %s failed.%s\n",
-                       me, help_msg());
-               rerere(allow_rerere_auto);
-               exit(1);
        }
-       write_message(msgbuf, defmsg);
-       fprintf(stderr, "Finished one %s.\n", me);
+
+       return !clean;
 }
 
 static int do_pick_commit(void)
@@ -375,6 +369,8 @@ static int do_pick_commit(void)
        struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
        char *defmsg = NULL;
        struct strbuf msgbuf = STRBUF_INIT;
+       struct strbuf mebuf = STRBUF_INIT;
+       int res;
 
        if (no_commit) {
                /*
@@ -470,30 +466,41 @@ static int do_pick_commit(void)
                }
        }
 
-       if (!strategy || !strcmp(strategy, "recursive") || action == REVERT)
-               do_recursive_merge(base, next, base_label, next_label,
-                                  head, &msgbuf, defmsg);
-       else {
-               int res;
+       strbuf_addstr(&mebuf, me);
+
+       if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
+               res = do_recursive_merge(base, next, base_label, next_label,
+                                        head, &msgbuf);
+               write_message(&msgbuf, defmsg);
+       } else {
                struct commit_list *common = NULL;
                struct commit_list *remotes = NULL;
+
+               strbuf_addf(&mebuf, " with strategy %s", strategy);
                write_message(&msgbuf, defmsg);
+
                commit_list_insert(base, &common);
                commit_list_insert(next, &remotes);
                res = try_merge_command(strategy, common,
                                        sha1_to_hex(head), remotes);
                free_commit_list(common);
                free_commit_list(remotes);
-               if (res) {
-                       fprintf(stderr, "Automatic %s with strategy %s failed.%s\n",
-                               me, strategy, help_msg());
-                       rerere(allow_rerere_auto);
-                       exit(1);
-               }
        }
 
+       if (res) {
+               fprintf(stderr, "Automatic %s failed.%s\n",
+                       mebuf.buf, help_msg());
+               rerere(allow_rerere_auto);
+       } else {
+               fprintf(stderr, "Finished one %s.\n", mebuf.buf);
+       }
+
+       strbuf_release(&mebuf);
        free_message(&msg);
 
+       if (res)
+               return 1;
+
        /*
         *
         * If we are cherry-pick, and if the merge did not result in

diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index f90ed3da3e0cb93b63d32d8002dc6d9bd3596a65..d90b365fd2efbe24f877d6695b87996f618963e4 100755 (executable)
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -23,12 +23,36 @@ test_expect_success setup '
 '
 
 test_expect_success 'cherry-pick first..fourth works' '
+       cat <<-\EOF >expected &&
+       Finished one cherry-pick.
+       Finished one cherry-pick.
+       Finished one cherry-pick.
+       EOF
+
+       git checkout -f master &&
+       git reset --hard first &&
+       test_tick &&
+       git cherry-pick first..fourth 2>actual &&
+       git diff --quiet other &&
+       git diff --quiet HEAD other &&
+       test_cmp expected actual &&
+       test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
+'
+
+test_expect_success 'cherry-pick --strategy resolve first..fourth works' '
+       cat <<-\EOF >expected &&
+       Finished one cherry-pick with strategy resolve.
+       Finished one cherry-pick with strategy resolve.
+       Finished one cherry-pick with strategy resolve.
+       EOF
+
        git checkout -f master &&
        git reset --hard first &&
        test_tick &&
-       git cherry-pick first..fourth &&
+       git cherry-pick --strategy resolve first..fourth 2>actual &&
        git diff --quiet other &&
        git diff --quiet HEAD other &&
+       test_cmp expected actual &&
        test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
 '