Re: [RFC/PATCH] Bisect: implement "git bisect run <cmd>..." to automatically bisect.

From: Christian Couder <chriscool@tuxfamily.org>
Date: Mar 22, 2007, 21:59 UTC
Message-ID: <200703222259.49089.chriscool@tuxfamily.org>
In-Reply-To: <7vejnhwy0y.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano a écrit :

[...]

Show 15 quoted lines

> > +      # Check for really bad run error.
> > +      if [ $res -lt 0 -o $res -ge 128 ]; then
> > +	  echo >&2 "bisect run failed:"
> > +	  echo >&2 "exit code $res from '$@' is < 0 or >= 128"
> > +	  exit $res
> > +      fi
>
> I am not sure if this flexibility/leniency is desirable.  It
> certainly allows a sloppily written shell script that exits with
> any random small-positive values to report a badness, which may
> be handy, but allowing sloppiness might lead to wasted time
> after all.  I dunno.  A more strict convention that says the run
> script should exit 1 to signal "bad, please continue", 0 for
> "good, please continue" and other values for "no good, abort"
> might be less error prone.

Perhaps, but when running "git bisect run make" to automatically find the commit that broke the build, it would fail because make will usually return 2 in case of error.

It seems that there are few standards for exit code, so whatever convention we choose will not work in all cases.

> In any case, the exit status convention needs documentation.

Yes, I will work on it.

[...]

Show 16 quoted lines

> > +      # Use "git-bisect good" or "git-bisect bad"
> > +      # depending on run success or failure.
> > +      # We cannot use bisect_good or bisect_bad functions
> > +      # because they can exit.
> > +      if [ $res -gt 0 ]; then
> > +	  next_bisect='git-bisect bad'
> > +      else
> > +	  next_bisect='git-bisect good'
> > +      fi
> > +
> > +      $next_bisect > "$GIT_DIR/BISECT_RUN"
>
> If their exiting, and possibly variable assignments, are the
> only problem, you can run that in subshell, can't you?  Like:
>
> 	( $next_bisect >"$GIT_DIR/BISECT_RUN" )

You are right. I will submit an updated patch with this change and some documentation.

Thanks, Christian.