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 <author@example.com>
(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
Signed-off-by: C O Mmitter <committer@example.com>
instead of this:

Signed-off-by: A U Thor <author@example.com>
(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)

Signed-off-by: C O Mmitter <committer@example.com>
[with improvements from Jonathan Nieder]

Signed-off-by: Brandon Casey <bcasey@nvidia.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sequencer.c b/sequencer.c
index 1a80dac6c52b876f5f90d026d671fae61bf423fa..7889b6bc19c0a9e5b81203ea5465f21ed6306d51 100644 (file)
--- 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 2a040b7664d7b3ed72154e1ba8c8599913b2b2a1..73da182e1a0294de7f9dd7cdeb88536acf4131c0 100755 (executable)
--- 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 <cuthor@example.com>"
+
 
 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