Re: [PATCH 2/2] Bisect: implement "bisect dunno" to mark untestable revisions.
- From
Christian Couder <chriscool@tuxfamily.org>
- Date
- Oct 8, 2007, 05:34 UTC
- Message-ID
- <200710080734.23878.chriscool@tuxfamily.org>
- In-Reply-To
- <Pine.LNX.4.64.0710080444290.4174@racer.site>
Le lundi 8 octobre 2007, Johannes Schindelin a écrit :
Show 15 quoted lines
> Hi,
>
> On Mon, 8 Oct 2007, Christian Couder wrote:
> > diff --git a/git-bisect.sh b/git-bisect.sh
> > index 388887a..c556318 100755
> > --- a/git-bisect.sh
> > +++ b/git-bisect.sh
> > @@ -143,7 +145,7 @@ bisect_write_bad() {
> >
> > bisect_good() {
> > bisect_autostart
> > - case "$#" in
> > + case "$#" in
>
> White space breakage.The patch tries to fix some white space breakages.
Show 7 quoted lines
> > @@ -153,7 +155,6 @@ bisect_good() {
> > rev=$(git rev-parse --verify "$rev^{commit}") || exit
> > bisect_write_good "$rev"
> > echo "git-bisect good $rev" >>"$GIT_DIR/BISECT_LOG"
> > -
>
> ?It also removes this unneeded blank line.
Show 21 quoted lines
> > @@ -164,6 +165,28 @@ bisect_write_good() {
> > echo "# good: "$(git show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
> > }
> >
> > +bisect_dunno() {
> > + bisect_autostart
> > + case "$#" in
> > + 0) revs=$(git rev-parse --verify HEAD) || exit ;;
> > + *) revs=$(git rev-parse --revs-only --no-flags "$@") &&
> > + test '' != "$revs" || die "Bad rev input: $@" ;;
> > + esac
> > + for rev in $revs
> > + do
> > + rev=$(git rev-parse --verify "$rev^{commit}") || exit
> > + bisect_write_dunno "$rev"
> > + echo "git-bisect dunno $rev" >>"$GIT_DIR/BISECT_LOG"
>
> Should the last line not be put into bisect_write_dunno? OTOH this is
> the only call site of that function, so I strongly doubt that the
> function (consisting of 3 lines, where the first is 'rev="$1"') is
> necessary at all.Well, there are "bisect_write_bad" and "bisect_write_good" that already do the same thing as "bisect_write_dunno". In fact I thought that it was better to just copy "bisect_dunno" from "bisect_good" and "bisect_write_dunno" from "bisect_write_good".
If needed I can send another patch to factorise these functions.
Show 17 quoted lines
> > @@ -206,17 +229,104 @@ bisect_auto_next() {
> > bisect_next_check && bisect_next || :
> > }
> >
> > +search_dunno() {
> > + _hash="$1"
> > + _dunno="$2"
> > +
> > + for _val in $_dunno ; do
> > + case $_hash in $_val) return 1 ;; esac
> > + done
>
> This would be faster as
>
> case " $1" in " $2") return 1 ;; esac
>
> I guess.I will try your suggestion and send an updated patch. Thanks.
> But as I said in the other reply, I think this logic belongs into the C > core, instead of generating mostly useless information, passing it down > to the script, and filtering it out again.
Yeah, it's not efficient.
Best regards, Christian.