[PATCH] cherry-pick -x: add newline before pick note

[PATCH] cherry-pick -x: add newline before pick note

[Michael J Gruber]

Currently, cherry-pick -x sticks the pick note immediately after the
existing commit message. This

* is bad for commits with 1 line subject (it makes a 2 line subject)
* is different from git-svn, e.g., which leaves an empty line before.

Make cherry-pick always insert an empty line before the pick note.

Reported-by: Martin Svensson <martin.k.svensson@netinsight.se>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 builtin/revert.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 57b51e4..9251257 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -485,7 +485,7 @@ static int do_pick_commit(void)
 		set_author_ident_env(msg.message);
 		add_message_to_msg(&msgbuf, msg.message);
 		if (no_replay) {
-			strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+			strbuf_addstr(&msgbuf, "\n(cherry picked from commit ");
 			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
 			strbuf_addstr(&msgbuf, ")\n");
 		}
-- 
1.7.3.2.193.g78bbb

Re: [PATCH] cherry-pick -x: add newline before pick note

[Jeff King]

On Tue, Nov 16, 2010 at 04:11:17PM +0100, Michael J Gruber wrote:

> Currently, cherry-pick -x sticks the pick note immediately after the
> existing commit message. This
> 
> * is bad for commits with 1 line subject (it makes a 2 line subject)
> * is different from git-svn, e.g., which leaves an empty line before.
> 
> Make cherry-pick always insert an empty line before the pick note.

Hmm. Should this respect pseudo-header blocks at the end? E.g., if I
have:

  message subject

  Message body.

  Signed-off-by: Jeff King <peff@peff.net>

shouldn't it result in:

  message subject

  Message body.

  (cherry picked from commit ...)

  Signed-off-by: Jeff King <peff@peff.net>

?

Even better, I wonder if it should actually be:

  message subject

  Message body.

  Signed-off-by: Jeff King <peff@peff.net>
  Cherry-picked-from: ...

And then you could actually sign off the cherry-pick separately, too, if
you wanted, by adding a line _below_ the cherry-picked-from. I have no
idea if people are trying to grep for "cherry picked from commit...",
which my proposal would break.

Note that none of this is introduced by your patch. The current output
for this case is terribly ugly. But I thought I would mention it, as my
third version means we _do_ want the current behavior in some cases
(i.e., when there is already a pseudo-header block).

-Peff

[PATCH] commit -s: allow "(cherry picked " lines in sign-off section

[Jonathan Nieder]

Using cherry-pick -x -s to backport a public commit results in
an unsightly gap in the sign-off chain:

	Reported-by: Jarek Poplawski <jarkao2@gmail.com>
	Tested-by: Jarek Poplawski <jarkao2@gmail.com>
	Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
	Cc: Jeff Mahoney <jeffm@suse.com>
	Cc: All since 2.6.32 <stable@kernel.org>
	Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
	Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)

	Signed-off-by: Back Porter <backporter@example.com>

The cherry-pick is a step in the line of a patch like any other,
so one might prefer to lose the extra newline.

	...
	Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
	Signed-off-by: Back Porter <backporter@example.com>

This commit teaches "git commit --signoff", and thus cherry-pick -s,
to do exactly that.  It works by treating the "(cherry picked" line as
just another line in the signoff chain, except as the first line (that
last exception is to avoid false positives).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jeff King wrote:

> Even better, I wonder if it should actually be:
> 
>   message subject
> 
>   Message body.
> 
>   Signed-off-by: Jeff King <peff@peff.net>
>   Cherry-picked-from: ...

Here's something like that.  I use "git cherry-pick -x -s" to
backport patches from a public upstream.  Now you can, too.

Ideally inline notes like

	 [akpm@linux-foundation.org: coding-style fixes]

also ought to be tolerated.

 builtin/commit.c               |   20 +++++++
 t/t3510-cherry-pick-message.sh |  112 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 132 insertions(+), 0 deletions(-)
 create mode 100755 t/t3510-cherry-pick-message.sh

