Subject: [PATCH/RFC v2] sequencer.c: record revert/cherry-pick commit with trailer lines

From: Nguyễn Thái Ngọc Duy <[email-masked]>
Date: Sun, 4 Nov 2018 19:10:26 +0100

When a commit is reverted (or cherry-picked with -x) we add an English
sentence recording that commit id in the new commit message. Make
these real trailer lines instead so that they are more friendly to
parsers (especially "git interpret-trailers").

A reverted commit will have a new trailer

    Revert: <commit-id>

Similarly a cherry-picked commit with -x will have

    Cherry-picked-from: <commit-id>

When reverting or cherry picking a merge, the reverted/cherry-picked
branch will be shown using extended SHA-1 syntax, e.g.

    Revert: <commit-id>~2

Since we're not producing the old lines "This reverts commit ..." and
"(cherry picked from commit .." anymore, scripts that look for these
lines will need to be updated to handle both. Fresh new history could
just rely on git-interpret-trailers instead.

Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
---
 v2 adds the third syntax "~2" and renames Cherry-Pick: to
 Cherry-picked-from: and acknowledge the problem with breaking
 scripts. I don't have a pretty solution for that though.

 Documentation/git-cherry-pick.txt |  5 ++---
 sequencer.c                       | 33 +++++++++++++++++--------------
 t/t3510-cherry-pick-sequence.sh   | 12 +++++------
 t/t3511-cherry-pick-x.sh          | 26 ++++++++++++------------
 4 files changed, 39 insertions(+), 37 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index d35d771fc8..54e73e74c0 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -58,9 +58,8 @@ OPTIONS
 	message prior to committing.
 
 -x::
-	When recording the commit, append a line that says
-	"(cherry picked from commit ...)" to the original commit
-	message in order to indicate which commit this change was
+	When recording the commit, append "Cherry-picked-from:" trailer line
+	recording the commit name which commit this change was
 	cherry-picked from.  This is done only for cherry
 	picks without conflicts.  Do not use this option if
 	you are cherry-picking from your private branch because
diff --git a/sequencer.c b/sequencer.c
index 9e1ab3a2a7..f804406b68 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -36,7 +36,6 @@
 #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 ";
 
 GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG")
 
@@ -1669,7 +1668,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 	char *author = NULL;
 	struct commit_message msg = { NULL, NULL, NULL, NULL };
 	struct strbuf msgbuf = STRBUF_INIT;
