Skip to content

Commit

Permalink
Merge branch 'ad/bisect-terms'
Browse files Browse the repository at this point in the history
The use of 'good/bad' in "git bisect" made it confusing to use when
hunting for a state change that is not a regression (e.g. bugfix).
The command learned 'old/new' and then allows the end user to
say e.g. "bisect start --term-old=fast --term=new=slow" to find a
performance regression.

Michael's idea to make 'good/bad' more intelligent does have
certain attractiveness ($gname/272867), and makes some of the work
on this topic a moot point.

* ad/bisect-terms:
  bisect: allow setting any user-specified in 'git bisect start'
  bisect: add 'git bisect terms' to view the current terms
  bisect: add the terms old/new
  bisect: sanity check on terms
  • Loading branch information
gitster committed Oct 5, 2015
2 parents dc5400e + 06e6a74 commit 22dd6eb
Show file tree
Hide file tree
Showing 4 changed files with 350 additions and 16 deletions.
103 changes: 100 additions & 3 deletions Documentation/git-bisect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ DESCRIPTION
The command takes various subcommands, and different options depending
on the subcommand:

git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<paths>...]
git bisect bad [<rev>]
git bisect good [<rev>...]
git bisect start [--term-{old,good}=<term> --term-{new,bad}=<term>]
[--no-checkout] [<bad> [<good>...]] [--] [<paths>...]
git bisect (bad|new) [<rev>]
git bisect (good|old) [<rev>...]
git bisect terms [--term-good | --term-bad]
git bisect skip [(<rev>|<range>)...]
git bisect reset [<commit>]
git bisect visualize
Expand All @@ -36,6 +38,13 @@ whether the selected commit is "good" or "bad". It continues narrowing
down the range until it finds the exact commit that introduced the
change.

In fact, `git bisect` can be used to find the commit that changed
*any* property of your project; e.g., the commit that fixed a bug, or
the commit that caused a benchmark's performance to improve. To
support this more general usage, the terms "old" and "new" can be used
in place of "good" and "bad", or you can choose your own terms. See
section "Alternate terms" below for more information.

Basic bisect commands: start, bad, good
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -111,6 +120,79 @@ bad revision, while `git bisect reset HEAD` will leave you on the
current bisection commit and avoid switching commits at all.


Alternate terms
~~~~~~~~~~~~~~~

Sometimes you are not looking for the commit that introduced a
breakage, but rather for a commit that caused a change between some
other "old" state and "new" state. For example, you might be looking
for the commit that introduced a particular fix. Or you might be
looking for the first commit in which the source-code filenames were
finally all converted to your company's naming standard. Or whatever.

In such cases it can be very confusing to use the terms "good" and
"bad" to refer to "the state before the change" and "the state after
the change". So instead, you can use the terms "old" and "new",
respectively, in place of "good" and "bad". (But note that you cannot
mix "good" and "bad" with "old" and "new" in a single session.)

In this more general usage, you provide `git bisect` with a "new"
commit has some property and an "old" commit that doesn't have that
property. Each time `git bisect` checks out a commit, you test if that
commit has the property. If it does, mark the commit as "new";
otherwise, mark it as "old". When the bisection is done, `git bisect`
will report which commit introduced the property.

To use "old" and "new" instead of "good" and bad, you must run `git
bisect start` without commits as argument and then run the following
commands to add the commits:

------------------------------------------------
git bisect old [<rev>]
------------------------------------------------

to indicate that a commit was before the sought change, or

------------------------------------------------
git bisect new [<rev>...]
------------------------------------------------

to indicate that it was after.

To get a reminder of the currently used terms, use

------------------------------------------------
git bisect terms
------------------------------------------------

You can get just the old (respectively new) term with `git bisect term
--term-old` or `git bisect term --term-good`.

If you would like to use your own terms instead of "bad"/"good" or
"new"/"old", you can choose any names you like (except existing bisect
subcommands like `reset`, `start`, ...) by starting the
bisection using

------------------------------------------------
git bisect start --term-old <term-old> --term-new <term-new>
------------------------------------------------

For example, if you are looking for a commit that introduced a
performance regression, you might use

------------------------------------------------
git bisect start --term-old fast --term-new slow
------------------------------------------------

Or if you are looking for the commit that fixed a bug, you might use

------------------------------------------------
git bisect start --term-new fixed --term-old broken
------------------------------------------------

Then, use `git bisect <term-old>` and `git bisect <term-new>` instead
of `git bisect good` and `git bisect bad` to mark commits.

Bisect visualize
~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -387,6 +469,21 @@ In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit
has at least one parent whose reachable graph is fully traversable in the sense
required by 'git pack objects'.

* Look for a fix instead of a regression in the code
+
------------
$ git bisect start
$ git bisect new HEAD # current commit is marked as new
$ git bisect old HEAD~10 # the tenth commit from now is marked as old
------------
+
or:
------------
$ git bisect start --term-old broken --term-new fixed
$ git bisect fixed
$ git bisect broken HEAD~10
------------

Getting help
~~~~~~~~~~~~

