[PATCH] Fix merge name generation in "merge in C"

[PATCH] Fix merge name generation in "merge in C"

[Junio C Hamano]

When merging an early part of a branch, e.g. "git merge xyzzy~20", we were
supposed to say "branch 'xyzzy' (early part)", but it incorrectly said
"branch 'refs/heads/xy' (early part)" instead.

The logic was supposed to first strip away "~20" part to make sure that
what follows "~" is a non-zero posint, prefix it with "refs/heads/" and
ask resolve_ref() if it is a ref.  If it is, then we know xyzzy was a
branch, and we can give the correct message.

However, there were a few bugs.  First of all, the logic to build this
"true branch refname" did not count the characters correctly.  At this
point of the code, "len" is the number of trailing, non-name part of the
given extended SHA-1 expression given by the user, i.e. number of bytes in
"~20" in the above example.

In addition, the message forgot to skip "refs/heads/" it prefixed from the
output.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * It is a bit surprising that after beating merge-in-C to death, we
   still find a minor breakage like this.

 builtin-merge.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index e78fa18..dde0c7e 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -396,12 +396,12 @@ static void merge_name(const char *remote, struct strbuf *msg)
 		struct strbuf truname = STRBUF_INIT;
 		strbuf_addstr(&truname, "refs/heads/");
 		strbuf_addstr(&truname, remote);
-		strbuf_setlen(&truname, len+11);
+		strbuf_setlen(&truname, truname.len - len);
 		if (resolve_ref(truname.buf, buf_sha, 0, 0)) {
 			strbuf_addf(msg,
 				    "%s\t\tbranch '%s'%s of .\n",
 				    sha1_to_hex(remote_head->sha1),
-				    truname.buf,
+				    truname.buf + 11,
 				    (early ? " (early part)" : ""));
 			return;
 		}

[PATCH] Add testcase to ensure merging an early part of a branch is done properly

[Miklos Vajna]

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Wed, Jul 30, 2008 at 01:12:19AM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> In addition, the message forgot to skip "refs/heads/" it prefixed from
> the
> output.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  * It is a bit surprising that after beating merge-in-C to death, we
>    still find a minor breakage like this.

Uh-oh. Here is a testcase that fails with master, but passes with your
patch.

Thanks for catching this.

 t/t7607-merge-early.sh |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)
 create mode 100755 t/t7607-merge-early.sh

diff --git a/t/t7607-merge-early.sh b/t/t7607-merge-early.sh
new file mode 100755
index 0000000..9dd3ac5
--- /dev/null
+++ b/t/t7607-merge-early.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+
+test_description='git-merge
+
+Testing merging an early part of a branch.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo c0 > c0.c &&
+	git add c0.c &&
+	git commit -m c0 &&
+	git tag c0 &&
+	echo c1 > c1.c &&
+	git add c1.c &&
+	git commit -m c1 &&
+	git tag c1 &&
+	echo c2 > c2.c &&
+	git add c2.c &&
+	git commit -m c2 &&
+	git tag c2 &&
+	git reset --hard c0 &&
+	echo c3 > c3.c &&
+	git add c3.c &&
+	git commit -m c3 &&
+	git tag c3
+'
+
+cat >expected <<EOF
+Merge branch 'c2' (early part)
+EOF
+
+test_expect_success 'merge early part of c2' '
+	git merge c2~1 &&
+	git show -s --pretty=format:%s HEAD > actual &&
+	test_cmp actual expected
+'
+
+test_done
-- 
1.6.0.rc0.14.g95f8.dirty

[PATCH] Add testcase to ensure merging an early part of a branch is done properly

[Miklos Vajna]

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

[ Adding Shawn to CC as the interim maintainer. ]

On Wed, Jul 30, 2008 at 07:23:28PM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
> Uh-oh. Here is a testcase that fails with master, but passes with your
> patch.

I remember you had a note about it's a bad habit to write a new test for
every bug, so here is an updated version that just improves
t7600-merge.sh and does not adds a new one.

 t/t7600-merge.sh |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 7313ac2..a4443a7 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -520,4 +520,30 @@ test_expect_success 'refresh the index before merging' '
 
 test_debug 'gitk --all'
 
+cat >expected <<EOF
+Merge branch 'c5' (early part)
+EOF
+
+test_expect_success 'merge early part of c2' '
+	git reset --hard c3 &&
+	echo c4 > c4.c &&
+	git add c4.c &&
+	git commit -m c4 &&
+	git tag c4 &&
+	echo c5 > c5.c &&
+	git add c5.c &&
+	git commit -m c5 &&
+	git tag c5 &&
+	git reset --hard c3 &&
+	echo c6 > c6.c &&
+	git add c6.c &&
+	git commit -m c6 &&
+	git tag c6 &&
+	git merge c5~1 &&
+	git show -s --pretty=format:%s HEAD > actual &&
+	test_cmp actual expected
+'
+
+test_debug 'gitk --all'
+
 test_done
-- 
1.6.0.2

Re: [PATCH] Add testcase to ensure merging an early part of a branch is done properly

[Shawn O. Pearce]

Miklos Vajna <vmiklos@frugalware.org> wrote:
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
> 
> [ Adding Shawn to CC as the interim maintainer. ]

In case you missed it, Junio has the pumpkin now.  (He's picked up
my topics, merged my tree, and has freed me from the time sink that
is known as maintainership.)
 
-- 
Shawn.