[PATCH 0/3] commit: fix abbrev-sha regression

After c197702 (pretty: Respect --abbrev option), the summary output for
git-commit began to throw up non-abbreviated SHA-1s.

For example,

  $ git commit --allow-empty -m "empty"
  [master f869bf09c8eefee54094bb21387241aaf5f10324] empty

I also notice that this happens for merges after conflict resolutions.

This afflicts 'master', but not any release.

Here's the hunk that fixed it:

-->8--

diff --git a/builtin/commit.c b/builtin/commit.c
index a4e4966..ab4a7cf 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -1166,6 +1166,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 		struct pretty_print_context ctx = {0};
 		struct strbuf buf = STRBUF_INIT;
 		ctx.date_mode = DATE_NORMAL;
+		ctx.abbrev = DEFAULT_ABBREV;
 		format_commit_message(commit, format.buf + 7, &buf, &ctx);
 		printf("%s\n", buf.buf);
 		strbuf_release(&buf);
-->8--

However, I noticed that this would also work:

-->8--

diff --git a/builtin/commit.c b/builtin/commit.c
index ab4a7cf..13a30ab 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -1148,7 +1148,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
        rev.verbose_header = 1;
        rev.show_root_diff = 1;
        get_commit_format(format.buf, &rev);
-       rev.always_show_header = 0;
+       rev.always_show_header = 1;
        rev.diffopt.detect_rename = 1;
        rev.diffopt.rename_limit = 100;
        rev.diffopt.break_opt = 0;
-->8--

This would make the code block that handled the situation in which
log_tree_commit() did not print any output redundant, so perhaps this
would also follow:

-->8--

diff --git a/builtin/commit.c b/builtin/commit.c
index ab4a7cf..e8def55 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -1163,13 +1163,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
                initial_commit ? " (root-commit)" : "");

        if (!log_tree_commit(&rev, commit)) {
-               struct pretty_print_context ctx = {0};
-               struct strbuf buf = STRBUF_INIT;
-               ctx.date_mode = DATE_NORMAL;
-               ctx.abbrev = DEFAULT_ABBREV;
-               format_commit_message(commit, format.buf + 7, &buf, &ctx);
-               printf("%s\n", buf.buf);
-               strbuf_release(&buf);
+               die("unable to print summary");
        }
        strbuf_release(&format);
 }
-->8--

In a sense, it reverts parts of bf82a15 (commit: do not add extra LF at
the end of the summary.), except for the extra LF. Would this be a more
sensible route?

Contents:
[PATCH 0/3] commit: fix abbrev-sha regression
[PATCH 1/3] t7502-commit: add tests for summary output
[PATCH 2/3] t7502-commit: add summary output tests for empty and merge commits
[PATCH 3/3] commit: show abbreviated sha for commits with empty diffs

Signed-off-by: Tay Ray Chuan <redacted>
---
 t/t7502-commit.sh |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 844fb43..589e8e6 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh

@@ -4,8 +4,44 @@ test_description='git commit porcelain-ish'
 
 . ./test-lib.sh
 
+# Arguments: [<prefix] [<commit message>]
+check_summary_oneline() {
+	test_tick &&
+	git commit -m "$2" | head -1 > act &&
+
+	# branch name
+	SUMMARY_PREFIX="$(git name-rev --name-only HEAD)" &&
+
+	# append the "special" prefix, like "root-commit", "detached HEAD"
+	if test -n "$1"
+	then
+		SUMMARY_PREFIX="$SUMMARY_PREFIX ($1)"
+	fi
+
+	# abbrev SHA-1
+	SUMMARY_POSTFIX="$(git log -1 --pretty='format:%h')"
+	echo "[$SUMMARY_PREFIX $SUMMARY_POSTFIX] $2" >exp &&
+
+	test_cmp exp act
+}
+
+test_expect_success 'output summary format' '
+
+	echo new >file1 &&
+	git add file1 &&
+	check_summary_oneline "root-commit" "initial" &&
+
+	echo change >>file1 &&
+	git add file1 &&
+	check_summary_oneline "" "a change"
+'
+
 test_expect_success 'the basics' '
 
