cherry-pick: don't forget -s on failure
In case 'git cherry-pick -s <commit>' failed, the user had to use 'git commit -s' (i.e. state the -s option again), which is easy to forget about. Instead, write the signed-off-by line early, so plain 'git commit' will have the same result. Also update 'git commit -s', so that in case there is already a relevant Signed-off-by line before the Conflicts: line, it won't add one more at the end of the message. If there is no such line, then add it before the the Conflicts: line. Signed-off-by: Miklos Vajna <vmiklos@suse.cz> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Miklos Vajna committed
Sep 14, 2012 at 08:52 UTC
5ed75e2a3fb30f93fea7772e481ec6091e9a2c5f
6 files changed
+144
-65
builtin/commit.c
+24
-57
@@ -28,6 +28,7 @@
28
#include "submodule.h"
29
#include "gpg-interface.h"
30
#include "column.h"
31
+#include "sequencer.h"
32
33
static const char * const builtin_commit_usage[] = {
34
N_("git commit [options] [--] <filepattern>..."),
@@ -466,8 +467,6 @@ static int is_a_merge(const struct commit *current_head)
467
return !!(current_head->parents && current_head->parents->next);
468
}
469
469
-static const char sign_off_header[] = "Signed-off-by: ";
470
-
470
static void export_one(const char *var, const char *s, const char *e, int hack)
471
{
472
struct strbuf buf = STRBUF_INIT;
@@ -552,47 +551,6 @@ static void determine_author_info(struct strbuf *author_ident)
551
}
552
}
553
555
-static int ends_rfc2822_footer(struct strbuf *sb)
556
-{
557
- int ch;
558
- int hit = 0;
559
- int i, j, k;
560
- int len = sb->len;
561
- int first = 1;
562
- const char *buf = sb->buf;
563
-
564
- for (i = len - 1; i > 0; i--) {
565
- if (hit && buf[i] == '\n')
566
- break;
567
- hit = (buf[i] == '\n');
568
- }
569
-
570
- while (i < len - 1 && buf[i] == '\n')
571
- i++;
572
-
573
- for (; i < len; i = k) {
574
- for (k = i; k < len && buf[k] != '\n'; k++)
575
- ; /* do nothing */
576
- k++;
577
-
578
- if ((buf[k] == ' ' || buf[k] == '\t') && !first)
579
- continue;
580
-
581
- first = 0;
582
-
583
- for (j = 0; i + j < len; j++) {
584
- ch = buf[i + j];
585
- if (ch == ':')
586
- break;
587
- if (isalnum(ch) ||
588
- (ch == '-'))
589
- continue;
590
- return 0;
591
- }
592
- }
593
- return 1;
594
-}
595
-
554
static char *cut_ident_timestamp_part(char *string)
555
{
556
char *ket = strrchr(string, '>');
@@ -717,21 +675,30 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
675
stripspace(&sb, 0);
676
677
if (signoff) {
720
- struct strbuf sob = STRBUF_INIT;
721
- int i;
722
-
723
- strbuf_addstr(&sob, sign_off_header);
724
- strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"),
725
- getenv("GIT_COMMITTER_EMAIL")));
726
- strbuf_addch(&sob, '\n');
727
- for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
728
- ; /* do nothing */
729
- if (prefixcmp(sb.buf + i, sob.buf)) {
730
- if (!i || !ends_rfc2822_footer(&sb))
731
- strbuf_addch(&sb, '\n');
732
- strbuf_addbuf(&sb, &sob);
678
+ /*
679
+ * See if we have a Conflicts: block at the end. If yes, count
680
+ * its size, so we can ignore it.
681
+ */
682
+ int ignore_footer = 0;
683
+ int i, eol, previous = 0;
684
+ const char *nl;
685
+
686
+ for (i = 0; i < sb.len; i++) {
687
+ nl = memchr(sb.buf + i, '\n', sb.len - i);
688
+ if (nl)
689
+ eol = nl - sb.buf;
690
+ else
691
+ eol = sb.len;
692
+ if (!prefixcmp(sb.buf + previous, "\nConflicts:\n")) {
693
+ ignore_footer = sb.len - previous;
694
+ break;
695
+ }
696
+ while (i < eol)
697
+ i++;
698
+ previous = eol;
699
}
734
- strbuf_release(&sob);
700
+
701
+ append_signoff(&sb, ignore_footer);
702
}
703
704
if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
sequencer.c
+65
@@ -17,6 +17,8 @@
17
18
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
19
20
+const char sign_off_header[] = "Signed-off-by: ";
21
+
22
void remove_sequencer_state(void)
23
{
24
struct strbuf seq_dir = STRBUF_INIT;
@@ -233,6 +235,9 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
235
die(_("%s: Unable to write new index file"), action_name(opts));
236
rollback_lock_file(&index_lock);
237
238
+ if (opts->signoff)
239
+ append_signoff(msgbuf, 0);
240
+
241
if (!clean) {
242
int i;
243
strbuf_addstr(msgbuf, "\nConflicts:\n");
@@ -1011,3 +1016,63 @@ int sequencer_pick_revisions(struct replay_opts *opts)
1016
save_opts(opts);
1017
return pick_commits(todo_list, opts);
1018
}
1019
+
1020
+static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
1021
+{
1022
+ int ch;
1023
+ int hit = 0;
1024
+ int i, j, k;
1025
+ int len = sb->len - ignore_footer;
1026
+ int first = 1;
1027
+ const char *buf = sb->buf;
1028
+
1029
+ for (i = len - 1; i > 0; i--) {
1030
+ if (hit && buf[i] == '\n')
1031
+ break;
1032
+ hit = (buf[i] == '\n');
1033
+ }
1034
+
1035
+ while (i < len - 1 && buf[i] == '\n')
1036
+ i++;
1037
+
1038
+ for (; i < len; i = k) {
1039
+ for (k = i; k < len && buf[k] != '\n'; k++)
1040
+ ; /* do nothing */
1041
+ k++;
1042
+
1043
+ if ((buf[k] == ' ' || buf[k] == '\t') && !first)
1044
+ continue;
1045
+
1046
+ first = 0;
1047
+
1048
+ for (j = 0; i + j < len; j++) {
1049
+ ch = buf[i + j];
1050
+ if (ch == ':')
1051
+ break;
1052
+ if (isalnum(ch) ||
1053
+ (ch == '-'))
1054
+ continue;
1055
+ return 0;
1056
+ }
1057
+ }
1058
+ return 1;
1059
+}
1060
+
1061
+void append_signoff(struct strbuf *msgbuf, int ignore_footer)
1062
+{
1063
+ struct strbuf sob = STRBUF_INIT;
1064
+ int i;
1065
+
1066
+ strbuf_addstr(&sob, sign_off_header);
1067
+ strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"),
1068
+ getenv("GIT_COMMITTER_EMAIL")));
1069
+ strbuf_addch(&sob, '\n');
1070
+ for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--)
1071
+ ; /* do nothing */
1072
+ if (prefixcmp(msgbuf->buf + i, sob.buf)) {
1073
+ if (!i || !ends_rfc2822_footer(msgbuf, ignore_footer))
1074
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
1075
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len);
1076
+ }
1077
+ strbuf_release(&sob);
1078
+}
sequencer.h
+4
@@ -49,4 +49,8 @@ extern void remove_sequencer_state(void);
49
50
int sequencer_pick_revisions(struct replay_opts *opts);
51
52
+extern const char sign_off_header[];
53
+
54
+void append_signoff(struct strbuf *msgbuf, int ignore_footer);
55
+
56
#endif
t/t3507-cherry-pick-conflict.sh
+32
@@ -30,6 +30,7 @@ test_expect_success setup '
30
test_commit initial foo a &&
31
test_commit base foo b &&
32
test_commit picked foo c &&
33
+ test_commit --signoff picked-signed foo d &&
34
git config advice.detachedhead false
35
36
'
@@ -340,4 +341,35 @@ test_expect_success 'revert conflict, diff3 -m style' '
341
test_cmp expected actual
342
'
343
344
+test_expect_success 'failed cherry-pick does not forget -s' '
345
+ pristine_detach initial &&
346
+ test_must_fail git cherry-pick -s picked &&
347
+ test_i18ngrep -e "Signed-off-by" .git/MERGE_MSG
348
+'
349
+
350
+test_expect_success 'commit after failed cherry-pick does not add duplicated -s' '
351
+ pristine_detach initial &&
352
+ test_must_fail git cherry-pick -s picked-signed &&
353
+ git commit -a -s &&
354
+ test $(git show -s |grep -c "Signed-off-by") = 1
355
+'
356
+
357
+test_expect_success 'commit after failed cherry-pick adds -s at the right place' '
358
+ pristine_detach initial &&
359
+ test_must_fail git cherry-pick picked &&
360
+ git commit -a -s &&
361
+ pwd &&
362
+ cat <<EOF > expected &&
363
+picked
364
+
365
+Signed-off-by: C O Mitter <committer@example.com>
366
+
367
+Conflicts:
368
+ foo
369
+EOF
370
+
371
+ git show -s --pretty=format:%B > actual &&
372
+ test_cmp expected actual
373
+'
374
+
375
test_done
t/t3510-cherry-pick-sequence.sh
+3
-3
@@ -410,7 +410,7 @@ test_expect_success '--continue respects -x in first commit in multi-pick' '
410
grep "cherry picked from.*$picked" msg
411
'
412
413
-test_expect_success '--signoff is not automatically propagated to resolved conflict' '
413
+test_expect_failure '--signoff is automatically propagated to resolved conflict' '
414
pristine_detach initial &&
415
test_expect_code 1 git cherry-pick --signoff base..anotherpick &&
416
echo "c" >foo &&
@@ -428,7 +428,7 @@ test_expect_success '--signoff is not automatically propagated to resolved confl
428
grep "Signed-off-by:" anotherpick_msg
429
'
430
431
-test_expect_success '--signoff dropped for implicit commit of resolution, multi-pick case' '
431
+test_expect_failure '--signoff dropped for implicit commit of resolution, multi-pick case' '
432
pristine_detach initial &&
433
test_must_fail git cherry-pick -s picked anotherpick &&
434
echo c >foo &&
@@ -441,7 +441,7 @@ test_expect_success '--signoff dropped for implicit commit of resolution, multi-
441
! grep Signed-off-by: msg
442
'
443
444
-test_expect_success 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '
444
+test_expect_failure 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '
445
pristine_detach initial &&
446
test_must_fail git cherry-pick -s picked &&
447
echo c >foo &&
t/test-lib-functions.sh
+16
-5
@@ -144,11 +144,22 @@ test_pause () {
144
145
test_commit () {
146
notick= &&
147
- if test "z$1" = "z--notick"
148
- then
149
- notick=yes
147
+ signoff= &&
148
+ while test $# != 0
149
+ do
150
+ case "$1" in
151
+ --notick)
152
+ notick=yes
153
+ ;;
154
+ --signoff)
155
+ signoff="$1"
156
+ ;;
157
+ *)
158
+ break
159
+ ;;
160
+ esac
161
shift
151
- fi &&
162
+ done &&
163
file=${2:-"$1.t"} &&
164
echo "${3-$1}" > "$file" &&
165
git add "$file" &&
@@ -156,7 +167,7 @@ test_commit () {
167
then
168
test_tick
169
fi &&
159
- git commit -m "$1" &&
170
+ git commit $signoff -m "$1" &&
171
git tag "$1"
172
}
173