From: Junio C Hamano <junkio@cox.net>
Date: Tue, 03 Apr 2007 20:02:13 GMT
Subject: Re: [RFC/PATCH] Bisect: teach "bisect start" to optionally use one bad and many good revs.
Message-ID: <7vzm5pw7ju.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20070401145723.27e17177.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> In fact this patch implements:
>
>     git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
>
> I think this is more backward compatible because older script
> probably didn't used -- before <pathspec>...
>
> On the other hand, there may be some confusion between revs
> (<bad> and <good>...) and <pathspec>... if -- is not used
> and if an invalid rev or a pathspec that looks like a rev is
> given.

I think the intent of the check you have for '--' to cover the
above issue is good.

>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  git-bisect.sh         |  105 ++++++++++++++++++++++++++++++++++++++-----------
>  t/t6030-bisect-run.sh |   20 ++++++++-
>  2 files changed, 99 insertions(+), 26 deletions(-)
>
> diff --git a/git-bisect.sh b/git-bisect.sh
> index 11313a7..d5a13ee 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -1,15 +1,24 @@
>  #!/bin/sh
>  
>  USAGE='[start|bad|good|next|reset|visualize|replay|log|run]'
> -LONG_USAGE='git bisect start [<pathspec>]	reset bisect state and start bisection.
> -git bisect bad [<rev>]		mark <rev> a known-bad revision.
> -git bisect good [<rev>...]	mark <rev>... known-good revisions.
> -git bisect next			find next bisection to test and check it out.
> -git bisect reset [<branch>]	finish bisection search and go back to branch.
> -git bisect visualize            show bisect status in gitk.
> -git bisect replay <logfile>	replay bisection log.
> -git bisect log			show bisect log.
> -git bisect run <cmd>... 	use <cmd>... to automatically bisect.'
> +LONG_USAGE='git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
> +        reset bisect state and start bisection.
> +git bisect bad [<rev>]
> +        mark <rev> a known-bad revision.
> +git bisect good [<rev>...]
> +        mark <rev>... known-good revisions.
> +git bisect next
> +        find next bisection to test and check it out.
> +git bisect reset [<branch>]
> +        finish bisection search and go back to branch.
> +git bisect visualize
> +        show bisect status in gitk.
> +git bisect replay <logfile>
> +        replay bisection log.
> +git bisect log
> +        show bisect log.
> +git bisect run <cmd>...
> +        use <cmd>... to automatically bisect.'

Much easier to read ;-).

>  	mkdir "$GIT_DIR/refs/bisect"
> +
> +	#
> +	# Check for one bad and then some good revisions.
> +	#
> +	has_double_dash=0
> +	for arg in "$@"; do
> +	    case "$arg" in --) has_double_dash=1; break ;; esac
> +	done

Style.  'in "$@"' is superfluous.

	for arg
        do
        	...
	done

> +	orig_args="$@"

Doesn't this defeat the whole point of later running 'sq' on it?
The reason we do sq is to protect whitespaces in pathspecs and
make the strings correctly split when evaled/sourced.

A simple test program:

        #!/bin/sh
        sq () {
                echo "** fake sq $* **"
                i=0
                for j
                do
                        echo "$i: <$j>"
                        i=$(( $i + 1 ))
                done
                echo
        }

        set x a 'b c' d
        shift
        # Now, $# = 3, $1 = 'a', $2 = 'b c' and $3 = 'd'
        orig_args="$@"
        sq "$@"
        sq "$orig_args"

> ...
> -	    sq "$@"
> -	} >"$GIT_DIR/BISECT_LOG"
> -	sq "$@" >"$GIT_DIR/BISECT_NAMES"
> +	    sq "$orig_args"
> +	} >>"$GIT_DIR/BISECT_LOG"
> +	bisect_auto_next
>  }