git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 05/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer

From
Brandon Casey <drafnel@gmail.com>
Date
Jan 28, 2013, 01:11 UTC
Message-ID
<1359335515-13818-6-git-send-email-drafnel@gmail.com>
In-Reply-To
<1359335515-13818-1-git-send-email-drafnel@gmail.com>

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>
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
---
 sequencer.c              | 46 ++++++++++++++++++++++++++++------------
 t/t3511-cherry-pick-x.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 13 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 39a752b..0b5cd18 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)
 {
@@ -496,7 +497,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");
 		}
@@ -1021,11 +1022,36 @@ 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)
+{
+	int i;
+
+	for (i = 0; i < len; i++) {
+		int ch = buf[i];
+		if (ch == ':')
+			break;
+		if (isalnum(ch) || (ch == '-'))
+			continue;
+		return 0;
+	}
+
+	return 1;
+}
+
+static int is_cherry_picked_from_line(const char *buf, int len)
+{
+	/*
+	 * We only care that it looks roughly like (cherry picked from ...)
+	 */
+	return !prefixcmp(buf, cherry_picked_prefix) &&
+		(buf[len - 1] == ')' ||
+		 (buf[len - 1] == '\n' && buf[len - 2] == ')'));
+}
+
+static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
 {
-	int ch;
 	int last_char_was_nl, this_char_is_nl;
-	int i, j, k;
+	int i, k;
 	int len = sb->len - ignore_footer;
 	const char *buf = sb->buf;
 
@@ -1046,15 +1072,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) ||
+			is_cherry_picked_from_line(buf + i, k - i)))
 			return 0;
-		}
 	}
 	return 1;
 }
@@ -1071,7 +1091,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 2a040b7..73da182 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 <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

-- 
1.8.1.1.450.g0327af3
Previous: Jonathan NiederNext: Jonathan Nieder
Message 9 of 36 in “unify appending of sob”
  1. 00/11 unify appending of sobBrandon Casey, Jan 28, 2013
  2. 01/11 sequencer.c: rework search for start of footer to improve clarityBrandon Casey, Jan 28, 2013
  3. Jonathan NiederJan 28, 2013
  4. 02/11 commit, cherry-pick -s: remove broken support for multiline rfc2822 fieldsBrandon Casey, Jan 28, 2013
  5. Jonathan NiederJan 28, 2013
  6. 03/11 t/test-lib-functions.sh: allow to specify the tag name to test_commitBrandon Casey, Jan 28, 2013
  7. 04/11 t/t3511: add some tests of 'cherry-pick -s' functionalityBrandon Casey, Jan 28, 2013
  8. Jonathan NiederJan 28, 2013
  9. 05/11 sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footerBrandon Casey, Jan 28, 2013
  10. Jonathan NiederJan 28, 2013
  11. Jonathan NiederJan 28, 2013
  12. Brandon CaseyJan 28, 2013
  13. 06/11 sequencer.c: always separate "(cherry picked from" from commit bodyBrandon Casey, Jan 28, 2013
  14. Jonathan NiederJan 28, 2013
  15. Brandon CaseyFeb 10, 2013
  16. 07/11 sequencer.c: teach append_signoff how to detect duplicate s-o-bBrandon Casey, Jan 28, 2013
  17. Jonathan NiederJan 28, 2013
  18. Jonathan NiederJan 28, 2013
  19. Brandon CaseyFeb 10, 2013
  20. 08/11 sequencer.c: teach append_signoff to avoid adding a duplicate newlineBrandon Casey, Jan 28, 2013
  21. Jonathan NiederJan 28, 2013
  22. 09/11 t4014: more tests about appending s-o-b linesBrandon Casey, Jan 28, 2013
  23. Jonathan NiederJan 28, 2013
  24. Junio C HamanoJan 28, 2013
  25. Junio C HamanoJan 28, 2013
  26. 10/11 format-patch: update append_signoff prototypeBrandon Casey, Jan 28, 2013
  27. Jonathan NiederJan 28, 2013
  28. 11/11 Unify appending signoff in format-patch, commit and sequencerBrandon Casey, Jan 28, 2013
  29. Jonathan NiederJan 28, 2013
  30. Brandon CaseyFeb 10, 2013
  31. Jonathan NiederFeb 11, 2013
  32. Brandon CaseyFeb 11, 2013
  33. Jonathan NiederJan 28, 2013
  34. Junio C HamanoJan 28, 2013
  35. Junio C HamanoJan 30, 2013
  36. Brandon CaseyJan 31, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.