[PATCH] commit -s: allow "(cherry picked " lines in sign-off section

[Jonathan Nieder <jrnieder@gmail.com>]

Using cherry-pick -x -s to backport a public commit results in
an unsightly gap in the sign-off chain:

	Reported-by: Jarek Poplawski <jarkao2@gmail.com>
	Tested-by: Jarek Poplawski <jarkao2@gmail.com>
	Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
	Cc: Jeff Mahoney <jeffm@suse.com>
	Cc: All since 2.6.32 <stable@kernel.org>
	Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
	Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)

	Signed-off-by: Back Porter <backporter@example.com>

The cherry-pick is a step in the line of a patch like any other,
so one might prefer to lose the extra newline.

	...
	Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
	Signed-off-by: Back Porter <backporter@example.com>

This commit teaches "git commit --signoff", and thus cherry-pick -s,
to do exactly that.  It works by treating the "(cherry picked" line as
just another line in the signoff chain, except as the first line (that
last exception is to avoid false positives).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jeff King wrote:

> Even better, I wonder if it should actually be:
> 
>   message subject
> 
>   Message body.
> 
>   Signed-off-by: Jeff King <peff@peff.net>
>   Cherry-picked-from: ...

Here's something like that.  I use "git cherry-pick -x -s" to
backport patches from a public upstream.  Now you can, too.

Ideally inline notes like

	 [akpm@linux-foundation.org: coding-style fixes]

also ought to be tolerated.

 builtin/commit.c               |   20 +++++++
 t/t3510-cherry-pick-message.sh |  112 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 132 insertions(+), 0 deletions(-)
 create mode 100755 t/t3510-cherry-pick-message.sh

diff --git a/builtin/commit.c b/builtin/commit.c
index 66fdd22..71dd52b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -528,6 +528,8 @@ static int ends_rfc2822_footer(struct strbuf *sb)
 		i++;
 
 	for (; i < len; i = k) {
+		static const char cherry_pick[] = "(cherry picked from commit ";
+
 		for (k = i; k < len && buf[k] != '\n'; k++)
 			; /* do nothing */
 		k++;
@@ -535,6 +537,20 @@ static int ends_rfc2822_footer(struct strbuf *sb)
 		if ((buf[k] == ' ' || buf[k] == '\t') && !first)
 			continue;
 
+		if (!first && buf[k] == '(' && k + strlen(cherry_pick) < len) {
+			/* Might be a cherry-pick notice. */
+			const char *p = buf + k;
+			if (!memcmp(p, cherry_pick, strlen(cherry_pick))) {
+				p = memchr(buf + k, '\n', len - k);
+				if (!p)
+					return 0;
+				if (p + 1 == buf + len)
+					return 1;
+				k = p - buf;
+				continue;
+			}
+		}
+
 		first = 0;
 
 		for (j = 0; i + j < len; j++) {
@@ -625,6 +641,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
 			; /* do nothing */
 		if (prefixcmp(sb.buf + i, sob.buf)) {
+			/*
+			 * Only insert an extra newline if the previous line
+			 * is not part of a Signed-off-by:/Acked-by:/etc chain.
+			 */
 			if (!i || !ends_rfc2822_footer(&sb))
 				strbuf_addch(&sb, '\n');
 			strbuf_addbuf(&sb, &sob);
diff --git a/t/t3510-cherry-pick-message.sh b/t/t3510-cherry-pick-message.sh
new file mode 100755
index 0000000..83e50a3
--- /dev/null
+++ b/t/t3510-cherry-pick-message.sh
@@ -0,0 +1,112 @@
+#!/bin/sh
+
+test_description='tests for the log messages cherry-pick produces
+
+  ----
+  +      cherry-pick of branch
+     +   signoff
+    +    basic
+  ++     mainline
+  +++    initial
+'
+. ./test-lib.sh
+
+prepare_commit () {
+	git checkout initial &&
+	test_commit "$1" &&
+	test_tick &&
+	git commit --amend --allow-empty-message -F "$1.message" &&
+	git tag -d "$1" &&
+	git tag "$1"
+}
+
+test_cmp_message () {
+	expect=$1 &&
+	shift &&
+	git log -1 --pretty=format:%B "$@" >actual &&
+	test_cmp "$expect" actual
+}
+
+cat >basic.message <<\EOF
+ A branch
+
+Here comes the lovely description of a change to pick up.
+Contributions come from many people:
+EOF
+
+{
+	cat basic.message
+	cat <<-\EOF
+
+	Signed-off-by: Foo <foo@example.com>
+	Signed-off-by: Bar <bar@example.com>
+	Tested-by: Baz <baz@example.com>
+	EOF
+} >signoff.message
+
+test_expect_success 'setup' '
+	test_commit initial &&
+	prepare_commit basic &&
+	prepare_commit signoff
+'
+
+test_expect_success 'cherry-pick preserves message' '
+	cat basic.message >expect &&
+	git checkout initial &&
+	git cherry-pick basic &&
+	test_cmp_message expect
+'
+
+test_expect_success 'cherry-pick -s adds signoff' '
+	{
+		cat basic.message &&
+		echo &&
+		echo "Signed-off-by: C O Mitter <committer@example.com>"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -s basic &&
+	test_cmp_message expect HEAD
+'
+
+test_expect_success 'cherry-pick -s integrates into existing signoff chain' '
+	{
+		cat signoff.message &&
+		echo "Signed-off-by: C O Mitter <committer@example.com>"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -s signoff &&
+	test_cmp_message expect HEAD
+'
+
+test_expect_success 'cherry-pick -x adds old commit id' '
+	{
+		cat basic.message &&
+		echo "(cherry picked from commit $(git rev-parse basic^0))"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -x basic &&
+	test_cmp_message expect HEAD
+'
+
+test_expect_success 'cherry-pick -x integrates into signoff chain' '
+	{
+		cat signoff.message &&
+		echo "(cherry picked from commit $(git rev-parse signoff^0))"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -x signoff &&
+	test_cmp_message expect HEAD
+'
+
+test_expect_success 'cherry-pick -x -s' '
+	{
+		cat signoff.message &&
+		echo "(cherry picked from commit $(git rev-parse signoff^0))"
+		echo "Signed-off-by: C O Mitter <committer@example.com>"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -x -s signoff &&
+	test_cmp_message expect HEAD
+'
+
+test_done
-- 
1.7.2.3.551.g13682.dirty