+	# this is needed for the "partial removal" test to pass
+	git rm file1 &&
+	git commit -m "cleanup" &&
+
 	echo doing partial >"commit is" &&
 	mkdir not &&
 	echo very much encouraged but we should >not/forbid &&

-- 
1.7.1.189.g07419

Signed-off-by: Tay Ray Chuan <redacted>
---
 builtin/commit.c  |    1 +
 t/t7502-commit.sh |    4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index a4e4966..ab4a7cf 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -1166,6 +1166,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 		struct pretty_print_context ctx = {0};
 		struct strbuf buf = STRBUF_INIT;
 		ctx.date_mode = DATE_NORMAL;
+		ctx.abbrev = DEFAULT_ABBREV;
 		format_commit_message(commit, format.buf + 7, &buf, &ctx);
 		printf("%s\n", buf.buf);
 		strbuf_release(&buf);

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 8a4a277..109ae00 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh

@@ -36,12 +36,12 @@ test_expect_success 'output summary format' '
 	check_summary_oneline "" "a change"
 '
 
-test_expect_failure 'output summary format for commit with an empty diff' '
+test_expect_success 'output summary format for commit with an empty diff' '
 
 	check_summary_oneline "" "empty" "--allow-empty"
 '
 
-test_expect_failure 'output summary format for merges' '
+test_expect_success 'output summary format for merges' '
 
 	git checkout -b recursive-base &&
 	test_commit base file1 &&

-- 
1.7.1.189.g07419

After c197702 (pretty: Respect --abbrev option), non-abbreviated hashes
began to appear, leading to failures for these tests.

Signed-off-by: Tay Ray Chuan <redacted>
---
 t/t7502-commit.sh |   29 +++++++++++++++++++++++++++--
 1 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 589e8e6..8a4a277 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh

@@ -4,10 +4,10 @@ test_description='git commit porcelain-ish'
 
 . ./test-lib.sh
 