diff --git a/builtin/commit.c b/builtin/commit.c
index 66fdd22..71dd52b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -528,6 +528,8 @@ static int ends_rfc2822_footer(struct strbuf *sb)
 		i++;
 
 	for (; i < len; i = k) {
+		static const char cherry_pick[] = "(cherry picked from commit ";
+
 		for (k = i; k < len && buf[k] != '\n'; k++)
 			; /* do nothing */
 		k++;
@@ -535,6 +537,20 @@ static int ends_rfc2822_footer(struct strbuf *sb)
 		if ((buf[k] == ' ' || buf[k] == '\t') && !first)
 			continue;
 
+		if (!first && buf[k] == '(' && k + strlen(cherry_pick) < len) {
+			/* Might be a cherry-pick notice. */
+			const char *p = buf + k;
+			if (!memcmp(p, cherry_pick, strlen(cherry_pick))) {
+				p = memchr(buf + k, '\n', len - k);
+				if (!p)
+					return 0;
+				if (p + 1 == buf + len)
+					return 1;
+				k = p - buf;
+				continue;
+			}
+		}
+
 		first = 0;
 
 		for (j = 0; i + j < len; j++) {
@@ -625,6 +641,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
 			; /* do nothing */
 		if (prefixcmp(sb.buf + i, sob.buf)) {
+			/*
+			 * Only insert an extra newline if the previous line
+			 * is not part of a Signed-off-by:/Acked-by:/etc chain.
+			 */
 			if (!i || !ends_rfc2822_footer(&sb))
 				strbuf_addch(&sb, '\n');
 			strbuf_addbuf(&sb, &sob);
diff --git a/t/t3510-cherry-pick-message.sh b/t/t3510-cherry-pick-message.sh
new file mode 100755
index 0000000..83e50a3
--- /dev/null
+++ b/t/t3510-cherry-pick-message.sh
@@ -0,0 +1,112 @@
+#!/bin/sh
+
+test_description='tests for the log messages cherry-pick produces
+
+  ----
+  +      cherry-pick of branch
+     +   signoff
+    +    basic
+  ++     mainline
+  +++    initial
+'
+. ./test-lib.sh
+
+prepare_commit () {
+	git checkout initial &&
+	test_commit "$1" &&
+	test_tick &&
+	git commit --amend --allow-empty-message -F "$1.message" &&
+	git tag -d "$1" &&
+	git tag "$1"
+}
+
+test_cmp_message () {
+	expect=$1 &&
+	shift &&
+	git log -1 --pretty=format:%B "$@" >actual &&
+	test_cmp "$expect" actual
+}
+
+cat >basic.message <<\EOF
+ A branch
+
+Here comes the lovely description of a change to pick up.
+Contributions come from many people:
+EOF
+
+{
+	cat basic.message
+	cat <<-\EOF
+
+	Signed-off-by: Foo <foo@example.com>
+	Signed-off-by: Bar <bar@example.com>
+	Tested-by: Baz <baz@example.com>
+	EOF
+} >signoff.message
+
+test_expect_success 'setup' '
+	test_commit initial &&
+	prepare_commit basic &&
+	prepare_commit signoff
+'
+
+test_expect_success 'cherry-pick preserves message' '
+	cat basic.message >expect &&
+	git checkout initial &&
+	git cherry-pick basic &&
+	test_cmp_message expect
+'
+
+test_expect_success 'cherry-pick -s adds signoff' '
+	{
+		cat basic.message &&
+		echo &&
+		echo "Signed-off-by: C O Mitter <committer@example.com>"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -s basic &&
+	test_cmp_message expect HEAD
+'
+
+test_expect_success 'cherry-pick -s integrates into existing signoff chain' '
+	{
+		cat signoff.message &&
+		echo "Signed-off-by: C O Mitter <committer@example.com>"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -s signoff &&
+	test_cmp_message expect HEAD
+'
+
+test_expect_success 'cherry-pick -x adds old commit id' '
+	{
+		cat basic.message &&
+		echo "(cherry picked from commit $(git rev-parse basic^0))"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -x basic &&
+	test_cmp_message expect HEAD
+'
+
+test_expect_success 'cherry-pick -x integrates into signoff chain' '
+	{
+		cat signoff.message &&
+		echo "(cherry picked from commit $(git rev-parse signoff^0))"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -x signoff &&
+	test_cmp_message expect HEAD
+'
+
+test_expect_success 'cherry-pick -x -s' '
+	{
+		cat signoff.message &&
+		echo "(cherry picked from commit $(git rev-parse signoff^0))"
+		echo "Signed-off-by: C O Mitter <committer@example.com>"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -x -s signoff &&
+	test_cmp_message expect HEAD
+'
+
+test_done
-- 
1.7.2.3.551.g13682.dirty

Re: [PATCH] commit -s: allow "(cherry picked " lines in sign-off section

[Jonathan Nieder]

Jonathan Nieder wrote:

> 	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
> 
> 	Signed-off-by: Back Porter <backporter@example.com>
> 
> The cherry-pick is a step in the line of a patch like any other,
> so one might prefer to lose the extra newline.

Sigh.  s/line/life/

[...]
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

Let's kick off the reviews.

> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -528,6 +528,8 @@ static int ends_rfc2822_footer(struct strbuf *sb)
>  		i++;
>  
>  	for (; i < len; i = k) {
> +		static const char cherry_pick[] = "(cherry picked from commit ";
> +

Better to share this string with builtin/revert.c, no?

What would happen when "(cherry picked ..." gets translated?
Should only the current language's version be tolerated in
the commit footer, or is there something more generic to
match for that could take care of wording changes automatically?

> @@ -535,6 +537,20 @@ static int ends_rfc2822_footer(struct strbuf *sb)
>  		if ((buf[k] == ' ' || buf[k] == '\t') && !first)
>  			continue;
>  
> +		if (!first && buf[k] == '(' && k + strlen(cherry_pick) < len) {
> +			/* Might be a cherry-pick notice. */
> +			const char *p = buf + k;
> +			if (!memcmp(p, cherry_pick, strlen(cherry_pick))) {
> +				p = memchr(buf + k, '\n', len - k);

Maybe simpler:

	p = memchr(...
	if (!p)
		return 0;
	i = p - buf;

to reuse the termination condition in the sign-off parser.

Presumably the main loop could use memchr() instead of open-coding
it as well.

Re: [PATCH] commit -s: allow "(cherry picked " lines in sign-off section

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Jonathan Nieder wrote:
>
>> 	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
>> 
>> 	Signed-off-by: Back Porter <backporter@example.com>
>> 
>> The cherry-pick is a step in the line of a patch like any other,
>> so one might prefer to lose the extra newline.
>
> Sigh.  s/line/life/
>
> [...]
>> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>
> Let's kick off the reviews.
>
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -528,6 +528,8 @@ static int ends_rfc2822_footer(struct strbuf *sb)
>>  		i++;
>>  
>>  	for (; i < len; i = k) {
>> +		static const char cherry_pick[] = "(cherry picked from commit ";
>> +
>
> Better to share this string with builtin/revert.c, no?
>
> What would happen when "(cherry picked ..." gets translated?
> Should only the current language's version be tolerated in
> the commit footer, or is there something more generic to
> match for that could take care of wording changes automatically?

With this patch you are declaring that "(cherry picked from..." is a magic
marker just like "Signed-off-by: " never to be translated, no?

I am not sure I agree with the reasoning of this patch, by the way.  A
cherry-pick is an event that breaks the life of the patch, so it may even
be a sensible thing to do to express "the above sign-off chain shows who
were involved in the original commit; I am cherry-picking it out of
context, and these people do not have much to do with the result" with a
blank line on both sides of the "cherry picked" line, like this:

        A concise summary of the change

	A detailed description of the change, why it is needed, what
        was broken and why applying this is the best course of action.

	Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
	Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)

	Signed-off-by: Back Porter <backporter@example.com>

Re: [PATCH] commit -s: allow "(cherry picked " lines in sign-off section

[Jonathan Nieder]

Junio C Hamano wrote:

> I am not sure I agree with the reasoning of this patch, by the way.  A
> cherry-pick is an event that breaks the life of the patch, so it may even
> be a sensible thing to do to express "the above sign-off chain shows who
> were involved in the original commit; I am cherry-picking it out of
> context, and these people do not have much to do with the result" with a
> blank line on both sides of the "cherry picked" line, like this:
> 
>	A concise summary of the change
> 
> 	A detailed description of the change, why it is needed, what
>	was broken and why applying this is the best course of action.
> 
> 	Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> 	Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> 
> 	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
> 
> 	Signed-off-by: Back Porter <backporter@example.com>

How is the cherry-pick event different from the send-by-mail-and-apply
event?

In both cases, the result has a distinct commit id and distinct
signoff and it is unlikely that the previous patch handler was testing
with the same tree as the next one.  (And each patch handler should add
relevant comments if the new situation warrants that.)

Re: [PATCH] commit -s: allow "(cherry picked " lines in sign-off section

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> How is the cherry-pick event different from the send-by-mail-and-apply
> event?
>
> In both cases, the result has a distinct commit id and distinct
> signoff and it is unlikely that the previous patch handler was testing
> with the same tree as the next one.  (And each patch handler should add
> relevant comments if the new situation warrants that.)

Fair enough.  If that is the direction we would want to go, perhaps it
suggests that we might eventually want to use "Cherry-picked-from: " as
the marker for this information?

And if we go that route, and if this information is being used, we have a
rather serious backward compatibility problem.  Older scripts will break,
and this cannot be handwaved away with a configuration option or a command
line switch (even if you personally choose to keep using the old format,
you may get a commit with the new style trailer from elsewhere).

Hmm.

Re: [PATCH] commit -s: allow "(cherry picked " lines in sign-off section

[Jay Soffian]

On Tue, Nov 16, 2010 at 5:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I am not sure I agree with the reasoning of this patch, by the way.  A
> cherry-pick is an event that breaks the life of the patch, so it may even
> be a sensible thing to do to express "the above sign-off chain shows who
> were involved in the original commit; I am cherry-picking it out of
> context, and these people do not have much to do with the result" with a
> blank line on both sides of the "cherry picked" line, like this:
>
>        A concise summary of the change
>
>        A detailed description of the change, why it is needed, what
>        was broken and why applying this is the best course of action.
>
>        Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>        Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>
>        (cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
>
>        Signed-off-by: Back Porter <backporter@example.com>

Or perhaps prefix them with Original-, inspired by email headers, and
which I think makes it even more clear that the sob lines don't apply
to the new commit.

        Original-Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
        Original-Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
        Cherry-picked-from: 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0
        Signed-off-by: Back Porter <backporter@example.com>

j.

Re: [PATCH] cherry-pick -x: add newline before pick note

[Jay Soffian]

On Tue, Nov 16, 2010 at 2:30 PM, Jeff King <peff@peff.net> wrote:
> shouldn't it result in:
>
>  message subject
>
>  Message body.
>
>  (cherry picked from commit ...)
>
>  Signed-off-by: Jeff King <peff@peff.net>

+1.

> Even better, I wonder if it should actually be:
>
>  message subject
>
>  Message body.
>
>  Signed-off-by: Jeff King <peff@peff.net>
>  Cherry-picked-from: ...

+2.

> And then you could actually sign off the cherry-pick separately, too, if
> you wanted, by adding a line _below_ the cherry-picked-from. I have no
> idea if people are trying to grep for "cherry picked from commit...",
> which my proposal would break.

I can fix my regex easily enough, but I'd also be happy to have this
use some other switch than -x.

BTW, I notice that cherry-pick also misbehaves if the original commit
message doesn't end in a newline. I'm not sure whether that's a
cherry-pick bug for not checking that case, or whether it's a commit
bug for not ensuring a newline terminates the commit message.

j.

Re: [PATCH] cherry-pick -x: add newline before pick note

[Oswald Buddenhagen]

Michael J Gruber <git <at> drmicha.warpmail.net> writes:
> Currently, cherry-pick -x sticks the pick note immediately after the
> existing commit message. This
> 
> * is bad for commits with 1 line subject (it makes a 2 line subject)
> * is different from git-svn, e.g., which leaves an empty line before.
> 
> Make cherry-pick always insert an empty line before the pick note.
> 
> Reported-by: Martin Svensson <martin.k.svensson <at> netinsight.se>
> Signed-off-by: Michael J Gruber <git <at> drmicha.warpmail.net>
> ---
>  builtin/revert.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 57b51e4..9251257 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -485,7 +485,7 @@ static int do_pick_commit(void)
>  		set_author_ident_env(msg.message);
>  		add_message_to_msg(&msgbuf, msg.message);
>  		if (no_replay) {
> -			strbuf_addstr(&msgbuf, "(cherry picked from commit ");
> +			strbuf_addstr(&msgbuf, "\n(cherry picked from commit ");
>  			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
>  			strbuf_addstr(&msgbuf, ")\n");
>  		}

so while everybody is apparently thinking about totally over-engineering
things as much as possible, could we please have this patch applied so we
have a solution for the time being? i really hate to tell my coworkers that
they have to amend the cherry-picks just to make them comply with git's
own guidelines for well-formed commit messages (and thus have them pass
our pre-receive hook).

regards

Re: [PATCH] cherry-pick -x: add newline before pick note

[Jonathan Nieder]

(please do not cull the CC list)
Hi Oswald,

Oswald Buddenhagen wrote:

> so while everybody is apparently thinking about totally over-engineering
> things as much as possible, could we please have this patch applied so we
> have a solution for the time being?

I am not convinced that this patch makes a positive change in general.
Starting from a message

	Foo the bar

	Make some excellent improvement to the frobnicator.

	Signed-off-by: A U Thor <author@example.com>

a person passing on the patch might write

	Foo the bar
[...]
	Signed-off-by: A U Thor <author@example.com>
	[committer@example.com: avoid multiple return points]
	Signed-off-by: C O Mitter <committer@example.com>

Similarly, when cherry-picking from permanent history, it can make
sense to write

	Foo the bar
[...]
	Signed-off-by: A U Thor <author@example.com>
	(cherry picked from commit 78a8b989a76c8798a9898c98a98c98a98ca)
	Signed-off-by: C O Mitter <committer@example.com>

In both cases, it's just another hop in the life of a patch and not
something that seems to deserve emphasis with extra whitespace.

> i really hate to tell my coworkers that
> they have to amend the cherry-picks just to make them comply with git's
> own guidelines for well-formed commit messages (and thus have them pass
> our pre-receive hook).

I assume you are referring to one-line commit messages becoming two-line?
Here's something rough to start.

diff --git a/builtin/revert.c b/builtin/revert.c
index dc1b702..343e7e7 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -198,6 +198,28 @@ static void add_message_to_msg(struct strbuf *msgbuf, const char *message)
 	strbuf_addstr(msgbuf, p);
 }
 
+static void add_blank_line_if_oneline(struct strbuf *msgbuf)
+{
+	const char *newline = memchr(msgbuf->buf, '\n', msgbuf->len);
+
+	if (!newline) {	/* No newline at end of message. */
+		strbuf_addch(msgbuf, '\n');
+		newline = msgbuf->buf + msgbuf->len - 1;
+	}
+
+	/*
+	 * If the change description consists of a single line,
+	 * add a blank line separating the title from the new
+	 * message body (the "(cherry picked from" line).
+	 *
+	 * NEEDSWORK: it would be better to reuse the append_signoff
+	 * logic.
+	 */
+	newline = memchr(newline, '\n', msgbuf-> buf + msgbuf->len - newline);
+	if (!newline)
+		strbuf_addch(msgbuf, '\n');
+}
+
 static void set_author_ident_env(const char *message)
 {
 	const char *p = message;
@@ -499,6 +521,7 @@ static int do_pick_commit(void)
 		next_label = msg.label;
 		set_author_ident_env(msg.message);
 		add_message_to_msg(&msgbuf, msg.message);
+		add_blank_line_if_oneline(&msgbuf);
 		if (no_replay) {
 			strbuf_addstr(&msgbuf, "(cherry picked from commit ");
 			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
-- 

Re: [PATCH] cherry-pick -x: add newline before pick note

[Oswald Buddenhagen]

On Tue, Mar 08, 2011 at 04:08:43PM -0600, Jonathan Nieder wrote:
> (please do not cull the CC list)
>
i posted via the gmane webform ...

> Oswald Buddenhagen wrote:
> > i really hate to tell my coworkers that they have to amend the
> > cherry-picks just to make them comply with git's own guidelines for
> > well-formed commit messages
> 
> I assume you are referring to one-line commit messages becoming
> two-line?
>
yes

> Here's something rough to start.
> 
i did a much simpler patch in that vein as well, but scrapped it again -
totally overengineered. the idea is to optimize the simple case -
cherry-pick -x, push. everything else needs amends anyway.

regards

Re: [PATCH] cherry-pick -x: add newline before pick note

[Jonathan Nieder]

Hi again.

Oswald Buddenhagen wrote:

> i posted via the gmane webform ...

(See <http://thread.gmane.org/gmane.comp.version-control.git/154490>.
It's obnoxious that there is not an easier way, I agree.)

> i did a much simpler patch in that vein as well, but scrapped it again -
> totally overengineered. the idea is to optimize the simple case -
> cherry-pick -x, push. everything else needs amends anyway.

It's been a vague wish of mine for a while to fix "cherry-pick -x -s".
I currently use it without amends and tolerate the blank line between
the "cherry picked" and the sign-off.

Maybe you can convince people that wanting no extra blank line between
an existing sign-off and the "cherry picked from" line is
overengineering and worth regressing.  I am not a fanatic about it ---
I just thought it was worth mentioning that this proposed change would
not be seen as positive by everyone.

Put another way, I don't find the two words "totally overengineered"
very convincing here.  For what it's worth.

Cheers,
Jonathan