Re: [PATCH] bisect: Store first bad commit as comment in log file

On Mon, Apr 15, 2013 at 06:38:09 +0200, Christian Couder wrote:

I wonder if we should also write something into the bisect log if for
example the bisection stopped because there are only 'skip'ped commits
left to test. But maybe this could go into another patch after this
one.

Yes, that would be useful, but I wasn't able to determine all the cases
that would be relevant to log. Only skipped commits left to test is one,
but bisect--helper also exits on various problems related to merge base
handling. The handling of problems related to inconsistent user input is
probably not relevant to log.

I think the successful bisect case is most important to log and the one
that requires the least amount of invasive changes.

quoted

diff --git a/git-bisect.sh b/git-bisect.sh
index 99efbe8..c58eea7 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh

@@ -311,7 +311,13 @@ bisect_next() {
 	res=$?
 
 	# Check if we should exit because bisection is finished
-	test $res -eq 10 && exit 0
+	if test $res -eq 10
+	then
+		bad_rev=$(git show-ref --hash --verify refs/bisect/bad)

I had a look to make sure that refs/bisect/bad always refered to the
first bad commit at this point, and it is true indeed.

According to Documentation/git-bisect.txt, refs/bisect/bad is the proper
way to determine the first bad commit at the end of a bisection.

Maybe you could have used "git rev-parse --verify" instead of "git
show-ref --hash --verify". It looks simpler to me.

I was wondering why "git grep show-ref *.sh" gave so few users. It looks
like rev-parse is more common.

And maybe, just in case, you could have added: || die "$(gettext "Bad rev: refs/bisect/bad")"

Yes, I should probably have done that.

Otherwise this patch looks good to me.

Thanks.


Torstein