[BUG?] git-merge appends extra string to user's message?

[BUG?] git-merge appends extra string to user's message?

[Nanako Shiraishi]

When "git merge" is run with a message, it appends an extra message 
that it would add if it were run without one. For example, when I 
tried to merge an early part of a side branch.

% git merge -m "Merge early part of side branch" `git rev-parse side~2`
% git show -s 
commit 37217141e7519629353738d5e4e677a15096206f
Merge: e68e646 a1d2374
Author: しらいし ななこ <nanako3@lavabit.com>
Date:   Wed Dec 2 14:33:20 2009 +0900

    Merge early part of side branch

    Merge commit 'a1d2374f8f52f4e8a53171601a920b538a6cec23'

I gave my own message because I didn't want git to add the 
standard message (if I wanted to, I wouldn't have given one, 
or I would have prepared it using git-fmt-merge-msg command).

Is it possible to tell git-merge not to do this, or will it 
break compatibility and is a bad idea to change it?

I noticed that this was proposed before:

 http://thread.gmane.org/gmane.comp.version-control.git/77965/focus=80059

you were in favor of changing this behavior, and there was 
no objection from the list.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: [BUG?] git-merge appends extra string to user's message?

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

On Wed, Dec 02, 2009 at 07:20:30PM +0900, Nanako Shiraishi <nanako3@lavabit.com> wrote:
> % git merge -m "Merge early part of side branch" `git rev-parse side~2`
> % git show -s 
> commit 37217141e7519629353738d5e4e677a15096206f
> Merge: e68e646 a1d2374
> Author: ???????????? ????????? <nanako3@lavabit.com>
> Date:   Wed Dec 2 14:33:20 2009 +0900
> 
>     Merge early part of side branch
> 
>     Merge commit 'a1d2374f8f52f4e8a53171601a920b538a6cec23'

At least it's intentional. When I wrote builtin-merge I remember I
checked that the script version had this behaviour and that's why I
implemented the same way in C.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [BUG?] git-merge appends extra string to user's message?

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

> When "git merge" is run with a message, it appends an extra message 
> that it would add if it were run without one....
> 
> I gave my own message because I didn't want git to add the 
> standard message (if I wanted to, I wouldn't have given one, 
> or I would have prepared it using git-fmt-merge-msg command).
>
> Is it possible to tell git-merge not to do this, or will it 
> break compatibility and is a bad idea to change it?
>
> I noticed that this was proposed before:
>
>  http://thread.gmane.org/gmane.comp.version-control.git/77965/focus=80059
>
> you were in favor of changing this behavior, and there was no objection
> from the list.

I am still in favor, and I think we should do this change.  I know Miklos
said it is intentional, but I think he merely means "The version rewritten
in C does so intentionally because the version before rewrite did so", not
necessarily because "the version before rewrite did so intentionally with
good reasons."

Especially because we are going to update "merge" and eventually drop the
"merge <msg> HEAD <commit>..."  syntax, the first step should be to make
it equivalent to "merge -m <msg> <commit..."

How does this look?

I stole a major part of your message to the proposed commit log.

-- >8 --
Subject: [PATCH] merge: do not add standard message when message is given with -m option

Even if the user explicitly gave her own message to "git merge", the
command still added its standard merge message.  It resulted in a
useless repetition like this:

    % git merge -m "Merge early part of side branch" `git rev-parse side~2`
    % git show -s
    commit 37217141e7519629353738d5e4e677a15096206f
    Merge: e68e646 a1d2374
    Author: しらいし ななこ <nanako3@lavabit.com>
    Date:   Wed Dec 2 14:33:20 2009 +0900

	Merge early part of side branch

	Merge commit 'a1d2374f8f52f4e8a53171601a920b538a6cec23'

The gave her own message because she didn't want git to add the
standard message (if she wanted to, she wouldn't have given one,
or she would have prepared it using git-fmt-merge-msg command).

Noticed by Nanako Shiraishi

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-merge.c                 |   14 ++++++++------
 t/t7604-merge-custom-message.sh |    7 ++-----
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index 2104577..981fe4b 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -71,7 +71,7 @@ static int option_parse_message(const struct option *opt,
 	if (unset)
 		strbuf_setlen(buf, 0);
 	else if (arg) {
-		strbuf_addf(buf, "%s\n\n", arg);
+		strbuf_addf(buf, "%s%s", buf->len ? "\n\n" : "", arg);
 		have_message = 1;
 	} else
 		return error("switch `m' requires a value");
@@ -943,11 +943,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * codepath so we discard the error in this
 		 * loop.
 		 */
-		for (i = 0; i < argc; i++)
-			merge_name(argv[i], &msg);
-		fmt_merge_msg(option_log, &msg, &merge_msg);
-		if (merge_msg.len)
-			strbuf_setlen(&merge_msg, merge_msg.len-1);
+		if (!have_message) {
+			for (i = 0; i < argc; i++)
+				merge_name(argv[i], &msg);
+			fmt_merge_msg(option_log, &msg, &merge_msg);
+			if (merge_msg.len)
+				strbuf_setlen(&merge_msg, merge_msg.len-1);
+		}
 	}
 
 	if (head_invalid || !argc)
diff --git a/t/t7604-merge-custom-message.sh b/t/t7604-merge-custom-message.sh
index de977c5..269cfdf 100755
--- a/t/t7604-merge-custom-message.sh
+++ b/t/t7604-merge-custom-message.sh
@@ -22,15 +22,12 @@ test_expect_success 'setup' '
 	git tag c2
 '
 
-cat >expected <<\EOF
-custom message
 
-Merge commit 'c2'
-EOF
 test_expect_success 'merge c2 with a custom message' '
 	git reset --hard c1 &&
+	echo >expected "custom message" &&
 	git merge -m "custom message" c2 &&
-	git cat-file commit HEAD | sed -e "1,/^$/d" > actual &&
+	git cat-file commit HEAD | sed -e "1,/^$/d" >actual &&
 	test_cmp expected actual
 '
 
-- 
1.6.6.rc0

Re: [BUG?] git-merge appends extra string to user's message?

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 453 bytes --]

On Wed, Dec 02, 2009 at 09:09:49AM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> I am still in favor, and I think we should do this change.  I know Miklos
> said it is intentional, but I think he merely means "The version rewritten
> in C does so intentionally because the version before rewrite did so", not
> necessarily because "the version before rewrite did so intentionally with
> good reasons."

Exactly.

> How does this look?

Looks fine.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]