Merge branch 'ad/bisect-terms'
authorJunio C Hamano <gitster@pobox.com>
Mon, 5 Oct 2015 19:30:06 +0000 (12:30 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 5 Oct 2015 19:30:06 +0000 (12:30 -0700)
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

Documentation/git-bisect.txt
bisect.c
git-bisect.sh
t/t6030-bisect-porcelain.sh
index e97f2de21bdc58cc2de35731f7b339353b121bd0..2044fe6820e0507a6dcc1823f8fe3fce407e4d90 100644 (file)
@@ -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
@@ -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
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -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
 ~~~~~~~~~~~~~~~~
 
@@ -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
 ~~~~~~~~~~~~
 
index 041a13d093a21597c60d799c0f6943998f2d6a8a..053d1a2ab91a6c41a0fe957569ff905d12b00bee 100644 (file)
--- a/bisect.c
+++ b/bisect.c
@@ -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 "
@@ -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.
  */
index ea63223ab3b5d4f8fb0a9af45db4278aef010fb7..5d1cb00d86b3700b2012cae1ed3b3b8b744acaf2 100755 (executable)
@@ -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
@@ -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'")" ;;
                *)
@@ -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
@@ -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
@@ -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
@@ -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
 }
 
@@ -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 "$@" ;;
@@ -594,6 +689,8 @@ case "$#" in
                bisect_log ;;
        run)
                bisect_run "$@" ;;
+       terms)
+               bisect_terms "$@" ;;
        *)
                usage ;;
        esac
index 9e2c20374732d790edefc4cd87aa28223fbb24c4..e74662ba5c638de5acb4188e2cd0c5ff2456ec13 100755 (executable)
@@ -759,4 +759,139 @@ test_expect_success '"git bisect bad HEAD" behaves as "git bisect bad"' '
        git bisect reset
 '
 
+test_expect_success 'bisect starts with only one new' '
+       git bisect reset &&
+       git bisect start &&
+       git bisect new $HASH4 &&
+       git bisect next
+'
+
+test_expect_success 'bisect does not start with only one old' '
+       git bisect reset &&
+       git bisect start &&
+       git bisect old $HASH1 &&
+       test_must_fail git bisect next
+'
+
+test_expect_success 'bisect start with one new and old' '
+       git bisect reset &&
+       git bisect start &&
+       git bisect old $HASH1 &&
+       git bisect new $HASH4 &&
+       git bisect new &&
+       git bisect new >bisect_result &&
+       grep "$HASH2 is the first new commit" bisect_result &&
+       git bisect log >log_to_replay.txt &&
+       git bisect reset
+'
+
+test_expect_success 'bisect replay with old and new' '
+       git bisect replay log_to_replay.txt >bisect_result &&
+       grep "$HASH2 is the first new commit" bisect_result &&
+       git bisect reset
+'
+
+test_expect_success 'bisect cannot mix old/new and good/bad' '
+       git bisect start &&
+       git bisect bad $HASH4 &&
+       test_must_fail git bisect old $HASH1
+'
+
+test_expect_success 'bisect terms needs 0 or 1 argument' '
+       git bisect reset &&
+       test_must_fail git bisect terms only-one &&
+       test_must_fail git bisect terms 1 2 &&
+       test_must_fail git bisect terms 2>actual &&
+       echo "no terms defined" >expected &&
+       test_cmp expected actual
+'
+
+test_expect_success 'bisect terms shows good/bad after start' '
+       git bisect reset &&
+       git bisect start HEAD $HASH1 &&
+       git bisect terms --term-good >actual &&
+       echo good >expected &&
+       test_cmp expected actual &&
+       git bisect terms --term-bad >actual &&
+       echo bad >expected &&
+       test_cmp expected actual
+'
+
+test_expect_success 'bisect start with one term1 and term2' '
+       git bisect reset &&
+       git bisect start --term-old term2 --term-new term1 &&
+       git bisect term2 $HASH1 &&
+       git bisect term1 $HASH4 &&
+       git bisect term1 &&
+       git bisect term1 >bisect_result &&
+       grep "$HASH2 is the first term1 commit" bisect_result &&
+       git bisect log >log_to_replay.txt &&
+       git bisect reset
+'
+
+test_expect_success 'bisect replay with term1 and term2' '
+       git bisect replay log_to_replay.txt >bisect_result &&
+       grep "$HASH2 is the first term1 commit" bisect_result &&
+       git bisect reset
+'
+
+test_expect_success 'bisect start term1 term2' '
+       git bisect reset &&
+       git bisect start --term-new term1 --term-old term2 $HASH4 $HASH1 &&
+       git bisect term1 &&
+       git bisect term1 >bisect_result &&
+       grep "$HASH2 is the first term1 commit" bisect_result &&
+       git bisect log >log_to_replay.txt &&
+       git bisect reset
+'
+
+test_expect_success 'bisect cannot mix terms' '
+       git bisect reset &&
+       git bisect start --term-good term1 --term-bad term2 $HASH4 $HASH1 &&
+       test_must_fail git bisect a &&
+       test_must_fail git bisect b &&
+       test_must_fail git bisect bad &&
+       test_must_fail git bisect good &&
+       test_must_fail git bisect new &&
+       test_must_fail git bisect old
+'
+
+test_expect_success 'bisect terms rejects invalid terms' '
+       git bisect reset &&
+       test_must_fail git bisect start --term-good invalid..term &&
+       test_must_fail git bisect terms --term-bad invalid..term &&
+       test_must_fail git bisect terms --term-good bad &&
+       test_must_fail git bisect terms --term-good old &&
+       test_must_fail git bisect terms --term-good skip &&
+       test_must_fail git bisect terms --term-good reset &&
+       test_path_is_missing .git/BISECT_TERMS
+'
+
+test_expect_success 'bisect start --term-* does store terms' '
+       git bisect reset &&
+       git bisect start --term-bad=one --term-good=two &&
+       git bisect terms >actual &&
+       cat <<-EOF >expected &&
+       Your current terms are two for the old state
+       and one for the new state.
+       EOF
+       test_cmp expected actual &&
+       git bisect terms --term-bad >actual &&
+       echo one >expected &&
+       test_cmp expected actual &&
+       git bisect terms --term-good >actual &&
+       echo two >expected &&
+       test_cmp expected actual
+'
+
+test_expect_success 'bisect start takes options and revs in any order' '
+       git bisect reset &&
+       git bisect start --term-good one $HASH4 \
+               --term-good two --term-bad bad-term \
+               $HASH1 --term-good three -- &&
+       (git bisect terms --term-bad && git bisect terms --term-good) >actual &&
+       printf "%s\n%s\n" bad-term three >expected &&
+       test_cmp expected actual
+'
+
 test_done