-	int res, unborn = 0, allow;
+	int res, unborn = 0, allow, parent_id = -1;
 
 	if (opts->no_commit) {
 		/*
@@ -1716,6 +1715,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 			return error(_("commit %s does not have parent %d"),
 				oid_to_hex(&commit->object.oid), opts->mainline);
 		parent = p->item;
+		parent_id = cnt;
 	} else if (0 < opts->mainline)
 		return error(_("mainline was specified but commit %s is not a merge."),
 			oid_to_hex(&commit->object.oid));
@@ -1758,16 +1758,15 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 		base_label = msg.label;
 		next = parent;
 		next_label = msg.parent_label;
-		strbuf_addstr(&msgbuf, "Revert \"");
-		strbuf_addstr(&msgbuf, msg.subject);
-		strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
-		strbuf_addstr(&msgbuf, oid_to_hex(&commit->object.oid));
-
-		if (commit->parents && commit->parents->next) {
-			strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
-			strbuf_addstr(&msgbuf, oid_to_hex(&parent->object.oid));
-		}
-		strbuf_addstr(&msgbuf, ".\n");
+		strbuf_addf(&msgbuf, "Revert \"%s\"\n\n", msg.subject);
+
+		if (parent_id != -1)
+			strbuf_addf(&msgbuf, "Revert: %s~%d\n",
+				    oid_to_hex(&commit->object.oid),
+				    parent_id);
+		else
+			strbuf_addf(&msgbuf, "Revert: %s\n",
+				    oid_to_hex(&commit->object.oid));
 	} else {
 		const char *p;
 
@@ -1784,9 +1783,13 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 			strbuf_complete_line(&msgbuf);
 			if (!has_conforming_footer(&msgbuf, NULL, 0))
 				strbuf_addch(&msgbuf, '\n');
-			strbuf_addstr(&msgbuf, cherry_picked_prefix);
-			strbuf_addstr(&msgbuf, oid_to_hex(&commit->object.oid));
-			strbuf_addstr(&msgbuf, ")\n");
+			if (parent_id != -1)
+				strbuf_addf(&msgbuf, "Cherry-picked-from: %s~%d\n",
+					    oid_to_hex(&commit->object.oid),
+					    parent_id);
+			else
+				strbuf_addf(&msgbuf, "Cherry-picked-from: %s\n",
+					    oid_to_hex(&commit->object.oid));
 		}
 		if (!is_fixup(command))
 			author = get_author(msg.message);
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index c84eeefdc9..504423ec20 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -390,10 +390,10 @@ test_expect_success '--continue respects opts' '
 	git cat-file commit HEAD~1 >picked_msg &&
 	git cat-file commit HEAD~2 >unrelatedpick_msg &&
 	git cat-file commit HEAD~3 >initial_msg &&
-	! grep "cherry picked from" initial_msg &&
-	grep "cherry picked from" unrelatedpick_msg &&
-	grep "cherry picked from" picked_msg &&
-	grep "cherry picked from" anotherpick_msg
+	! grep "Cherry-picked-from:" initial_msg &&
+	grep "Cherry-picked-from: " unrelatedpick_msg &&
+	grep "Cherry-picked-from: " picked_msg &&
+	grep "Cherry-picked-from: " anotherpick_msg
 '
 
 test_expect_success '--continue of single-pick respects -x' '
@@ -404,7 +404,7 @@ test_expect_success '--continue of single-pick respects -x' '
 	git cherry-pick --continue &&
 	test_path_is_missing .git/sequencer &&
 	git cat-file commit HEAD >msg &&
-	grep "cherry picked from" msg
+	grep "Cherry-picked-from:" msg
 '
 
 test_expect_success '--continue respects -x in first commit in multi-pick' '
@@ -416,7 +416,7 @@ test_expect_success '--continue respects -x in first commit in multi-pick' '
 	test_path_is_missing .git/sequencer &&
 	git cat-file commit HEAD^ >msg &&
 	picked=$(git rev-parse --verify picked) &&
-	grep "cherry picked from.*$picked" msg
+	grep "Cherry-picked-from: $picked" msg
 '
 
 test_expect_failure '--signoff is automatically propagated to resolved conflict' '
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 9888bf34b9..798fdaf8da 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -32,7 +32,7 @@ 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)
+Cherry-picked-from: da39a3ee5e6b4b0d3255bfef95601890afd80709
 Tested-by: C.U. Thor <[email-masked]>"
 
 mesg_unclean="$mesg_one_line
@@ -81,7 +81,7 @@ test_expect_success 'cherry-pick -x inserts blank line after one line subject' '
 	cat <<-EOF >expect &&
 		$mesg_one_line
 
-		(cherry picked from commit $sha1)
+		Cherry-picked-from: $sha1
 	EOF
 	git log -1 --pretty=format:%B >actual &&
 	test_cmp expect actual
@@ -129,7 +129,7 @@ test_expect_success 'cherry-pick -x inserts blank line when conforming footer no
 	cat <<-EOF >expect &&
 		$mesg_no_footer
 
-		(cherry picked from commit $sha1)
+		Cherry-picked-from: $sha1
 	EOF
 	git log -1 --pretty=format:%B >actual &&
 	test_cmp expect actual
@@ -154,7 +154,7 @@ test_expect_success 'cherry-pick -x -s inserts blank line when conforming footer
 	cat <<-EOF >expect &&
 		$mesg_no_footer
 
-		(cherry picked from commit $sha1)
+		Cherry-picked-from: $sha1
 		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
 	EOF
 	git log -1 --pretty=format:%B >actual &&
@@ -178,7 +178,7 @@ test_expect_success 'cherry-pick -x -s adds sob when last sob doesnt match commi
 	git cherry-pick -x -s mesg-with-footer &&
 	cat <<-EOF >expect &&
 		$mesg_with_footer
-		(cherry picked from commit $sha1)
+		Cherry-picked-from: $sha1
 		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
 	EOF
 	git log -1 --pretty=format:%B >actual &&
@@ -201,7 +201,7 @@ test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists fo
 	git cherry-pick -x -s mesg-with-footer-sob &&
 	cat <<-EOF >expect &&
 		$mesg_with_footer_sob
-		(cherry picked from commit $sha1)
+		Cherry-picked-from: $sha1
 		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
 	EOF
 	git log -1 --pretty=format:%B >actual &&
@@ -215,7 +215,7 @@ test_expect_success 'cherry-pick -x handles commits with no NL at end of message
 	git cherry-pick -x $sha1 &&
 	git log -1 --pretty=format:%B >actual &&
 
-	printf "\n(cherry picked from commit %s)\n" $sha1 >>msg &&
+	printf "\nCherry-picked-from: %s\n" $sha1 >>msg &&
 	test_cmp msg actual
 '
 
@@ -226,7 +226,7 @@ test_expect_success 'cherry-pick -x handles commits with no footer and no NL at
 	git cherry-pick -x $sha1 &&
 	git log -1 --pretty=format:%B >actual &&
 
-	printf "\n\n(cherry picked from commit %s)\n" $sha1 >>msg &&
+	printf "\n\nCherry-picked-from: %s\n" $sha1 >>msg &&
 	test_cmp msg actual
 '
 
@@ -252,19 +252,19 @@ test_expect_success 'cherry-pick -s handles commits with no footer and no NL at
 	test_cmp msg actual
 '
 
-test_expect_success 'cherry-pick -x treats "(cherry picked from..." line as part of footer' '
+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)
+		Cherry-picked-from: $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' '
+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 &&
@@ -275,13 +275,13 @@ 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' '
+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)
+		Cherry-picked-from: $sha1
 		Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
 	EOF
 	git log -1 --pretty=format:%B >actual &&
-- 
2.19.1.1005.gac84295441