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

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

From
Brandon Casey <drafnel@gmail.com>
Date
Feb 12, 2013, 10:17 UTC
Message-ID
<1360664260-11803-6-git-send-email-drafnel@gmail.com>
In-Reply-To
<1360664260-11803-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>
[with improvements from Jonathan Nieder]
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
---
 sequencer.c              | 51 ++++++++++++++++++++++++++++++++------------
 t/t3511-cherry-pick-x.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 14 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index aa2cb8e..93495b0 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,16 +1022,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;
@@ -1045,15 +1074,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;
 }
@@ -1070,7 +1093,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.3.579.gd9af3b6
Previous: Brandon CaseyNext: Junio C Hamano
Message 7 of 41 in “unify appending of sob”
  1. 00/12 unify appending of sobBrandon Casey, Feb 12, 2013
  2. 01/12 sequencer.c: rework search for start of footer to improve clarityBrandon Casey, Feb 12, 2013
  3. 02/12 commit, cherry-pick -s: remove broken support for multiline rfc2822 fieldsBrandon Casey, Feb 12, 2013
  4. 03/12 t/test-lib-functions.sh: allow to specify the tag name to test_commitBrandon Casey, Feb 12, 2013
  5. Ævar Arnfjörð BjarmasonMay 13, 2017
  6. 04/12 t/t3511: add some tests of 'cherry-pick -s' functionalityBrandon Casey, Feb 12, 2013
  7. 05/12 sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footerBrandon Casey, Feb 12, 2013
  8. Junio C HamanoFeb 12, 2013
  9. Brandon CaseyFeb 12, 2013
  10. Junio C HamanoFeb 12, 2013
  11. Brandon CaseyFeb 12, 2013
  12. Junio C HamanoFeb 12, 2013
  13. Jonathan NiederFeb 12, 2013
  14. 06/12 sequencer.c: require a conforming footer to be preceded by a blank lineBrandon Casey, Feb 12, 2013
  15. 07/12 sequencer.c: always separate "(cherry picked from" from commit bodyBrandon Casey, Feb 12, 2013
  16. 08/12 sequencer.c: teach append_signoff how to detect duplicate s-o-bBrandon Casey, Feb 12, 2013
  17. 09/12 sequencer.c: teach append_signoff to avoid adding a duplicate newlineBrandon Casey, Feb 12, 2013
  18. 09/12 sequencer.c: teach append_signoff to avoid adding a duplicate newlineBrandon Casey, Feb 12, 2013
  19. John KeepingFeb 14, 2013
  20. Brandon CaseyFeb 15, 2013
  21. John KeepingFeb 17, 2013
  22. Junio C HamanoFeb 21, 2013
  23. Brandon CaseyFeb 21, 2013
  24. Brandon CaseyFeb 21, 2013
  25. Junio C HamanoFeb 21, 2013
  26. git-commit: populate the edit buffer with 2 blank lines before s-o-bBrandon Casey, Feb 22, 2013
  27. Junio C HamanoFeb 22, 2013
  28. Brandon CaseyFeb 22, 2013
  29. git-commit: populate the edit buffer with 2 blank lines before s-o-bBrandon Casey, Feb 22, 2013
  30. Jeff KingFeb 22, 2013
  31. Junio C HamanoFeb 22, 2013
  32. 10/12 t4014: more tests about appending s-o-b linesBrandon Casey, Feb 12, 2013
  33. 11/12 format-patch: update append_signoff prototypeBrandon Casey, Feb 12, 2013
  34. Junio C HamanoFeb 12, 2013
  35. Brandon CaseyFeb 12, 2013
  36. 12/12 Unify appending signoff in format-patch, commit and sequencerBrandon Casey, Feb 12, 2013
  37. 13/12 fixup! t/t3511: add some tests of 'cherry-pick -s' functionalityBrandon Casey, Feb 12, 2013
  38. Jonathan NiederFeb 12, 2013
  39. Junio C HamanoFeb 12, 2013
  40. Jonathan NiederFeb 12, 2013
  41. Junio C HamanoFeb 12, 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.