This patch 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().
In case of successful merge, do_recursive_merge() used to print
something like "Finished one cherry-pick." but nothing was
printed in case a special strategy like "resolve" was used.
Now in this case a message like "Finished one cherry-pick with
strategy resolve." is printed.
So the command behavior should be more consistent wether we are
using a special strategy or not.
Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
---
I also wonder why messages like "Finished one cherry-pick."
are printed on stderr instead of stdout.
Maybe this could be changed in this patch or in
another one.
And while at making a backward incompatible change, maybe
we could change the message to something like:
"Finished cherry-pick <sha1>"
builtin/revert.c | 51 ++++++++++++++++++++---------------
t/t3508-cherry-pick-many-commits.sh | 26 +++++++++++++++++-
2 files changed, 54 insertions(+), 23 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 8b9d829..b082bb4 100644
--- 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 f90ed3d..6555f92 100755
--- 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)"
'
--
1.7.2.rc1.213.gf3fb81
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html