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

[PATCH v3 06/11] sequencer.c: always separate "(cherry picked from" from commit body

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

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.

This commit is mostly a code movement, but notice that has_conforming_footer() was modified, in addition to being moved.

Introduce tests to test this functionality.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 sequencer.c              | 120 +++++++++++++++++++++++++----------------------
 t/t3511-cherry-pick-x.sh |  53 +++++++++++++++++++++
 2 files changed, 116 insertions(+), 57 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 0b5cd18..46d51b2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -20,6 +20,67 @@
 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 == ':')
+			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 last_char_was_nl, this_char_is_nl;
+	int i, k;
+	int len = sb->len - ignore_footer;
+	const char *buf = sb->buf;
+
+	/* find start of last paragraph */
+	last_char_was_nl = 0;
+	for (i = len - 1; i > 0; i--) {
+		this_char_is_nl = (buf[i] == '\n');
+		if (last_char_was_nl && this_char_is_nl)
+			break;
+		last_char_was_nl = this_char_is_nl;
+	}
+
+	/* require at least one blank line */
+	if (!last_char_was_nl || buf[i] != '\n')
+		return 0;
+
+	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) ||
+			is_cherry_picked_from_line(buf + i, k - i)))
+			return 0;
+	}
+	return 1;
+}
+
 static void remove_sequencer_state(void)
 {
 	struct strbuf seq_dir = STRBUF_INIT;
@@ -497,6 +558,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");
@@ -1022,63 +1085,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 == ':')
-			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 last_char_was_nl, this_char_is_nl;
-	int i, k;
-	int len = sb->len - ignore_footer;
-	const char *buf = sb->buf;
-
-	/* find start of last paragraph */
-	last_char_was_nl = 0;
-	for (i = len - 1; i > 0; i--) {
-		this_char_is_nl = (buf[i] == '\n');
-		if (last_char_was_nl && this_char_is_nl)
-			break;
-		last_char_was_nl = this_char_is_nl;
-	}
-
-	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) ||
-			is_cherry_picked_from_line(buf + i, k - i)))
-			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 73da182..a845e45 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

-- 
1.8.1.1.450.g0327af3
Previous: Brandon CaseyNext: Jonathan Nieder
Message 13 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.