Expand Down
11 changes: 8 additions & 3 deletions bisect.c
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,11 @@ static void handle_bad_merge_base(void)
"This means the bug has been fixed "
"between %s and [%s].\n",
bad_hex, bad_hex, good_hex);
} else if (!strcmp(term_bad, "new") && !strcmp(term_good, "old")) {
fprintf(stderr, "The merge base %s is new.\n"
"The property has changed "
"between %s and [%s].\n",
bad_hex, bad_hex, good_hex);
} else {
fprintf(stderr, "The merge base %s is %s.\n"
"This means the first '%s' commit is "
Expand Down Expand Up @@ -762,11 +767,11 @@ static void handle_skipped_merge_base(const unsigned char *mb)
}

/*
* "check_merge_bases" checks that merge bases are not "bad".
* "check_merge_bases" checks that merge bases are not "bad" (or "new").
*
* - If one is "bad", it means the user assumed something wrong
* - If one is "bad" (or "new"), it means the user assumed something wrong
* and we must exit with a non 0 error code.
* - If one is "good", that's good, we have nothing to do.
* - If one is "good" (or "old"), that's good, we have nothing to do.
* - If one is "skipped", we can't know but we should warn.
* - If we don't know, we should check it out and ask the user to test.
*/
Expand Down
117 changes: 107 additions & 10 deletions git-bisect.sh
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
#!/bin/sh

USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
LONG_USAGE='git bisect help
print this long help message.
git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<pathspec>...]
git bisect start [--term-{old,good}=<term> --term-{new,bad}=<term>]
[--no-checkout] [<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 (bad|new) [<rev>]
mark <rev> a known-bad revision/
a revision after change in a given property.
git bisect (good|old) [<rev>...]
mark <rev>... known-good revisions/
revisions before change in a given property.
git bisect terms [--term-good | --term-bad]
show the terms used for old and new commits (default: bad, good)
git bisect skip [(<rev>|<range>)...]
mark <rev>... untestable revisions.
git bisect next
Expand Down Expand Up @@ -95,6 +100,24 @@ bisect_start() {
--no-checkout)
mode=--no-checkout
shift ;;
--term-good|--term-old)
shift
must_write_terms=1
TERM_GOOD=$1
shift ;;
--term-good=*|--term-old=*)
must_write_terms=1
TERM_GOOD=${1#*=}
shift ;;
--term-bad|--term-new)
shift
must_write_terms=1
TERM_BAD=$1
shift ;;
--term-bad=*|--term-new=*)
must_write_terms=1
TERM_BAD=${1#*=}
shift ;;
--*)
die "$(eval_gettext "unrecognised option: '\$arg'")" ;;
*)
Expand Down Expand Up @@ -294,7 +317,7 @@ bisect_next_check() {
false
;;
t,,"$TERM_GOOD")
# have bad but not good. we could bisect although
# have bad (or new) but not good (or old). we could bisect although
# this is less optimum.
eval_gettextln "Warning: bisecting only with a \$TERM_BAD commit." >&2
if test -t 0
Expand Down Expand Up @@ -451,6 +474,8 @@ bisect_replay () {
eval "$cmd" ;;
"$TERM_GOOD"|"$TERM_BAD"|skip)
bisect_write "$command" "$rev" ;;
terms)
bisect_terms $rev ;;
*)
die "$(gettext "?? what are you talking about?")" ;;
esac
Expand Down Expand Up @@ -535,9 +560,42 @@ get_terms () {
write_terms () {
TERM_BAD=$1
TERM_GOOD=$2
if test "$TERM_BAD" = "$TERM_GOOD"
then
die "$(gettext "please use two different terms")"
fi
check_term_format "$TERM_BAD" bad
check_term_format "$TERM_GOOD" good
printf '%s\n%s\n' "$TERM_BAD" "$TERM_GOOD" >"$GIT_DIR/BISECT_TERMS"
}

check_term_format () {
term=$1
git check-ref-format refs/bisect/"$term" ||
die "$(eval_gettext "'\$term' is not a valid term")"
case "$term" in
help|start|terms|skip|next|reset|visualize|replay|log|run)
die "$(eval_gettext "can't use the builtin command '\$term' as a term")"
;;
bad|new)
if test "$2" != bad
then
# In theory, nothing prevents swapping
# completely good and bad, but this situation
# could be confusing and hasn't been tested
# enough. Forbid it for now.
die "$(eval_gettext "can't change the meaning of term '\$term'")"
fi
;;
good|old)
if test "$2" != good
then
die "$(eval_gettext "can't change the meaning of term '\$term'")"
fi
;;
esac
}

check_and_set_terms () {
cmd="$1"
case "$cmd" in
Expand All @@ -554,14 +612,51 @@ check_and_set_terms () {
write_terms bad good
fi
;;
new|old)
if ! test -s "$GIT_DIR/BISECT_TERMS"
then
write_terms new old
fi
;;
esac ;;
esac
}

bisect_voc () {
case "$1" in
bad) echo "bad" ;;
good) echo "good" ;;
bad) echo "bad|new" ;;
good) echo "good|old" ;;
esac
}

bisect_terms () {
get_terms
if ! test -s "$GIT_DIR/BISECT_TERMS"
then
die "$(gettext "no terms defined")"
fi
case "$#" in
0)
gettextln "Your current terms are $TERM_GOOD for the old state
and $TERM_BAD for the new state."
;;
1)
arg=$1
case "$arg" in
--term-good|--term-old)
printf '%s\n' "$TERM_GOOD"
;;
--term-bad|--term-new)
printf '%s\n' "$TERM_BAD"
;;
*)
die "$(eval_gettext "invalid argument \$arg for 'git bisect terms'.
Supported options are: --term-good|--term-old and --term-bad|--term-new.")"
;;
esac
;;
*)
usage ;;
esac
}

Expand All @@ -577,7 +672,7 @@ case "$#" in
git bisect -h ;;
start)
bisect_start "$@" ;;
bad|good|"$TERM_BAD"|"$TERM_GOOD")
bad|good|new|old|"$TERM_BAD"|"$TERM_GOOD")
bisect_state "$cmd" "$@" ;;
skip)
bisect_skip "$@" ;;
Expand All @@ -594,6 +689,8 @@ case "$#" in
bisect_log ;;
run)
bisect_run "$@" ;;
terms)
bisect_terms "$@" ;;
*)
usage ;;
esac
Expand Down
Loading

0 comments on commit 22dd6eb

Please sign in to comment.