From 4c9941943bebbe23039254cdda576ecd97e1d397 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 12 Feb 2013 02:17:30 -0800 Subject: t/test-lib-functions.sh: allow to specify the tag name to test_commit The part of test_commit() may not be appropriate for a tag name. So let's allow test_commit to accept a fourth argument to specify the tag name. Signed-off-by: Brandon Casey Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/test-lib-functions.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 't') diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 8889ba5104..7c4c633592 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -135,12 +135,12 @@ test_pause () { fi } -# Call test_commit with the arguments " [ []]" +# Call test_commit with the arguments " [ [ []]]" # # This will commit a file with the given contents and the given commit -# message. It will also add a tag with as name. +# message, and tag the resulting commit with the given tag name. # -# Both and default to . +# , , and all default to . test_commit () { notick= && @@ -168,7 +168,7 @@ test_commit () { test_tick fi && git commit $signoff -m "$1" && - git tag "$1" + git tag "${4:-$1}" } # Call test_merge with the arguments " ", where -- cgit v1.3 From f2b9a7555bb7791fa8d137da033b403d16af5085 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 12 Feb 2013 02:17:31 -0800 Subject: t/t3511: add some tests of 'cherry-pick -s' functionality Add some tests to ensure that 'cherry-pick -s' operates in the following manner: * Inserts a blank line before appending a s-o-b to a commit message that does not contain a s-o-b footer * Does not mistake first line "subject: description" as a s-o-b footer * Does not mistake single word message body as conforming to rfc2822 * Appends a s-o-b when last s-o-b in footer does not match committer s-o-b, even when committer's s-o-b exists elsewhere in footer. * Does not append a s-o-b when last s-o-b matches committer s-o-b * Correctly detects a non-conforming footer containing a mix of s-o-b like elements and s-o-b elements. (marked "expect failure") Signed-off-by: Brandon Casey Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t3511-cherry-pick-x.sh | 111 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100755 t/t3511-cherry-pick-x.sh (limited to 't') diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh new file mode 100755 index 0000000000..2a040b7664 --- /dev/null +++ b/t/t3511-cherry-pick-x.sh @@ -0,0 +1,111 @@ +#!/bin/sh + +test_description='Test cherry-pick -x and -s' + +. ./test-lib.sh + +pristine_detach () { + git cherry-pick --quit && + git checkout -f "$1^0" && + git read-tree -u --reset HEAD && + git clean -d -f -f -q -x +} + +mesg_one_line='base: commit message' + +mesg_no_footer="$mesg_one_line + +OneWordBodyThatsNotA-S-o-B" + +mesg_with_footer="$mesg_no_footer + +Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> +Signed-off-by: A.U. Thor +Signed-off-by: B.U. Thor " + +mesg_broken_footer="$mesg_no_footer + +The signed-off-by string should begin with the words Signed-off-by followed +by a colon and space, and then the signers name and email address. e.g. +Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" + +mesg_with_footer_sob="$mesg_with_footer +Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" + + +test_expect_success setup ' + git config advice.detachedhead false && + echo unrelated >unrelated && + git add unrelated && + test_commit initial foo a && + test_commit "$mesg_one_line" foo b mesg-one-line && + git reset --hard initial && + test_commit "$mesg_no_footer" foo b mesg-no-footer && + git reset --hard initial && + test_commit "$mesg_broken_footer" foo b mesg-broken-footer && + git reset --hard initial && + test_commit "$mesg_with_footer" foo b mesg-with-footer && + git reset --hard initial && + test_commit "$mesg_with_footer_sob" foo b mesg-with-footer-sob && + pristine_detach initial && + test_commit conflicting unrelated +' + +test_expect_success 'cherry-pick -s inserts blank line after one line subject' ' + pristine_detach initial && + git cherry-pick -s mesg-one-line && + cat <<-EOF >expect && + $mesg_one_line + + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + +test_expect_failure 'cherry-pick -s inserts blank line after non-conforming footer' ' + pristine_detach initial && + git cherry-pick -s mesg-broken-footer && + cat <<-EOF >expect && + $mesg_broken_footer + + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + +test_expect_success 'cherry-pick -s inserts blank line when conforming footer not found' ' + pristine_detach initial && + git cherry-pick -s mesg-no-footer && + cat <<-EOF >expect && + $mesg_no_footer + + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + +test_expect_success 'cherry-pick -s adds sob when last sob doesnt match committer' ' + pristine_detach initial && + git cherry-pick -s mesg-with-footer && + cat <<-EOF >expect && + $mesg_with_footer + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + +test_expect_success 'cherry-pick -s refrains from adding duplicate trailing sob' ' + pristine_detach initial && + git cherry-pick -s mesg-with-footer-sob && + cat <<-EOF >expect && + $mesg_with_footer_sob + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + +test_done -- cgit v1.3 From cd650a4eee148dadb0943f415e88c210f18e70df Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 12 Feb 2013 02:17:32 -0800 Subject: sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer When 'cherry-pick -s' is used to append a signed-off-by line to a cherry picked commit, it does not currently detect the "(cherry picked from..." that may have been appended by a previous 'cherry-pick -x' as part of the s-o-b footer and it will insert a blank line before appending a new s-o-b. Let's detect "(cherry picked from...)" as part of the footer so that we will produce this: Signed-off-by: A U Thor (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709) Signed-off-by: C O Mmitter instead of this: Signed-off-by: A U Thor (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709) Signed-off-by: C O Mmitter [with improvements from Jonathan Nieder] Signed-off-by: Brandon Casey Acked-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- sequencer.c | 51 ++++++++++++++++++++++++++++++++------------ t/t3511-cherry-pick-x.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 14 deletions(-) (limited to 't') diff --git a/sequencer.c b/sequencer.c index 1a80dac6c5..7889b6bc19 100644 --- a/sequencer.c +++ b/sequencer.c @@ -18,6 +18,7 @@ #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" const char sign_off_header[] = "Signed-off-by: "; +static const char cherry_picked_prefix[] = "(cherry picked from commit "; static void remove_sequencer_state(void) { @@ -492,7 +493,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) } if (opts->record_origin) { - strbuf_addstr(&msgbuf, "(cherry picked from commit "); + strbuf_addstr(&msgbuf, cherry_picked_prefix); strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1)); strbuf_addstr(&msgbuf, ")\n"); } @@ -1017,16 +1018,44 @@ int sequencer_pick_revisions(struct replay_opts *opts) return pick_commits(todo_list, opts); } -static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer) +static int is_rfc2822_line(const char *buf, int len) { - char ch, prev; - int i, j, k; + int i; + + for (i = 0; i < len; i++) { + int ch = buf[i]; + if (ch == ':') + return 1; + if (!isalnum(ch) && ch != '-') + break; + } + + return 0; +} + +static int is_cherry_picked_from_line(const char *buf, int len) +{ + /* + * We only care that it looks roughly like (cherry picked from ...) + */ + return len > strlen(cherry_picked_prefix) + 1 && + !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')'; +} + +static int has_conforming_footer(struct strbuf *sb, int ignore_footer) +{ + char prev; + int i, k; int len = sb->len - ignore_footer; const char *buf = sb->buf; + /* footer must end with newline */ + if (!len || buf[len - 1] != '\n') + return 0; + prev = '\0'; for (i = len - 1; i > 0; i--) { - ch = buf[i]; + char ch = buf[i]; if (prev == '\n' && ch == '\n') /* paragraph break */ break; prev = ch; @@ -1041,15 +1070,9 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer) ; /* do nothing */ k++; - for (j = 0; i + j < len; j++) { - ch = buf[i + j]; - if (ch == ':') - break; - if (isalnum(ch) || - (ch == '-')) - continue; + if (!is_rfc2822_line(buf + i, k - i - 1) && + !is_cherry_picked_from_line(buf + i, k - i - 1)) return 0; - } } return 1; } @@ -1066,7 +1089,7 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer) for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--) ; /* do nothing */ if (prefixcmp(msgbuf->buf + i, sob.buf)) { - if (!i || !ends_rfc2822_footer(msgbuf, ignore_footer)) + if (!i || !has_conforming_footer(msgbuf, ignore_footer)) strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1); strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len); } diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh index 2a040b7664..73da182e1a 100755 --- a/t/t3511-cherry-pick-x.sh +++ b/t/t3511-cherry-pick-x.sh @@ -32,6 +32,10 @@ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" mesg_with_footer_sob="$mesg_with_footer Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" +mesg_with_cherry_footer="$mesg_with_footer_sob +(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709) +Tested-by: C.U. Thor " + test_expect_success setup ' git config advice.detachedhead false && @@ -47,6 +51,8 @@ test_expect_success setup ' test_commit "$mesg_with_footer" foo b mesg-with-footer && git reset --hard initial && test_commit "$mesg_with_footer_sob" foo b mesg-with-footer-sob && + git reset --hard initial && + test_commit "$mesg_with_cherry_footer" foo b mesg-with-cherry-footer && pristine_detach initial && test_commit conflicting unrelated ' @@ -98,6 +104,19 @@ test_expect_success 'cherry-pick -s adds sob when last sob doesnt match committe test_cmp expect actual ' +test_expect_success 'cherry-pick -x -s adds sob when last sob doesnt match committer' ' + pristine_detach initial && + sha1=`git rev-parse mesg-with-footer^0` && + git cherry-pick -x -s mesg-with-footer && + cat <<-EOF >expect && + $mesg_with_footer + (cherry picked from commit $sha1) + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + test_expect_success 'cherry-pick -s refrains from adding duplicate trailing sob' ' pristine_detach initial && git cherry-pick -s mesg-with-footer-sob && @@ -108,4 +127,40 @@ test_expect_success 'cherry-pick -s refrains from adding duplicate trailing sob' test_cmp expect actual ' +test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists for committer' ' + pristine_detach initial && + sha1=`git rev-parse mesg-with-footer-sob^0` && + git cherry-pick -x -s mesg-with-footer-sob && + cat <<-EOF >expect && + $mesg_with_footer_sob + (cherry picked from commit $sha1) + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + +test_expect_success 'cherry-pick -x treats "(cherry picked from..." line as part of footer' ' + pristine_detach initial && + sha1=`git rev-parse mesg-with-cherry-footer^0` && + git cherry-pick -x mesg-with-cherry-footer && + cat <<-EOF >expect && + $mesg_with_cherry_footer + (cherry picked from commit $sha1) + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + +test_expect_success 'cherry-pick -s treats "(cherry picked from..." line as part of footer' ' + pristine_detach initial && + git cherry-pick -s mesg-with-cherry-footer && + cat <<-EOF >expect && + $mesg_with_cherry_footer + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + test_done -- cgit v1.3 From b971e04f54e73a5a2af54ad62540273bb90ee7bb Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 12 Feb 2013 02:17:34 -0800 Subject: sequencer.c: always separate "(cherry picked from" from commit body Start treating the "(cherry picked from" line added by cherry-pick -x the same way that the s-o-b lines are treated. Namely, separate them from the main commit message body with an empty line. Introduce tests to test this functionality. Signed-off-by: Brandon Casey Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- sequencer.c | 128 ++++++++++++++++++++++++----------------------- t/t3511-cherry-pick-x.sh | 53 ++++++++++++++++++++ 2 files changed, 118 insertions(+), 63 deletions(-) (limited to 't') diff --git a/sequencer.c b/sequencer.c index 5a9c43d806..f9f9f8a772 100644 --- a/sequencer.c +++ b/sequencer.c @@ -20,6 +20,69 @@ const char sign_off_header[] = "Signed-off-by: "; static const char cherry_picked_prefix[] = "(cherry picked from commit "; +static int is_rfc2822_line(const char *buf, int len) +{ + int i; + + for (i = 0; i < len; i++) { + int ch = buf[i]; + if (ch == ':') + return 1; + if (!isalnum(ch) && ch != '-') + break; + } + + return 0; +} + +static int is_cherry_picked_from_line(const char *buf, int len) +{ + /* + * We only care that it looks roughly like (cherry picked from ...) + */ + return len > strlen(cherry_picked_prefix) + 1 && + !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')'; +} + +static int has_conforming_footer(struct strbuf *sb, int ignore_footer) +{ + char prev; + int i, k; + int len = sb->len - ignore_footer; + const char *buf = sb->buf; + + /* footer must end with newline */ + if (!len || buf[len - 1] != '\n') + return 0; + + prev = '\0'; + for (i = len - 1; i > 0; i--) { + char ch = buf[i]; + if (prev == '\n' && ch == '\n') /* paragraph break */ + break; + prev = ch; + } + + /* require at least one blank line */ + if (prev != '\n' || buf[i] != '\n') + return 0; + + /* advance to start of last paragraph */ + while (i < len - 1 && buf[i] == '\n') + i++; + + for (; i < len; i = k) { + for (k = i; k < len && buf[k] != '\n'; k++) + ; /* do nothing */ + k++; + + if (!is_rfc2822_line(buf + i, k - i - 1) && + !is_cherry_picked_from_line(buf + i, k - i - 1)) + return 0; + } + return 1; +} + static void remove_sequencer_state(void) { struct strbuf seq_dir = STRBUF_INIT; @@ -493,6 +556,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) } if (opts->record_origin) { + if (!has_conforming_footer(&msgbuf, 0)) + strbuf_addch(&msgbuf, '\n'); strbuf_addstr(&msgbuf, cherry_picked_prefix); strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1)); strbuf_addstr(&msgbuf, ")\n"); @@ -1018,69 +1083,6 @@ int sequencer_pick_revisions(struct replay_opts *opts) return pick_commits(todo_list, opts); } -static int is_rfc2822_line(const char *buf, int len) -{ - int i; - - for (i = 0; i < len; i++) { - int ch = buf[i]; - if (ch == ':') - return 1; - if (!isalnum(ch) && ch != '-') - break; - } - - return 0; -} - -static int is_cherry_picked_from_line(const char *buf, int len) -{ - /* - * We only care that it looks roughly like (cherry picked from ...) - */ - return len > strlen(cherry_picked_prefix) + 1 && - !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')'; -} - -static int has_conforming_footer(struct strbuf *sb, int ignore_footer) -{ - char prev; - int i, k; - int len = sb->len - ignore_footer; - const char *buf = sb->buf; - - /* footer must end with newline */ - if (!len || buf[len - 1] != '\n') - return 0; - - prev = '\0'; - for (i = len - 1; i > 0; i--) { - char ch = buf[i]; - if (prev == '\n' && ch == '\n') /* paragraph break */ - break; - prev = ch; - } - - /* require at least one blank line */ - if (prev != '\n' || buf[i] != '\n') - return 0; - - /* advance to start of last paragraph */ - while (i < len - 1 && buf[i] == '\n') - i++; - - for (; i < len; i = k) { - for (k = i; k < len && buf[k] != '\n'; k++) - ; /* do nothing */ - k++; - - if (!is_rfc2822_line(buf + i, k - i - 1) && - !is_cherry_picked_from_line(buf + i, k - i - 1)) - return 0; - } - return 1; -} - void append_signoff(struct strbuf *msgbuf, int ignore_footer) { struct strbuf sob = STRBUF_INIT; diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh index 73da182e1a..a845e45d40 100755 --- a/t/t3511-cherry-pick-x.sh +++ b/t/t3511-cherry-pick-x.sh @@ -57,6 +57,19 @@ test_expect_success setup ' test_commit conflicting unrelated ' +test_expect_success 'cherry-pick -x inserts blank line after one line subject' ' + pristine_detach initial && + sha1=`git rev-parse mesg-one-line^0` && + git cherry-pick -x mesg-one-line && + cat <<-EOF >expect && + $mesg_one_line + + (cherry picked from commit $sha1) + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + test_expect_success 'cherry-pick -s inserts blank line after one line subject' ' pristine_detach initial && git cherry-pick -s mesg-one-line && @@ -81,6 +94,19 @@ test_expect_failure 'cherry-pick -s inserts blank line after non-conforming foot test_cmp expect actual ' +test_expect_success 'cherry-pick -x inserts blank line when conforming footer not found' ' + pristine_detach initial && + sha1=`git rev-parse mesg-no-footer^0` && + git cherry-pick -x mesg-no-footer && + cat <<-EOF >expect && + $mesg_no_footer + + (cherry picked from commit $sha1) + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + test_expect_success 'cherry-pick -s inserts blank line when conforming footer not found' ' pristine_detach initial && git cherry-pick -s mesg-no-footer && @@ -93,6 +119,20 @@ test_expect_success 'cherry-pick -s inserts blank line when conforming footer no test_cmp expect actual ' +test_expect_success 'cherry-pick -x -s inserts blank line when conforming footer not found' ' + pristine_detach initial && + sha1=`git rev-parse mesg-no-footer^0` && + git cherry-pick -x -s mesg-no-footer && + cat <<-EOF >expect && + $mesg_no_footer + + (cherry picked from commit $sha1) + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + test_expect_success 'cherry-pick -s adds sob when last sob doesnt match committer' ' pristine_detach initial && git cherry-pick -s mesg-with-footer && @@ -163,4 +203,17 @@ test_expect_success 'cherry-pick -s treats "(cherry picked from..." line as part test_cmp expect actual ' +test_expect_success 'cherry-pick -x -s treats "(cherry picked from..." line as part of footer' ' + pristine_detach initial && + sha1=`git rev-parse mesg-with-cherry-footer^0` && + git cherry-pick -x -s mesg-with-cherry-footer && + cat <<-EOF >expect && + $mesg_with_cherry_footer + (cherry picked from commit $sha1) + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + test_done -- cgit v1.3 From bab4d1097c8be7d688a53b992232063dbf300fd4 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 12 Feb 2013 02:17:35 -0800 Subject: sequencer.c: teach append_signoff how to detect duplicate s-o-b Teach append_signoff how to detect a duplicate s-o-b in the commit footer. This is in preparation to unify the append_signoff implementations in log-tree.c and sequencer.c. Fixes test in t3511. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- builtin/commit.c | 2 +- sequencer.c | 59 ++++++++++++++++++++++++++++++++++++------------ sequencer.h | 4 +++- t/t3511-cherry-pick-x.sh | 2 +- 4 files changed, 50 insertions(+), 17 deletions(-) (limited to 't') diff --git a/builtin/commit.c b/builtin/commit.c index 1dd2ec5e6f..7b9e2ac2eb 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -700,7 +700,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, previous = eol; } - append_signoff(&sb, ignore_footer); + append_signoff(&sb, ignore_footer, 0); } if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len) diff --git a/sequencer.c b/sequencer.c index f9f9f8a772..965b954baf 100644 --- a/sequencer.c +++ b/sequencer.c @@ -44,12 +44,20 @@ static int is_cherry_picked_from_line(const char *buf, int len) !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')'; } -static int has_conforming_footer(struct strbuf *sb, int ignore_footer) +/* + * Returns 0 for non-conforming footer + * Returns 1 for conforming footer + * Returns 2 when sob exists within conforming footer + * Returns 3 when sob exists within conforming footer as last entry + */ +static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, + int ignore_footer) { char prev; int i, k; int len = sb->len - ignore_footer; const char *buf = sb->buf; + int found_sob = 0; /* footer must end with newline */ if (!len || buf[len - 1] != '\n') @@ -72,14 +80,25 @@ static int has_conforming_footer(struct strbuf *sb, int ignore_footer) i++; for (; i < len; i = k) { + int found_rfc2822; + for (k = i; k < len && buf[k] != '\n'; k++) ; /* do nothing */ k++; - if (!is_rfc2822_line(buf + i, k - i - 1) && - !is_cherry_picked_from_line(buf + i, k - i - 1)) + found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1); + if (found_rfc2822 && sob && + !strncmp(buf + i, sob->buf, sob->len)) + found_sob = k; + + if (!(found_rfc2822 || + is_cherry_picked_from_line(buf + i, k - i - 1))) return 0; } + if (found_sob == i) + return 3; + if (found_sob) + return 2; return 1; } @@ -300,7 +319,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next, rollback_lock_file(&index_lock); if (opts->signoff) - append_signoff(msgbuf, 0); + append_signoff(msgbuf, 0, 0); if (!clean) { int i; @@ -556,7 +575,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) } if (opts->record_origin) { - if (!has_conforming_footer(&msgbuf, 0)) + if (!has_conforming_footer(&msgbuf, NULL, 0)) strbuf_addch(&msgbuf, '\n'); strbuf_addstr(&msgbuf, cherry_picked_prefix); strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1)); @@ -1083,21 +1102,33 @@ int sequencer_pick_revisions(struct replay_opts *opts) return pick_commits(todo_list, opts); } -void append_signoff(struct strbuf *msgbuf, int ignore_footer) +void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag) { + unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP; struct strbuf sob = STRBUF_INIT; - int i; + int has_footer; strbuf_addstr(&sob, sign_off_header); strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"), getenv("GIT_COMMITTER_EMAIL"))); strbuf_addch(&sob, '\n'); - for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--) - ; /* do nothing */ - if (prefixcmp(msgbuf->buf + i, sob.buf)) { - if (!has_conforming_footer(msgbuf, ignore_footer)) - strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1); - strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len); - } + + /* + * If the whole message buffer is equal to the sob, pretend that we + * found a conforming footer with a matching sob + */ + if (msgbuf->len - ignore_footer == sob.len && + !strncmp(msgbuf->buf, sob.buf, sob.len)) + has_footer = 3; + else + has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer); + + if (!has_footer) + strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1); + + if (has_footer != 3 && (!no_dup_sob || has_footer != 2)) + strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, + sob.buf, sob.len); + strbuf_release(&sob); } diff --git a/sequencer.h b/sequencer.h index 9d57d57524..1fc22dcabe 100644 --- a/sequencer.h +++ b/sequencer.h @@ -6,6 +6,8 @@ #define SEQ_TODO_FILE "sequencer/todo" #define SEQ_OPTS_FILE "sequencer/opts" +#define APPEND_SIGNOFF_DEDUP (1u << 0) + enum replay_action { REPLAY_REVERT, REPLAY_PICK @@ -48,6 +50,6 @@ int sequencer_pick_revisions(struct replay_opts *opts); extern const char sign_off_header[]; -void append_signoff(struct strbuf *msgbuf, int ignore_footer); +void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag); #endif diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh index a845e45d40..f97727975b 100755 --- a/t/t3511-cherry-pick-x.sh +++ b/t/t3511-cherry-pick-x.sh @@ -82,7 +82,7 @@ test_expect_success 'cherry-pick -s inserts blank line after one line subject' ' test_cmp expect actual ' -test_expect_failure 'cherry-pick -s inserts blank line after non-conforming footer' ' +test_expect_success 'cherry-pick -s inserts blank line after non-conforming footer' ' pristine_detach initial && git cherry-pick -s mesg-broken-footer && cat <<-EOF >expect && -- cgit v1.3 From 79133a66f79e70dab88f9f0558dd029bf02f2ac9 Mon Sep 17 00:00:00 2001 From: Nguyễn Thái Ngọc Duy Date: Tue, 12 Feb 2013 02:17:37 -0800 Subject: t4014: more tests about appending s-o-b lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [bc: Squash the tests from Duy's original unify-appending-sob series. Fix test 90 "signoff: some random signoff-alike" and mark as failing. Correct behavior should insert a blank line after message body and signed-off-by. Add two additional tests: 1. failure to detect non-conforming elements in the footer when last line matches committer's s-o-b. 2. ensure various s-o-b -like elements in the footer are handled as conforming. e.g. "Change-id: IXXXX or Bug: 1234" ] Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- t/t4014-format-patch.sh | 241 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 241 insertions(+) (limited to 't') diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 16a4ca1d60..0ed899f0f2 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -963,4 +963,245 @@ test_expect_success 'format patch ignores color.ui' ' test_cmp expect actual ' +append_signoff() +{ + C=$(git commit-tree HEAD^^{tree} -p HEAD) && + git format-patch --stdout --signoff $C^..$C >append_signoff.patch && + sed -n -e "1,/^---$/p" append_signoff.patch | + egrep -n "^Subject|Sign|^$" +} + +test_expect_success 'signoff: commit with no body' ' + append_signoff actual && + cat <<\EOF | sed "s/EOL$//" >expected && +4:Subject: [PATCH] EOL +8: +9:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_success 'signoff: commit with only subject' ' + echo subject | append_signoff >actual && + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +9:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_success 'signoff: commit with only subject that does not end with NL' ' + printf subject | append_signoff >actual && + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +9:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_success 'signoff: no existing signoffs' ' + append_signoff <<\EOF >actual && +subject + +body +EOF + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +10: +11:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_success 'signoff: no existing signoffs and no trailing NL' ' + printf "subject\n\nbody" | append_signoff >actual && + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +10: +11:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_success 'signoff: some random signoff' ' + append_signoff <<\EOF >actual && +subject + +body + +Signed-off-by: my@house +EOF + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +10: +11:Signed-off-by: my@house +12:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_failure 'signoff: some random signoff-alike' ' + append_signoff <<\EOF >actual && +subject + +body +Fooled-by-me: my@house +EOF + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +11: +12:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_failure 'signoff: not really a signoff' ' + append_signoff <<\EOF >actual && +subject + +I want to mention about Signed-off-by: here. +EOF + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +9:I want to mention about Signed-off-by: here. +10: +11:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_failure 'signoff: not really a signoff (2)' ' + append_signoff <<\EOF >actual && +subject + +My unfortunate +Signed-off-by: example happens to be wrapped here. +EOF + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +10:Signed-off-by: example happens to be wrapped here. +11: +12:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_failure 'signoff: valid S-o-b paragraph in the middle' ' + append_signoff <<\EOF >actual && +subject + +Signed-off-by: my@house +Signed-off-by: your@house + +A lot of houses. +EOF + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +9:Signed-off-by: my@house +10:Signed-off-by: your@house +11: +13: +14:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_success 'signoff: the same signoff at the end' ' + append_signoff <<\EOF >actual && +subject + +body + +Signed-off-by: C O Mitter +EOF + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +10: +11:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_success 'signoff: the same signoff at the end, no trailing NL' ' + printf "subject\n\nSigned-off-by: C O Mitter " | + append_signoff >actual && + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +9:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_success 'signoff: the same signoff NOT at the end' ' + append_signoff <<\EOF >actual && +subject + +body + +Signed-off-by: C O Mitter +Signed-off-by: my@house +EOF + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +10: +11:Signed-off-by: C O Mitter +12:Signed-off-by: my@house +EOF + test_cmp expected actual +' + +test_expect_failure 'signoff: detect garbage in non-conforming footer' ' + append_signoff <<\EOF >actual && +subject + +body + +Tested-by: my@house +Some Trash +Signed-off-by: C O Mitter +EOF + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +10: +13:Signed-off-by: C O Mitter +14: +15:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_success 'signoff: footer begins with non-signoff without @ sign' ' + append_signoff <<\EOF >actual && +subject + +body + +Reviewed-id: Noone +Tested-by: my@house +Change-id: Ideadbeef +Signed-off-by: C O Mitter +Bug: 1234 +EOF + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +10: +14:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + test_done -- cgit v1.3 From 959a26231f53b9cc7da40d36c550aef193647731 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 12 Feb 2013 02:17:39 -0800 Subject: Unify appending signoff in format-patch, commit and sequencer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are two implementations of append_signoff in log-tree.c and sequencer.c, which do more or less the same thing. Unify on top of the sequencer.c implementation. Add a test in t4014 to demonstrate support for non-s-o-b elements in the commit footer provided by sequence.c:append_sob. Mark tests fixed as appropriate. [Commit message mostly stolen from Nguyễn Thái Ngọc Duy's original unification patch] Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- log-tree.c | 91 +------------------------------------------------ t/t4014-format-patch.sh | 31 ++++++++++++++--- 2 files changed, 27 insertions(+), 95 deletions(-) (limited to 't') diff --git a/log-tree.c b/log-tree.c index e3044a3e94..cf274058ca 100644 --- a/log-tree.c +++ b/log-tree.c @@ -9,8 +9,7 @@ #include "string-list.h" #include "color.h" #include "gpg-interface.h" - -#define APPEND_SIGNOFF_DEDUP (1u <<0) +#include "sequencer.h" struct decoration name_decoration = { "object names" }; @@ -208,94 +207,6 @@ void show_decorations(struct rev_info *opt, struct commit *commit) putchar(')'); } -/* - * Search for "^[-A-Za-z]+: [^@]+@" pattern. It usually matches - * Signed-off-by: and Acked-by: lines. - */ -static int detect_any_signoff(char *letter, int size) -{ - char *cp; - int seen_colon = 0; - int seen_at = 0; - int seen_name = 0; - int seen_head = 0; - - cp = letter + size; - while (letter <= --cp && *cp == '\n') - continue; - - while (letter <= cp) { - char ch = *cp--; - if (ch == '\n') - break; - - if (!seen_at) { - if (ch == '@') - seen_at = 1; - continue; - } - if (!seen_colon) { - if (ch == '@') - return 0; - else if (ch == ':') - seen_colon = 1; - else - seen_name = 1; - continue; - } - if (('A' <= ch && ch <= 'Z') || - ('a' <= ch && ch <= 'z') || - ch == '-') { - seen_head = 1; - continue; - } - /* no empty last line doesn't match */ - return 0; - } - return seen_head && seen_name; -} - -static void append_signoff(struct strbuf *sb, int ignore_footer, unsigned flag) -{ - unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP; - static const char signed_off_by[] = "Signed-off-by: "; - char *signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"), - getenv("GIT_COMMITTER_EMAIL"))); - size_t signoff_len = strlen(signoff); - int has_signoff = 0; - char *cp; - - cp = sb->buf; - - /* First see if we already have the sign-off by the signer */ - while ((cp = strstr(cp, signed_off_by))) { - - has_signoff = 1; - - cp += strlen(signed_off_by); - if (cp + signoff_len >= sb->buf + sb->len) - break; - if (strncmp(cp, signoff, signoff_len)) - continue; - if (!isspace(cp[signoff_len])) - continue; - /* we already have him */ - free(signoff); - return; - } - - if (!has_signoff) - has_signoff = detect_any_signoff(sb->buf, sb->len); - - if (!has_signoff) - strbuf_addch(sb, '\n'); - - strbuf_addstr(sb, signed_off_by); - strbuf_add(sb, signoff, signoff_len); - strbuf_addch(sb, '\n'); - free(signoff); -} - static unsigned int digits_in_number(unsigned int number) { unsigned int i = 10, result = 1; diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 0ed899f0f2..95af73f596 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1045,7 +1045,28 @@ EOF test_cmp expected actual ' -test_expect_failure 'signoff: some random signoff-alike' ' +test_expect_success 'signoff: misc conforming footer elements' ' + append_signoff <<\EOF >actual && +subject + +body + +Signed-off-by: my@house +(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709) +Tested-by: Some One +Bug: 1234 +EOF + cat >expected <<\EOF && +4:Subject: [PATCH] subject +8: +10: +11:Signed-off-by: my@house +15:Signed-off-by: C O Mitter +EOF + test_cmp expected actual +' + +test_expect_success 'signoff: some random signoff-alike' ' append_signoff <<\EOF >actual && subject @@ -1061,7 +1082,7 @@ EOF test_cmp expected actual ' -test_expect_failure 'signoff: not really a signoff' ' +test_expect_success 'signoff: not really a signoff' ' append_signoff <<\EOF >actual && subject @@ -1077,7 +1098,7 @@ EOF test_cmp expected actual ' -test_expect_failure 'signoff: not really a signoff (2)' ' +test_expect_success 'signoff: not really a signoff (2)' ' append_signoff <<\EOF >actual && subject @@ -1094,7 +1115,7 @@ EOF test_cmp expected actual ' -test_expect_failure 'signoff: valid S-o-b paragraph in the middle' ' +test_expect_success 'signoff: valid S-o-b paragraph in the middle' ' append_signoff <<\EOF >actual && subject @@ -1162,7 +1183,7 @@ EOF test_cmp expected actual ' -test_expect_failure 'signoff: detect garbage in non-conforming footer' ' +test_expect_success 'signoff: detect garbage in non-conforming footer' ' append_signoff <<\EOF >actual && subject -- cgit v1.3 From 8c613fd5efa1c3bbd48845ed91c533192e699c08 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Fri, 22 Feb 2013 14:05:27 -0800 Subject: git-commit: populate the edit buffer with 2 blank lines before s-o-b 'commit -s' populates the edit buffer with a blank line before the Signed-off-by line, to allow the user to immediately start typing the log message. But commit 33f2f9ab removed this space, forcing the user to first push the Signed-off-by line down to open a place to type the log message. Fix this regression and let's ensure that the Signed-off-by line is preceded by two blank lines, instead of just one, to hint that something should be filled in, and that a blank line should separate it from the body and the Signed-off-by line. Add a test for this behavior. Reported-by: John Keeping Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- sequencer.c | 27 +++++++++++++++++++++++++-- t/t7502-commit.sh | 12 ++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) (limited to 't') diff --git a/sequencer.c b/sequencer.c index 53ee49a9b0..a07d2d00b9 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1127,10 +1127,33 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag) const char *append_newlines = NULL; size_t len = msgbuf->len - ignore_footer; - if (len && msgbuf->buf[len - 1] != '\n') + if (!len) { + /* + * The buffer is completely empty. Leave foom for + * the title and body to be filled in by the user. + */ append_newlines = "\n\n"; - else if (len > 1 && msgbuf->buf[len - 2] != '\n') + } else if (msgbuf->buf[len - 1] != '\n') { + /* + * Incomplete line. Complete the line and add a + * blank one so that there is an empty line between + * the message body and the sob. + */ + append_newlines = "\n\n"; + } else if (len == 1) { + /* + * Buffer contains a single newline. Add another + * so that we leave room for the title and body. + */ + append_newlines = "\n"; + } else if (msgbuf->buf[len - 2] != '\n') { + /* + * Buffer ends with a single newline. Add another + * so that there is an empty line between the message + * body and the sob. + */ append_newlines = "\n"; + } /* else, the buffer already ends with two newlines. */ if (append_newlines) strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index deb187eb7b..c06a7526c1 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -349,6 +349,18 @@ test_expect_success 'A single-liner subject with a token plus colon is not a foo ' +test_expect_success 'commit -s places sob on third line after two empty lines' ' + git commit -s --allow-empty --allow-empty-message && + cat <<-EOF >expect && + + + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + + EOF + sed -e "/^#/d" -e "s/^:.*//" .git/COMMIT_EDITMSG >actual && + test_cmp expect actual +' + write_script .git/FAKE_EDITOR <<\EOF mv "$1" "$1.orig" ( -- cgit v1.3