Re: [PATCH] bisect: print abbrev sha1 for first bad commit

From: Jeff King <peff@peff.net>
Date: May 11, 2015, 01:10 UTC
Message-ID: <20150511011009.GA21830@peff.net>
In-Reply-To: <20150510231110.GA25157@tsaunders-iceball.corp.tor1.mozilla.com>

On Sun, May 10, 2015 at 07:12:45PM -0400, Trevor Saunders wrote:

Show 8 quoted lines

> > Yeah, I have always found bisect's output somewhat silly. It prints the
> > "--raw" diff output, which is not incredibly useful. And then to top it
> > off, it does not feed the "--recursive" switch to the diff, so you don't
> > even get to see the real list of changed files.
> 
>  So, fun fact it doesn't actually always print the raw diffoutput if
>  there is no diff, for example a merge where both sides only touched
>  different files as in test 40 in t6030.

Ah, that makes sense. It's basically just feeding the commit to "diff-tree" (except doing it internally rather than running it as a separate program). And the defaults there do not show anything for merge commits. It could do the equivalent of "--cc" (i.e., set the dense_combined_merges flag in the "struct rev_info").

Show 7 quoted lines

> > (Actually, it looks like all this is generated in bisect.c:show_diff_tree,
> > so it would have to be written in C; but it should be pretty easy to
> > tweak the display options).
> 
> yeah, that seems pretty straight forward, but I'm not really sure what
> to do about this case where no diff is printed, I guess I should figure
> out what bits need to be set for the commit to be shown anyway.

I'd argue for simply never showing the diff (dropping the "opt.diff = 1" line from bisect.c:show_diff_tree), but that is mostly my personal opinion. If we are going to show a diff, perhaps "--recursive --name-status" would be the most friendly, with "--cc" for the merge commits.

Translated into C, something like (this is completely untested):

diff --git a/bisect.c b/bisect.c
index 10f5e57..62786cf 100644
--- a/bisect.c
+++ b/bisect.c
@@ -876,6 +876,8 @@ static void show_diff_tree(const char *prefix, struct commit *commit)
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	opt.abbrev = 0;
 	opt.diff = 1;
+	opt.combine_merges = 1;
+	opt.dense_combined_merges = 1;
 
 	/* This is what "--pretty" does */
 	opt.verbose_header = 1;
@@ -884,7 +886,8 @@ static void show_diff_tree(const char *prefix, struct commit *commit)
 
 	/* diff-tree init */
 	if (!opt.diffopt.output_format)
-		opt.diffopt.output_format = DIFF_FORMAT_RAW;
+		opt.diffopt.output_format = DIFF_FORMAT_NAME_STATUS;
+	DIFF_OPT_SET(&opt.diffopt, RECURSIVE);
 
 	log_tree_commit(&opt, commit);
 }