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

[PATCH v3 07/11] sequencer.c: teach append_signoff how to detect duplicate s-o-b

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

Teach append_signoff how to detect a duplicate s-o-b in the commit footer. This is in preparation to unify the append_signoff implementations in log-tree.c and sequencer.c.

Fixes test in t3511.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 builtin/commit.c         |  2 +-
 sequencer.c              | 47 +++++++++++++++++++++++++++++++++++++----------
 sequencer.h              |  4 +++-
 t/t3511-cherry-pick-x.sh |  2 +-
 4 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 38b9a9c..67ea9e7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -700,7 +700,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			previous = eol;
 		}
 
-		append_signoff(&sb, ignore_footer);
+		append_signoff(&sb, ignore_footer, 0);
 	}
 
 	if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
diff --git a/sequencer.c b/sequencer.c
index 46d51b2..015cb58 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -46,12 +46,20 @@ static int is_cherry_picked_from_line(const char *buf, int len)
 		 (buf[len - 1] == '\n' && buf[len - 2] == ')'));
 }
 
-static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
+/*
+ * Returns 0 for non-conforming footer
+ * Returns 1 for conforming footer
+ * Returns 2 when sob exists within conforming footer
+ * Returns 3 when sob exists within conforming footer as last entry
+ */
+static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
+	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;
+	int found_sob = 0;
 
 	/* find start of last paragraph */
 	last_char_was_nl = 0;
@@ -70,14 +78,25 @@ static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
 		i++;
 
 	for (; i < len; i = k) {
+		int found_rfc2822;
+
 		for (k = i; k < len && buf[k] != '\n'; k++)
 			; /* do nothing */
 		k++;
 
-		if (!(is_rfc2822_line(buf + i, k - i) ||
+		found_rfc2822 = is_rfc2822_line(buf + i, k - i);
+		if (found_rfc2822 && sob &&
+			!strncmp(buf + i, sob->buf, sob->len))
+			found_sob = k;
+
+		if (!(found_rfc2822 ||
 			is_cherry_picked_from_line(buf + i, k - i)))
 			return 0;
 	}
+	if (found_sob == i)
+		return 3;
+	if (found_sob)
+		return 2;
 	return 1;
 }
 
@@ -299,7 +318,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
 	rollback_lock_file(&index_lock);
 
 	if (opts->signoff)
-		append_signoff(msgbuf, 0);
+		append_signoff(msgbuf, 0, 0);
 
 	if (!clean) {
 		int i;
@@ -558,7 +577,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 		}
 
 		if (opts->record_origin) {
-			if (!has_conforming_footer(&msgbuf, 0))
+			if (!has_conforming_footer(&msgbuf, NULL, 0))
 				strbuf_addch(&msgbuf, '\n');
 			strbuf_addstr(&msgbuf, cherry_picked_prefix);
 			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
@@ -1085,9 +1104,11 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 	return pick_commits(todo_list, opts);
 }
 
-void append_signoff(struct strbuf *msgbuf, int ignore_footer)
+void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
 {
+	unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
 	struct strbuf sob = STRBUF_INIT;
+	int has_footer = 0;
 	int i;
 
 	strbuf_addstr(&sob, sign_off_header);
@@ -1096,10 +1117,16 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer)
 	strbuf_addch(&sob, '\n');
 	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 || !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);
-	}
+
+	if (i)
+		has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
+
+	if (!has_footer)
+		strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
+
+	if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
+		strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
+				sob.buf, sob.len);
+
 	strbuf_release(&sob);
 }
diff --git a/sequencer.h b/sequencer.h
index 9d57d57..1fc22dc 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -6,6 +6,8 @@
 #define SEQ_TODO_FILE	"sequencer/todo"
 #define SEQ_OPTS_FILE	"sequencer/opts"
 
+#define APPEND_SIGNOFF_DEDUP (1u << 0)
+
 enum replay_action {
 	REPLAY_REVERT,
 	REPLAY_PICK
@@ -48,6 +50,6 @@ int sequencer_pick_revisions(struct replay_opts *opts);
 
 extern const char sign_off_header[];
 
-void append_signoff(struct strbuf *msgbuf, int ignore_footer);
+void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);
 
 #endif
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index a845e45..f977279 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -82,7 +82,7 @@ test_expect_success 'cherry-pick -s inserts blank line after one line subject' '
 	test_cmp expect actual
 '
 
-test_expect_failure 'cherry-pick -s inserts blank line after non-conforming footer' '
+test_expect_success 'cherry-pick -s inserts blank line after non-conforming footer' '
 	pristine_detach initial &&
 	git cherry-pick -s mesg-broken-footer &&
 	cat <<-EOF >expect &&

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