-# Arguments: [<prefix] [<commit message>]
+# Arguments: [<prefix] [<commit message>] [<commit options>]
 check_summary_oneline() {
 	test_tick &&
-	git commit -m "$2" | head -1 > act &&
+	echo "$3" | xargs git commit -m "$2" | head -1 > act &&
 
 	# branch name
 	SUMMARY_PREFIX="$(git name-rev --name-only HEAD)" &&

@@ -36,6 +36,31 @@ test_expect_success 'output summary format' '
 	check_summary_oneline "" "a change"
 '
 
+test_expect_failure 'output summary format for commit with an empty diff' '
+
+	check_summary_oneline "" "empty" "--allow-empty"
+'
+
+test_expect_failure 'output summary format for merges' '
+
+	git checkout -b recursive-base &&
+	test_commit base file1 &&
+
+	git checkout -b recursive-a recursive-base &&
+	test_commit commit-a file1 &&
+
+	git checkout -b recursive-b recursive-base &&
+	test_commit commit-b file1 &&
+
+	# conflict
+	git checkout recursive-a &&
+	test_must_fail git merge recursive-b &&
+	# resolve the conflict
+	echo commit-a > file1 &&
+	git add file1 &&
+	check_summary_oneline "" "Merge"
+'
+
 test_expect_success 'the basics' '
 
 	# this is needed for the "partial removal" test to pass

-- 
1.7.1.189.g07419

The fix itself might be trivial, but the series seems to break 7502.20
and 7502.22

Thanks.

Tay Ray Chuan [off-list ref] writes:

Why do you have to fork xargs?  Wouldn't/shouldn't

    git commit ${3+"$3"} -m "$2"

work?

Hi,

On Wed, May 26, 2010 at 1:07 PM, Junio C Hamano [off-list ref] wrote:

Tay Ray Chuan [off-list ref] writes:

quoted

After c197702 (pretty: Respect --abbrev option), non-abbreviated hashes
began to appear, leading to failures for these tests.

Signed-off-by: Tay Ray Chuan <redacted>
---
 t/t7502-commit.sh |   29 +++++++++++++++++++++++++++--
 1 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 589e8e6..8a4a277 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh

@@ -4,10 +4,10 @@ test_description='git commit porcelain-ish'

 . ./test-lib.sh

-# Arguments: [<prefix] [<commit message>]
+# Arguments: [<prefix] [<commit message>] [<commit options>]
 check_summary_oneline() {
      test_tick &&
-     git commit -m "$2" | head -1 > act &&
+     echo "$3" | xargs git commit -m "$2" | head -1 > act &&

Why do you have to fork xargs?  Wouldn't/shouldn't

   git commit ${3+"$3"} -m "$2"

work?

It would. Blame it on my poor shell-fu. :)

-- 
Cheers,
Ray Chuan

Hi,

On Wed, May 26, 2010 at 1:07 PM, Junio C Hamano [off-list ref] wrote:

The fix itself might be trivial, but the series seems to break 7502.20
and 7502.22

I tested this patch series on top of:

 - efd1311 (Merge branch 'jn/shortlog' into next), which contains
   c197702 (pretty: Respect --abbrev option), the first offending commit
   to print un-abbreviated SHA-1s.

 - b26ba11, the recent 'next'.

It both cases, #20 and #22 passed fine.

Also, I outlined another possible fix in the cover letter to this
series:

  1274694452-4200-1-git-send-email-rctay89@gmail.com
  http://thread.gmane.org/gmane.comp.version-control.git/147619

I find the alternative fix a bit more precise; I'd appreciate your
comments, if you get the chance.

-- 
Cheers,
Ray Chuan

Hi,

On Wed, May 26, 2010 at 1:37 PM, Tay Ray Chuan [off-list ref] wrote:

Hi,

On Wed, May 26, 2010 at 1:07 PM, Junio C Hamano [off-list ref] wrote:

quoted

The fix itself might be trivial, but the series seems to break 7502.20
and 7502.22

I tested this patch series on top of:

 - efd1311 (Merge branch 'jn/shortlog' into next), which contains
  c197702 (pretty: Respect --abbrev option), the first offending commit
  to print un-abbreviated SHA-1s.

 - b26ba11, the recent 'next'.

It both cases, #20 and #22 passed fine.

My apologies - I've already squashed in a fix to address that locally
and I haven't sent that out yet.

-- 
Cheers,
Ray Chuan

After c197702 (pretty: Respect --abbrev option), the summary output for
git-commit began to throw up non-abbreviated SHA-1s.

For example,

  $ git commit --allow-empty -m "empty"
  [master f869bf09c8eefee54094bb21387241aaf5f10324] empty

I also notice that this happens for merges after conflict resolutions.

This afflicts 'master', but not any release.

Changes from v1:

 - move cleanup commands into separate function in patch #1.
 - add a cleanup command to patch #1 to accomodate tests #20 and #22.
 - changed shell syntax in patch #2 (based on Junio's suggestion).
 - used a more "aggressive" fix in patch #3.

Contents:
[PATCH v2 1/3] t7502-commit: add tests for summary output
[PATCH v2 2/3] t7502-commit: add summary output tests for empty and merge commits
[PATCH v2 3/3] commit::print_summary(): set rev_info.always_show_header to 1

This attempts to fix a regression in git-commit, where non-abbreviated
SHA-1s were printed in the summary.

One possible fix would be to set ctx.abbrev to DEFAULT_ABBREV in the
`if` block. However, we remove this codeblock altogether, and set
rev.always_show_header.

This way, we use back the same show_log() mechanism (instead of
format_commit_message()).

Quoting log-tree.c:560:

	shown = log_tree_diff(opt, commit, &log);
	if (!shown && opt->loginfo && opt->always_show_header) {
		log.parent = NULL;
		show_log(opt);
		shown = 1;
	}

This is the only area that always_show_header is checked, so the
setting of this flag should only affect this area.

Signed-off-by: Tay Ray Chuan <redacted>
---

Changed from v1:
 - used an alternative fix - see patch message for more.

 builtin/commit.c  |   13 ++++---------
 t/t7502-commit.sh |    4 ++--
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index a4e4966..2884d0c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -1148,7 +1148,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 	rev.verbose_header = 1;
 	rev.show_root_diff = 1;
 	get_commit_format(format.buf, &rev);
-	rev.always_show_header = 0;
+	rev.always_show_header = 1;
 	rev.diffopt.detect_rename = 1;
 	rev.diffopt.rename_limit = 100;
 	rev.diffopt.break_opt = 0;

@@ -1162,14 +1162,9 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 				head,
 		initial_commit ? " (root-commit)" : "");

