All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Couder <chriscool@tuxfamily.org>
To: hegge@resisty.net
Cc: git@vger.kernel.org
Subject: Re: [PATCH] bisect: Store first bad commit as comment in log file
Date: Mon, 15 Apr 2013 06:38:09 +0200 (CEST)	[thread overview]
Message-ID: <20130415.063809.1055555229072260139.chriscool@tuxfamily.org> (raw)
In-Reply-To: <20130413152257.GB16040@pvv.ntnu.no>

From: Torstein Hegge <hegge@resisty.net>
Subject: [PATCH] bisect: Store first bad commit as comment in log file
Date: Sat, 13 Apr 2013 17:22:57 +0200

> When bisect successfully finds a single revision, the first bad commit
> should be shown to human readers of 'git bisect log'.
> 
> This resolves the apparent disconnect between the bisection result and
> the log when a bug reporter says "I know that the first bad commit is
> $rev, as you can see from $(git bisect log)".

I agree that it's a good idea to do that.

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.
 
> Signed-off-by: Torstein Hegge <hegge@resisty.net>
> ---
> I don't know how useful the added test is, I didn't find any existing
> tests that looks at the comment parts of bisect log.

Thanks for adding a test. It's always appreciated.

>  git-bisect.sh               |    8 +++++++-
>  t/t6030-bisect-porcelain.sh |   18 ++++++++++++++++++
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> 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.

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

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

Otherwise this patch looks good to me.

> +		bad_commit=$(git show-branch $bad_rev)
> +		echo "# first bad commit: $bad_commit" >>"$GIT_DIR/BISECT_LOG"
> +		exit 0
> +	fi

Thanks,
Christian.

  reply	other threads:[~2013-04-15  5:05 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-13 15:22 [PATCH] bisect: Store first bad commit as comment in log file Torstein Hegge
2013-04-15  4:38 ` Christian Couder [this message]
2013-04-15  9:53   ` Torstein Hegge
2013-04-15 15:00     ` Junio C Hamano
2013-04-22 21:02     ` Torstein Hegge
2013-04-22 21:13       ` Junio C Hamano
2013-04-22 22:20         ` Torstein Hegge
2013-04-22 22:37           ` Junio C Hamano
2013-04-25  4:26           ` Christian Couder
2013-05-22 22:27       ` [PATCH] bisect: Fix log output for multi-parent skip ranges Torstein Hegge
2013-04-15  6:50 ` [PATCH] bisect: Store first bad commit as comment in log file Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130415.063809.1055555229072260139.chriscool@tuxfamily.org \
    --to=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=hegge@resisty.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.