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>
Christian Couder committed
Jul 14, 2010 at 01:28 UTC
7b53b92fdb22b90d2be558db84c725641c4ad170
2 files changed
+54
-23
builtin/revert.c
+29
-22
@@ -311,10 +311,9 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from)
311
return write_ref_sha1(ref_lock, to, "cherry-pick");
312
}
313
314
-static void do_recursive_merge(struct commit *base, struct commit *next,
315
- const char *base_label, const char *next_label,
316
- unsigned char *head, struct strbuf *msgbuf,
317
- char *defmsg)
314
+static int do_recursive_merge(struct commit *base, struct commit *next,
315
+ const char *base_label, const char *next_label,
316
+ unsigned char *head, struct strbuf *msgbuf)
317
{
318
struct merge_options o;
319
struct tree *result, *next_tree, *base_tree, *head_tree;
@@ -357,14 +356,9 @@ static void do_recursive_merge(struct commit *base, struct commit *next,
356
i++;
357
}
358
}
360
- write_message(msgbuf, defmsg);
361
- fprintf(stderr, "Automatic %s failed.%s\n",
362
- me, help_msg());
363
- rerere(allow_rerere_auto);
364
- exit(1);
359
}
366
- write_message(msgbuf, defmsg);
367
- fprintf(stderr, "Finished one %s.\n", me);
360
+
361
+ return !clean;
362
}
363
364
static int do_pick_commit(void)
@@ -375,6 +369,8 @@ static int do_pick_commit(void)
369
struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
370
char *defmsg = NULL;
371
struct strbuf msgbuf = STRBUF_INIT;
372
+ struct strbuf mebuf = STRBUF_INIT;
373
+ int res;
374
375
if (no_commit) {
376
/*
@@ -470,30 +466,41 @@ static int do_pick_commit(void)
466
}
467
}
468
473
- if (!strategy || !strcmp(strategy, "recursive") || action == REVERT)
474
- do_recursive_merge(base, next, base_label, next_label,
475
- head, &msgbuf, defmsg);
476
- else {
477
- int res;
469
+ strbuf_addstr(&mebuf, me);
470
+
471
+ if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
472
+ res = do_recursive_merge(base, next, base_label, next_label,
473
+ head, &msgbuf);
474
+ write_message(&msgbuf, defmsg);
475
+ } else {
476
struct commit_list *common = NULL;
477
struct commit_list *remotes = NULL;
478
+
479
+ strbuf_addf(&mebuf, " with strategy %s", strategy);
480
write_message(&msgbuf, defmsg);
481
+
482
commit_list_insert(base, &common);
483
commit_list_insert(next, &remotes);
484
res = try_merge_command(strategy, common,
485
sha1_to_hex(head), remotes);
486
free_commit_list(common);
487
free_commit_list(remotes);
487
- if (res) {
488
- fprintf(stderr, "Automatic %s with strategy %s failed.%s\n",
489
- me, strategy, help_msg());
490
- rerere(allow_rerere_auto);
491
- exit(1);
492
- }
488
}
489
490
+ if (res) {
491
+ fprintf(stderr, "Automatic %s failed.%s\n",
492
+ mebuf.buf, help_msg());
493
+ rerere(allow_rerere_auto);
494
+ } else {
495
+ fprintf(stderr, "Finished one %s.\n", mebuf.buf);
496
+ }
497
+
498
+ strbuf_release(&mebuf);
499
free_message(&msg);
500
501
+ if (res)
502
+ return 1;
503
+
504
/*
505
*
506
* If we are cherry-pick, and if the merge did not result in
t/t3508-cherry-pick-many-commits.sh
+25
-1
@@ -23,12 +23,36 @@ test_expect_success setup '
23
'
24
25
test_expect_success 'cherry-pick first..fourth works' '
26
+ cat <<-\EOF >expected &&
27
+ Finished one cherry-pick.
28
+ Finished one cherry-pick.
29
+ Finished one cherry-pick.
30
+ EOF
31
+
32
+ git checkout -f master &&
33
+ git reset --hard first &&
34
+ test_tick &&
35
+ git cherry-pick first..fourth 2>actual &&
36
+ git diff --quiet other &&
37
+ git diff --quiet HEAD other &&
38
+ test_cmp expected actual &&
39
+ test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
40
+'
41
+
42
+test_expect_success 'cherry-pick --strategy resolve first..fourth works' '
43
+ cat <<-\EOF >expected &&
44
+ Finished one cherry-pick with strategy resolve.
45
+ Finished one cherry-pick with strategy resolve.
46
+ Finished one cherry-pick with strategy resolve.
47
+ EOF
48
+
49
git checkout -f master &&
50
git reset --hard first &&
51
test_tick &&
29
- git cherry-pick first..fourth &&
52
+ git cherry-pick --strategy resolve first..fourth 2>actual &&
53
git diff --quiet other &&
54
git diff --quiet HEAD other &&
55
+ test_cmp expected actual &&
56
test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
57
'
58