-	if (!log_tree_commit(&rev, commit)) {
-		struct pretty_print_context ctx = {0};
-		struct strbuf buf = STRBUF_INIT;
-		ctx.date_mode = DATE_NORMAL;
-		format_commit_message(commit, format.buf + 7, &buf, &ctx);
-		printf("%s\n", buf.buf);
-		strbuf_release(&buf);
-	}
+	if (!log_tree_commit(&rev, commit))
+		die("unable to print summary");
+
 	strbuf_release(&format);
 }

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index b10541d..08c0247 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh

@@ -36,12 +36,12 @@ test_expect_success 'output summary format' '
 	check_summary_oneline "" "a change"
 '

-test_expect_failure 'output summary format for commit with an empty diff' '
+test_expect_success 'output summary format for commit with an empty diff' '

 	check_summary_oneline "" "empty" "--allow-empty"
 '

-test_expect_failure 'output summary format for merges' '
+test_expect_success 'output summary format for merges' '

 	git checkout -b recursive-base &&
 	test_commit base file1 &&
--

1.7.1.189.g07419

Signed-off-by: Tay Ray Chuan <redacted>
---

Changes from v1:
 - move cleanup commands into a separate function,
   output_tests_cleanup().
 - add a cleanup command to accomodate tests #20 and #22.

 t/t7502-commit.sh |   43 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 844fb43..478b637 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh

@@ -4,8 +4,51 @@ test_description='git commit porcelain-ish'

 . ./test-lib.sh

+# Arguments: [<prefix] [<commit message>]
+check_summary_oneline() {
+	test_tick &&
+	git commit -m "$2" | head -1 > act &&
+
+	# branch name
+	SUMMARY_PREFIX="$(git name-rev --name-only HEAD)" &&
+
+	# append the "special" prefix, like "root-commit", "detached HEAD"
+	if test -n "$1"
+	then
+		SUMMARY_PREFIX="$SUMMARY_PREFIX ($1)"
+	fi
+
+	# abbrev SHA-1
+	SUMMARY_POSTFIX="$(git log -1 --pretty='format:%h')"
+	echo "[$SUMMARY_PREFIX $SUMMARY_POSTFIX] $2" >exp &&
+
+	test_cmp exp act
+}
+
+test_expect_success 'output summary format' '
+
+	echo new >file1 &&
+	git add file1 &&
+	check_summary_oneline "root-commit" "initial" &&
+
+	echo change >>file1 &&
+	git add file1 &&
+	check_summary_oneline "" "a change"
+'
+
+output_tests_cleanup() {
+	# this is needed for "do not fire editor in the presence of conflicts"
+	git checkout master &&
+
+	# this is needed for the "partial removal" test to pass
+	git rm file1 &&
+	git commit -m "cleanup"
+}
+
 test_expect_success 'the basics' '

+	output_tests_cleanup &&
+
 	echo doing partial >"commit is" &&
 	mkdir not &&
 	echo very much encouraged but we should >not/forbid &&
--

1.7.1.189.g07419

After c197702 (pretty: Respect --abbrev option), non-abbreviated hashes
began to appear, leading to failures for these tests.

Signed-off-by: Tay Ray Chuan <redacted>
---

Changes from v1:
 - changed shell syntax in check_summary_oneline(), based on Junio's
   suggestion.

 t/t7502-commit.sh |   29 +++++++++++++++++++++++++++--
 1 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 478b637..b10541d 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh

@@ -4,10 +4,10 @@ test_description='git commit porcelain-ish'

 . ./test-lib.sh

-# Arguments: [<prefix] [<commit message>]
+# Arguments: [<prefix] [<commit message>] [<commit options>]
 check_summary_oneline() {
 	test_tick &&
-	git commit -m "$2" | head -1 > act &&
+	git commit ${3+"$3"} -m "$2" | head -1 > act &&

 	# branch name
 	SUMMARY_PREFIX="$(git name-rev --name-only HEAD)" &&

@@ -36,6 +36,31 @@ test_expect_success 'output summary format' '
 	check_summary_oneline "" "a change"
 '

+test_expect_failure 'output summary format for commit with an empty diff' '
+
+	check_summary_oneline "" "empty" "--allow-empty"
+'
+
+test_expect_failure 'output summary format for merges' '
+
+	git checkout -b recursive-base &&
+	test_commit base file1 &&
+
+	git checkout -b recursive-a recursive-base &&
+	test_commit commit-a file1 &&
+
+	git checkout -b recursive-b recursive-base &&
+	test_commit commit-b file1 &&
+
+	# conflict
+	git checkout recursive-a &&
+	test_must_fail git merge recursive-b &&
+	# resolve the conflict
+	echo commit-a > file1 &&
+	git add file1 &&
+	check_summary_oneline "" "Merge"
+'
+
 output_tests_cleanup() {
 	# this is needed for "do not fire editor in the presence of conflicts"
 	git checkout master &&
--

1.7.1.189.g07419

On Thu, 2010-05-27 at 23:34 +0800, Tay Ray Chuan wrote:

After c197702 (pretty: Respect --abbrev option), the summary output for
git-commit began to throw up non-abbreviated SHA-1s.

Please CC: me on any future versions of this series, if there are any
-- 
-- Will

Tay Ray Chuan [off-list ref] writes:

This attempts to fix a regression in git-commit, where non-abbreviated
SHA-1s were printed in the summary.

One possible fix would be to set ctx.abbrev to DEFAULT_ABBREV in the
`if` block. However, we remove this codeblock altogether, and set
rev.always_show_header.

This way, we use back the same show_log() mechanism (instead of
format_commit_message()).

I like the removal of the handcrafted call to f-c-m.  Thanks.

Quoting log-tree.c:560:

	shown = log_tree_diff(opt, commit, &log);
	if (!shown && opt->loginfo && opt->always_show_header) {
		log.parent = NULL;
		show_log(opt);
		shown = 1;
	}

This is the only area that always_show_header is checked, so the
setting of this flag should only affect this area.

Hmm, but also setting this flag would affect anything that changes
behaviour depending on the value of log.parent, no?

+	if (!log_tree_commit(&rev, commit))
+		die("unable to print summary");

When always_show_header is set, what are the situations where
log_tree_commit() might return false?  I think your fix depends on the
fact that it will never return false (which I think is a correct thing to
assume---after all that is what "always_show" means ;-).

On Sat, May 29, 2010 at 9:10 AM, Junio C Hamano [off-list ref] wrote:

Tay Ray Chuan [off-list ref] writes:

quoted

Quoting log-tree.c:560:

      shown = log_tree_diff(opt, commit, &log);
      if (!shown && opt->loginfo && opt->always_show_header) {
              log.parent = NULL;
              show_log(opt);
              shown = 1;
      }

This is the only area that always_show_header is checked, so the
setting of this flag should only affect this area.

Hmm, but also setting this flag would affect anything that changes
behaviour depending on the value of log.parent, no?

A cursory look at log-tree.c leads me to believe only show_log() cares
about log.parent.

In any case, if you look a few more lines up, you would notice another
log.parent = NULL:

@552
	struct log_info log;
	int shown;

	log.commit = commit;
	log.parent = NULL;
	opt->loginfo = &log;

	shown = log_tree_diff(opt, commit, &log);
	if (!shown && opt->loginfo && opt->always_show_header) {
		log.parent = NULL;
		show_log(opt);
		shown = 1;
	}

Therefore, I suspect that log.parent = NULL is a kind of
initialization for show_log() (log_tree_diff() does call show_log()
too).

quoted

+     if (!log_tree_commit(&rev, commit))
+             die("unable to print summary");

When always_show_header is set, what are the situations where
log_tree_commit() might return false?  I think your fix depends on the
fact that it will never return false (which I think is a correct thing to
assume---after all that is what "always_show" means ;-).

Based on my reading, I can't think of any.

-- 
Cheers,
Ray Chuan

This attempts to fix a regression in git-commit, where non-abbreviated
SHA-1s were printed in the summary.

One possible fix would be to set ctx.abbrev to DEFAULT_ABBREV in the
`if` block, where format_commit_message() is used.

Instead, we do away with the format_commit_message() codeblock
altogether, replacing it with a re-run of log_tree_commit().

We re-run log_tree_commit() with rev.always_show_header set, to force
the invocation of show_log(). The effect of this flag can be seen from
this excerpt from log-tree.c:560, the only area that
rev.always_show_header is checked:

	shown = log_tree_diff(opt, commit, &log);
	if (!shown && opt->loginfo && opt->always_show_header) {
		log.parent = NULL;
		show_log(opt);
		shown = 1;
	}

We also set rev.use_terminator, so that a newline is appended at the end
of the log message. Note that callers in builtin/log.c that also set
rev.always_show_header don't have to set rev.use_terminator, but still
get a newline, because they are wrapped in a pager.

Signed-off-by: Tay Ray Chuan <redacted>
---

This is a reworked version of the third patch of the
'tc/commit-abbrev-fix' series; there are no changes to the first and
second patches.

Changes from v5:

 - don't set rev.always_show_header immediately, so that when there is
   no diff, log_tree_commit() returns false, as before the patch;

 - when log_tree_commit() returns false (ie. no diff), set
   rev.always_show_header, and re-run log_tree_commit();

 - add a missing newline to summary output by setting
   rev.use_terminator.

See the patch message for more details.

In case you're wondering where's v5, it was sent in a private review:

  http://github.com/gitster/git/commit/c69160d

 builtin/commit.c  |   10 ++++------
 t/t7502-commit.sh |    4 ++--
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index a4e4966..aa92362 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -1163,13 +1163,11 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 		initial_commit ? " (root-commit)" : "");

 	if (!log_tree_commit(&rev, commit)) {
-		struct pretty_print_context ctx = {0};
-		struct strbuf buf = STRBUF_INIT;
-		ctx.date_mode = DATE_NORMAL;
-		format_commit_message(commit, format.buf + 7, &buf, &ctx);
-		printf("%s\n", buf.buf);
-		strbuf_release(&buf);
+		rev.always_show_header = 1;
+		rev.use_terminator = 1;
+		log_tree_commit(&rev, commit);
 	}
+
 	strbuf_release(&format);
 }

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index b10541d..08c0247 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh

@@ -36,12 +36,12 @@ test_expect_success 'output summary format' '
 	check_summary_oneline "" "a change"
 '

-test_expect_failure 'output summary format for commit with an empty diff' '
+test_expect_success 'output summary format for commit with an empty diff' '

 	check_summary_oneline "" "empty" "--allow-empty"
 '

-test_expect_failure 'output summary format for merges' '
+test_expect_success 'output summary format for merges' '

 	git checkout -b recursive-base &&
 	test_commit base file1 &&
--

1.7.1.189.g07419