{"thread":{"id":"39520","subject":"[PATCH 2/3] bisect: use name_(bad|good) instead of bisect_(bad|good)","startedAt":"2015-06-04T07:59:11Z","lastAt":"2015-06-04T18:05:08Z","messageCount":6,"participants":["Antoine Delaite","Matthieu Moy","Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"262934","messageId":"1433404753-12039-1-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","threadId":"39520","inReplyTo":null,"subject":"[WIP PATCH 1/3] git bisect old/new","fromName":"Antoine Delaite","fromEmail":"antoine.delaite@ensimag.grenoble-inp.fr","sentAt":"2015-06-04T07:59:11Z","receivedAt":"2015-06-04T07:59:11Z","isPatch":true,"sender":{"key":"antoine.delaite@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12510393?v=4"},"body":"From: Christian Couder <chriscool@tuxfamily.org>\n\nWhen not looking for a regression during a bisect but for a fix or a\nchange in another given property, it can be confusing to use 'good'\nand 'bad'.\n\nThis patch introduce `git bisect new` and `git bisect old` as an\nalternative to 'bad' and good': the commits which have the most\nrecent version of the property must be marked as `new` and the ones\nwith the older version as `old`.\n\nThe output will be the first commit after the change in the property.\nDuring a new/old bisect session you cannot use bad/good commands and\nvice-versa.\n\n`git bisect replay` works fine for old/new bisect sessions.\n\nSome commands are still not available for old/new:\n\n     * git bisect start [<new> [<old>...]] is not possible: the\n       commits will be treated as bad and good.\n     * git rev-list --bisect does not treat the revs/bisect/new and\n       revs/bisect/old-SHA1 files.\n     * git bisect visualize seem to work partially: the tags are\n       displayed correctly but the tree is not limited to the bisect\n       section.\n\nRelated discussions:\n\n\t- http://thread.gmane.org/gmane.comp.version-control.git/86063\n\t\tintroduced bisect fix unfixed to find fix.\n\t- http://thread.gmane.org/gmane.comp.version-control.git/182398\n\t\tdiscussion around bisect yes/no or old/new.\n\t- http://thread.gmane.org/gmane.comp.version-control.git/199758\n\t\tlast discussion and reviews\n\nSigned-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>\nSigned-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>\nSigned-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>\nSigned-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>\nSigned-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>\nSigned-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>\nSigned-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>\n---\nWe took account of most of the \"easy\" reviews that were discuted two years ago.\nWe hope we didn't missed any.\nhttp://thread.gmane.org/gmane.comp.version-control.git/199758\n\nWe corrected various issues that were not reported: \n-one that caused a \"fatal ... not a valid ref\" at the end of the bisection.\n-the autostart was causing issues, the following lines were working :\ngit bisect new HEAD\ngit bisect bad HEAD\ngit bisect good aGoodCommit\n\nThe hard review which we were thinking on was the issue of the maintaining\nof old/new and allow easy support of new \"tags\" like yes/no in the future.\nI tried to remove the maximum of bad/good and old/new which were hard wrote in\nthe code but I'm not completly satisfied. This patch is clearly a v1.\n\nWe're currently working on:\n\n* rebasing the history of the patch\n* git rev-list --bisect does not treat the revs/bisect/new and\nrevs/bisect/old-SHA1 files.\n* git bisect visualize seem to work partially: the tags are displayed\ncorrectly but the tree is not limited to the bisect section.\n* adding tests about old/new\n\nSome other problems that might also be considerred later :\n* change/add valid terms (e.g \"unfixed/fixed\" instead of \"old/new\")\n*\n* git bisect start [<new> [<old>...]] is not possible: the commits\nwill be treated as bad and good.\n\n\n Documentation/git-bisect.txt |  43 +++++++++++++-\n bisect.c                     |  83 ++++++++++++++++++++-------\n git-bisect.sh                | 132 ++++++++++++++++++++++++++++++++-----------\n t/t6030-bisect-porcelain.sh  |  40 +++++++++++++\n 4 files changed, 243 insertions(+), 55 deletions(-)\n mode change 100755 => 100644 git-bisect.sh\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 4cb52a7..12c7711 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -18,8 +18,8 @@ on the subcommand:\n \n  git bisect help\n  git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<paths>...]\n- git bisect bad [<rev>]\n- git bisect good [<rev>...]\n+ git bisect (bad|new) [<rev>]\n+ git bisect (good|old) [<rev>...]\n  git bisect skip [(<rev>|<range>)...]\n  git bisect reset [<commit>]\n  git bisect visualize\n@@ -104,6 +104,33 @@ For example, `git bisect reset HEAD` will leave you on the current\n bisection commit and avoid switching commits at all, while `git bisect\n reset bisect/bad` will check out the first bad revision.\n \n+\n+Alternative terms: bisect new and bisect old\n+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n+\n+If you are not at ease with the terms \"bad\" and \"good\", perhaps\n+because you are looking for the commit that introduced a fix, you can\n+alternatively use \"new\" and \"old\" instead.\n+But note that you cannot mix \"bad\" and good\" with \"new\" and \"old\".\n+\n+------------------------------------------------\n+git bisect new [<rev>]\n+------------------------------------------------\n+\n+Marks the commit as new, which means the version is fixed.\n+It is the equivalent of \"git bisect bad [<rev>]\".\n+\n+------------------------------------------------\n+git bisect old [<rev>...]\n+------------------------------------------------\n+\n+Marks the commit as old, which means the bug is present.\n+It is the equivalent of \"git bisect good [<rev>...]\".\n+\n+You must run `git bisect start` without commits as argument and run\n+`git bisect new <rev>`/`git bisect old <rev>...` after to add the\n+commits.\n+\n Bisect visualize\n ~~~~~~~~~~~~~~~~\n \n@@ -379,6 +406,18 @@ In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit\n has at least one parent whose reachable graph is fully traversable in the sense\n required by 'git pack objects'.\n \n+* Look for a fix instead of a regression in the code\n++\n+------------\n+$ git bisect start\n+$ git bisect new\t\t# Current version is fixed\n+$ git bisect old bugged_version\t# bugged_version was the last version\n+\t\t\t\t# known to be unfixed\n+------------\n++\n+The \"new\" commits are the fixed ones and the \"old\" commits are the unfixed ones.\n+At the end of the bisect session, you will have the first fixed commit.\n+\n \n SEE ALSO\n --------\ndiff --git a/bisect.c b/bisect.c\nindex 10f5e57..d6c19fd 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -21,6 +21,9 @@ static const char *argv_checkout[] = {\"checkout\", \"-q\", NULL, \"--\", NULL};\n static const char *argv_show_branch[] = {\"show-branch\", NULL, NULL};\n static const char *argv_update_ref[] = {\"update-ref\", \"--no-deref\", \"BISECT_HEAD\", NULL, NULL};\n \n+static const char *bisect_bad;\n+static const char *bisect_good;\n+\n /* Remember to update object flag allocation in object.h */\n #define COUNTED\t\t(1u<<16)\n \n@@ -403,10 +406,11 @@ struct commit_list *find_bisection(struct commit_list *list,\n static int register_ref(const char *refname, const unsigned char *sha1,\n \t\t\tint flags, void *cb_data)\n {\n-\tif (!strcmp(refname, \"bad\")) {\n+\tif (!strcmp(refname, bisect_bad)) {\n \t\tcurrent_bad_oid = xmalloc(sizeof(*current_bad_oid));\n \t\thashcpy(current_bad_oid->hash, sha1);\n-\t} else if (starts_with(refname, \"good-\")) {\n+\t} else if (starts_with(refname, \"good-\") ||\n+\t\t   starts_with(refname, \"old-\")) {\n \t\tsha1_array_append(&good_revs, sha1);\n \t} else if (starts_with(refname, \"skip-\")) {\n \t\tsha1_array_append(&skipped_revs, sha1);\n@@ -634,7 +638,7 @@ static void exit_if_skipped_commits(struct commit_list *tried,\n \t\treturn;\n \n \tprintf(\"There are only 'skip'ped commits left to test.\\n\"\n-\t       \"The first bad commit could be any of:\\n\");\n+\t       \"The first %s commit could be any of:\\n\", bisect_bad);\n \tprint_commit_list(tried, \"%s\\n\", \"%s\\n\");\n \tif (bad)\n \t\tprintf(\"%s\\n\", oid_to_hex(bad));\n@@ -732,18 +736,25 @@ static void handle_bad_merge_base(void)\n \tif (is_expected_rev(current_bad_oid)) {\n \t\tchar *bad_hex = oid_to_hex(current_bad_oid);\n \t\tchar *good_hex = join_sha1_array_hex(&good_revs, ' ');\n-\n-\t\tfprintf(stderr, \"The merge base %s is bad.\\n\"\n-\t\t\t\"This means the bug has been fixed \"\n-\t\t\t\"between %s and [%s].\\n\",\n-\t\t\tbad_hex, bad_hex, good_hex);\n-\n+\t\tif (!strcmp(bisect_bad,\"bad\")) {\n+\t\t\tfprintf(stderr, \"The merge base %s is bad.\\n\"\n+\t\t\t\t\"This means the bug has been fixed \"\n+\t\t\t\t\"between %s and [%s].\\n\",\n+\t\t\t\tbad_hex, bad_hex, good_hex);\n+\t\t} else {\n+\t\t\tfprintf(stderr, \"The merge base %s is new.\\n\"\n+\t\t\t\t\"The property has changed \"\n+\t\t\t\t\"between %s and [%s].\\n\",\n+\t\t\t\tbad_hex, bad_hex, good_hex);\n+\t\t}\n \t\texit(3);\n \t}\n \n-\tfprintf(stderr, \"Some good revs are not ancestor of the bad rev.\\n\"\n+\tfprintf(stderr, \"Some %s revs are not ancestor of the %s rev.\\n\"\n \t\t\"git bisect cannot work properly in this case.\\n\"\n-\t\t\"Maybe you mistake good and bad revs?\\n\");\n+\t\t\"Maybe you mistook %s and %s revs?\\n\",\n+\t\tbisect_good, bisect_bad, bisect_good,\n+\t\tbisect_bad);\n \texit(1);\n }\n \n@@ -755,19 +766,19 @@ static void handle_skipped_merge_base(const unsigned char *mb)\n \n \twarning(\"the merge base between %s and [%s] \"\n \t\t\"must be skipped.\\n\"\n-\t\t\"So we cannot be sure the first bad commit is \"\n+\t\t\"So we cannot be sure the first %s commit is \"\n \t\t\"between %s and %s.\\n\"\n \t\t\"We continue anyway.\",\n-\t\tbad_hex, good_hex, mb_hex, bad_hex);\n+\t\tbad_hex, good_hex, bisect_bad, mb_hex, bad_hex);\n \tfree(good_hex);\n }\n \n /*\n- * \"check_merge_bases\" checks that merge bases are not \"bad\".\n+ * \"check_merge_bases\" checks that merge bases are not \"bad\" (or \"new\").\n  *\n- * - If one is \"bad\", it means the user assumed something wrong\n+ * - If one is \"bad\" (or \"new\"), it means the user assumed something wrong\n  * and we must exit with a non 0 error code.\n- * - If one is \"good\", that's good, we have nothing to do.\n+ * - If one is \"good\" (or \"old\"), that's good, we have nothing to do.\n  * - If one is \"skipped\", we can't know but we should warn.\n  * - If we don't know, we should check it out and ask the user to test.\n  */\n@@ -839,7 +850,7 @@ static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)\n \tint fd;\n \n \tif (!current_bad_oid)\n-\t\tdie(\"a bad revision is needed\");\n+\t\tdie(\"a %s revision is needed\", bisect_bad);\n \n \t/* Check if file BISECT_ANCESTORS_OK exists. */\n \tif (!stat(filename, &st) && S_ISREG(st.st_mode))\n@@ -890,6 +901,31 @@ static void show_diff_tree(const char *prefix, struct commit *commit)\n }\n \n /*\n+ * The terms used for this bisect session are stocked in\n+ * BISECT_TERMS: it can be bad/good or new/old.\n+ * We read them and stock them to adapt the messages\n+ * accordingly. Default is bad/good.\n+ */\n+void read_bisect_terms(void)\n+{\n+\tstruct strbuf str = STRBUF_INIT;\n+\tconst char *filename = git_path(\"BISECT_TERMS\");\n+\tFILE *fp = fopen(filename, \"r\");\n+\n+\tif (!fp) {\n+\t\tbisect_bad = \"bad\";\n+\t\tbisect_good = \"good\";\n+\t} else {\n+\tstrbuf_getline(&str, fp, '\\n');\n+\tbisect_bad = strbuf_detach(&str, NULL);\n+\tstrbuf_getline(&str, fp, '\\n');\n+\tbisect_good = strbuf_detach(&str, NULL);\n+\t}\n+\tstrbuf_release(&str);\n+\tfclose(fp);\n+}\n+\n+/*\n  * We use the convention that exiting with an exit code 10 means that\n  * the bisection process finished successfully.\n  * In this case the calling shell script should exit 0.\n@@ -905,6 +941,7 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \tconst unsigned char *bisect_rev;\n \tchar bisect_rev_hex[GIT_SHA1_HEXSZ + 1];\n \n+\tread_bisect_terms();\n \tif (read_bisect_refs())\n \t\tdie(\"reading bisect refs failed\");\n \n@@ -926,8 +963,10 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \t\t */\n \t\texit_if_skipped_commits(tried, NULL);\n \n-\t\tprintf(\"%s was both good and bad\\n\",\n-\t\t       oid_to_hex(current_bad_oid));\n+\t\tprintf(\"%s was both %s and %s\\n\",\n+\t\t       oid_to_hex(current_bad_oid),\n+\t\t       bisect_good,\n+\t\t       bisect_bad);\n \t\texit(1);\n \t}\n \n@@ -942,7 +981,8 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \n \tif (!hashcmp(bisect_rev, current_bad_oid->hash)) {\n \t\texit_if_skipped_commits(tried, current_bad_oid);\n-\t\tprintf(\"%s is the first bad commit\\n\", bisect_rev_hex);\n+\t\tprintf(\"%s is the first %s commit\\n\", bisect_rev_hex,\n+\t\t\tbisect_bad);\n \t\tshow_diff_tree(prefix, revs.commits->item);\n \t\t/* This means the bisection process succeeded. */\n \t\texit(10);\n@@ -954,6 +994,9 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \t       \"(roughly %d step%s)\\n\", nr, (nr == 1 ? \"\" : \"s\"),\n \t       steps, (steps == 1 ? \"\" : \"s\"));\n \n+\tfree((char*)bisect_bad);\n+\tfree((char*)bisect_good);\n+\n \treturn bisect_checkout(bisect_rev_hex, no_checkout);\n }\n \ndiff --git a/git-bisect.sh b/git-bisect.sh\nold mode 100755\nnew mode 100644\nindex ae3fec2..109bd65\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -1,14 +1,16 @@\n #!/bin/sh\n \n-USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'\n+USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run|new|old]'\n LONG_USAGE='git bisect help\n \tprint this long help message.\n git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<pathspec>...]\n \treset bisect state and start bisection.\n-git bisect bad [<rev>]\n-\tmark <rev> a known-bad revision.\n-git bisect good [<rev>...]\n-\tmark <rev>... known-good revisions.\n+git bisect (bad|new) [<rev>]\n+\tmark <rev> a known-bad revision/\n+\t\ta revision after change in a given property.\n+git bisect (good|old) [<rev>...]\n+\tmark <rev>... known-good revisions/\n+\t\trevisions before change in a given property.\n git bisect skip [(<rev>|<range>)...]\n \tmark <rev>... untestable revisions.\n git bisect next\n@@ -32,6 +34,8 @@ OPTIONS_SPEC=\n \n _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'\n _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n+BISECT_BAD=\"bad\"\n+BISECT_GOOD=\"good\"\n \n bisect_head()\n {\n@@ -75,6 +79,7 @@ bisect_start() {\n \torig_args=$(git rev-parse --sq-quote \"$@\")\n \tbad_seen=0\n \teval=''\n+\tstart_bad_good=0\n \tif test \"z$(git rev-parse --is-bare-repository)\" != zfalse\n \tthen\n \t\tmode=--no-checkout\n@@ -99,6 +104,10 @@ bisect_start() {\n \t\t\t\tdie \"$(eval_gettext \"'\\$arg' does not appear to be a valid revision\")\"\n \t\t\t\tbreak\n \t\t\t}\n+\n+\t\t\t# by default, if the user didnt type --fix\n+\t\t\tstart_bad_good=1\n+\n \t\t\tcase $bad_seen in\n \t\t\t0) state='bad' ; bad_seen=1 ;;\n \t\t\t*) state='good' ;;\n@@ -170,6 +179,11 @@ bisect_start() {\n \t} &&\n \tgit rev-parse --sq-quote \"$@\" >\"$GIT_DIR/BISECT_NAMES\" &&\n \teval \"$eval true\" &&\n+\tif test $start_bad_good -eq 1 -a ! -s \"$GIT_DIR/BISECT_TERMS\"\n+\tthen\n+\t\techo \"$BISECT_BAD\" >\"$GIT_DIR/BISECT_TERMS\" &&\n+\t\techo \"$BISECT_GOOD\" >>\"$GIT_DIR/BISECT_TERMS\"\n+\tfi &&\n \techo \"git bisect start$orig_args\" >>\"$GIT_DIR/BISECT_LOG\" || exit\n \t#\n \t# Check if we can proceed to the next bisect state.\n@@ -184,9 +198,12 @@ bisect_write() {\n \trev=\"$2\"\n \tnolog=\"$3\"\n \tcase \"$state\" in\n-\t\tbad)\t\ttag=\"$state\" ;;\n-\t\tgood|skip)\ttag=\"$state\"-\"$rev\" ;;\n-\t\t*)\t\tdie \"$(eval_gettext \"Bad bisect_write argument: \\$state\")\" ;;\n+\t\t\"$BISECT_BAD\")\n+\t\t\ttag=\"$state\" ;;\n+\t\t\"$BISECT_GOOD\"|skip)\n+\t\t\ttag=\"$state\"-\"$rev\" ;;\n+\t\t*)\n+\t\t\tdie \"$(eval_gettext \"Bad bisect_write argument: \\$state\")\" ;;\n \tesac\n \tgit update-ref \"refs/bisect/$tag\" \"$rev\" || exit\n \techo \"# $state: $(git show-branch $rev)\" >>\"$GIT_DIR/BISECT_LOG\"\n@@ -227,15 +244,16 @@ bisect_skip() {\n bisect_state() {\n \tbisect_autostart\n \tstate=$1\n+\tcheck_and_set_terms $state\n \tcase \"$#,$state\" in\n \t0,*)\n \t\tdie \"$(gettext \"Please call 'bisect_state' with at least one argument.\")\" ;;\n-\t1,bad|1,good|1,skip)\n+\t1,\"$BISECT_BAD\"|1,\"$BISECT_GOOD\"|1,skip)\n \t\trev=$(git rev-parse --verify $(bisect_head)) ||\n \t\t\tdie \"$(gettext \"Bad rev input: $(bisect_head)\")\"\n \t\tbisect_write \"$state\" \"$rev\"\n \t\tcheck_expected_revs \"$rev\" ;;\n-\t2,bad|*,good|*,skip)\n+\t2,\"$BISECT_BAD\"|*,\"$BISECT_GOOD\"|*,skip)\n \t\tshift\n \t\thash_list=''\n \t\tfor rev in \"$@\"\n@@ -249,8 +267,8 @@ bisect_state() {\n \t\t\tbisect_write \"$state\" \"$rev\"\n \t\tdone\n \t\tcheck_expected_revs $hash_list ;;\n-\t*,bad)\n-\t\tdie \"$(gettext \"'git bisect bad' can take only one argument.\")\" ;;\n+\t*,\"$BISECT_BAD\")\n+\t\tdie \"$(gettext \"'git bisect $BISECT_BAD' can take only one argument.\")\" ;;\n \t*)\n \t\tusage ;;\n \tesac\n@@ -259,21 +277,21 @@ bisect_state() {\n \n bisect_next_check() {\n \tmissing_good= missing_bad=\n-\tgit show-ref -q --verify refs/bisect/bad || missing_bad=t\n-\ttest -n \"$(git for-each-ref \"refs/bisect/good-*\")\" || missing_good=t\n+\tgit show-ref -q --verify refs/bisect/$BISECT_BAD || missing_bad=t\n+\ttest -n \"$(git for-each-ref \"refs/bisect/$BISECT_GOOD-*\")\" || missing_good=t\n \n \tcase \"$missing_good,$missing_bad,$1\" in\n \t,,*)\n-\t\t: have both good and bad - ok\n+\t\t: have both $BISECT_GOOD and $BISECT_BAD - ok\n \t\t;;\n \t*,)\n \t\t# do not have both but not asked to fail - just report.\n \t\tfalse\n \t\t;;\n-\tt,,good)\n-\t\t# have bad but not good.  we could bisect although\n+\tt,,\"$BISECT_GOOD\")\n+\t\t# have bad (or new) but not good (or old).  we could bisect although\n \t\t# this is less optimum.\n-\t\tgettextln \"Warning: bisecting only with a bad commit.\" >&2\n+\t\tgettextln \"Warning: bisecting only with a $BISECT_BAD commit.\" >&2\n \t\tif test -t 0\n \t\tthen\n \t\t\t# TRANSLATORS: Make sure to include [Y] and [n] in your\n@@ -283,18 +301,18 @@ bisect_next_check() {\n \t\t\tread yesno\n \t\t\tcase \"$yesno\" in [Nn]*) exit 1 ;; esac\n \t\tfi\n-\t\t: bisect without good...\n+\t\t: bisect without $BISECT_GOOD...\n \t\t;;\n \t*)\n \n \t\tif test -s \"$GIT_DIR/BISECT_START\"\n \t\tthen\n-\t\t\tgettextln \"You need to give me at least one good and one bad revision.\n-(You can use \\\"git bisect bad\\\" and \\\"git bisect good\\\" for that.)\" >&2\n+\t\t\tgettextln \"You need to give me at least one $(bisect_voc bad) and one $(bisect_voc good) revision.\n+(You can use \\\"git bisect $(bisect_voc bad)\\\" and \\\"git bisect $(bisect_voc good)\\\" for that.)\" >&2\n \t\telse\n \t\t\tgettextln \"You need to start by \\\"git bisect start\\\".\n-You then need to give me at least one good and one bad revision.\n-(You can use \\\"git bisect bad\\\" and \\\"git bisect good\\\" for that.)\" >&2\n+You then need to give me at least one $(bisect_voc good) and one $(bisect_voc bad) revision.\n+(You can use \\\"git bisect $(bisect_voc bad)\\\" and \\\"git bisect $(bisect_voc good)\\\" for that.)\" >&2\n \t\tfi\n \t\texit 1 ;;\n \tesac\n@@ -307,7 +325,7 @@ bisect_auto_next() {\n bisect_next() {\n \tcase \"$#\" in 0) ;; *) usage ;; esac\n \tbisect_autostart\n-\tbisect_next_check good\n+\tbisect_next_check $BISECT_GOOD\n \n \t# Perform all bisection computation, display and checkout\n \tgit bisect--helper --next-all $(test -f \"$GIT_DIR/BISECT_HEAD\" && echo --no-checkout)\n@@ -316,15 +334,15 @@ bisect_next() {\n \t# Check if we should exit because bisection is finished\n \tif test $res -eq 10\n \tthen\n-\t\tbad_rev=$(git show-ref --hash --verify refs/bisect/bad)\n+\t\tbad_rev=$(git show-ref --hash --verify refs/bisect/$BISECT_BAD)\n \t\tbad_commit=$(git show-branch $bad_rev)\n \t\techo \"# first bad commit: $bad_commit\" >>\"$GIT_DIR/BISECT_LOG\"\n \t\texit 0\n \telif test $res -eq 2\n \tthen\n \t\techo \"# only skipped commits left to test\" >>\"$GIT_DIR/BISECT_LOG\"\n-\t\tgood_revs=$(git for-each-ref --format=\"%(objectname)\" \"refs/bisect/good-*\")\n-\t\tfor skipped in $(git rev-list refs/bisect/bad --not $good_revs)\n+\t\tgood_revs=$(git for-each-ref --format=\"%(objectname)\" \"refs/bisect/$BISECT_GOOD-*\")\n+\t\tfor skipped in $(git rev-list refs/bisect/$BISECT_BAD --not $good_revs)\n \t\tdo\n \t\t\tskipped_commit=$(git show-branch $skipped)\n \t\t\techo \"# possible first bad commit: $skipped_commit\" >>\"$GIT_DIR/BISECT_LOG\"\n@@ -397,6 +415,7 @@ bisect_clean_state() {\n \trm -f \"$GIT_DIR/BISECT_LOG\" &&\n \trm -f \"$GIT_DIR/BISECT_NAMES\" &&\n \trm -f \"$GIT_DIR/BISECT_RUN\" &&\n+\trm -f \"$GIT_DIR/BISECT_TERMS\" &&\n \t# Cleanup head-name if it got left by an old version of git-bisect\n \trm -f \"$GIT_DIR/head-name\" &&\n \tgit update-ref -d --no-deref BISECT_HEAD &&\n@@ -417,11 +436,13 @@ bisect_replay () {\n \t\t\trev=\"$command\"\n \t\t\tcommand=\"$bisect\"\n \t\tfi\n+\t\tget_terms\n+\t\tcheck_and_set_terms \"$command\"\n \t\tcase \"$command\" in\n \t\tstart)\n \t\t\tcmd=\"bisect_start $rev\"\n \t\t\teval \"$cmd\" ;;\n-\t\tgood|bad|skip)\n+\t\tgood|bad|skip|old|new)\n \t\t\tbisect_write \"$command\" \"$rev\" ;;\n \t\t*)\n \t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n@@ -455,9 +476,9 @@ exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n \t\t\tstate='skip'\n \t\telif [ $res -gt 0 ]\n \t\tthen\n-\t\t\tstate='bad'\n+\t\t\tstate=\"$BISECT_BAD\"\n \t\telse\n-\t\t\tstate='good'\n+\t\t\tstate=\"$BISECT_GOOD\"\n \t\tfi\n \n \t\t# We have to use a subshell because \"bisect_state\" can exit.\n@@ -466,7 +487,7 @@ exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n \n \t\tcat \"$GIT_DIR/BISECT_RUN\"\n \n-\t\tif sane_grep \"first bad commit could be any of\" \"$GIT_DIR/BISECT_RUN\" \\\n+\t\tif sane_grep \"first $BISECT_BAD commit could be any of\" \"$GIT_DIR/BISECT_RUN\" \\\n \t\t\t>/dev/null\n \t\tthen\n \t\t\tgettextln \"bisect run cannot continue any more\" >&2\n@@ -480,7 +501,7 @@ exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n \t\t\texit $res\n \t\tfi\n \n-\t\tif sane_grep \"is the first bad commit\" \"$GIT_DIR/BISECT_RUN\" >/dev/null\n+\t\tif sane_grep \"is the first $BISECT_BAD commit\" \"$GIT_DIR/BISECT_RUN\" >/dev/null\n \t\tthen\n \t\t\tgettextln \"bisect run success\"\n \t\t\texit 0;\n@@ -494,18 +515,63 @@ bisect_log () {\n \tcat \"$GIT_DIR/BISECT_LOG\"\n }\n \n+get_terms () {\n+\tif test -s \"$GIT_DIR/BISECT_TERMS\"\n+\tthen\n+\t\tBISECT_BAD=\"$(sed -n 1p \"$GIT_DIR/BISECT_TERMS\")\"\n+\t\tBISECT_GOOD=\"$(sed -n 2p \"$GIT_DIR/BISECT_TERMS\")\"\n+\tfi\n+}\n+\n+check_and_set_terms () {\n+\tcmd=\"$1\"\n+\tcase \"$cmd\" in\n+\tbad|good|new|old)\n+\t\tif test -s \"$GIT_DIR/BISECT_TERMS\" -a \"$cmd\" != \"$BISECT_BAD\" -a \"$cmd\" != \"$BISECT_GOOD\"\n+\t\tthen\n+\t\t\tdie \"$(eval_gettext \"Invalid command : you're currently in a \\$BISECT_BAD/\\$BISECT_GOOD bisect.\")\"\n+\t\tfi\n+\t\tcase \"$cmd\" in\n+\t\tbad|good)\n+\t\t\tif test ! -s \"$GIT_DIR/BISECT_TERMS\"\n+\t\t\tthen\n+\t\t\t\techo \"bad\" >\"$GIT_DIR/BISECT_TERMS\" &&\n+\t\t\t\techo \"good\" >>\"$GIT_DIR/BISECT_TERMS\"\n+\t\t\tfi\n+\t\t\tBISECT_BAD=\"bad\"\n+\t\t\tBISECT_GOOD=\"good\" ;;\n+\t\tnew|old)\n+\t\t\tif test ! -s \"$GIT_DIR/BISECT_TERMS\"\n+\t\t\tthen\n+\t\t\t\techo \"new\" >\"$GIT_DIR/BISECT_TERMS\" &&\n+\t\t\t\techo \"old\" >>\"$GIT_DIR/BISECT_TERMS\"\n+\t\t\tfi\n+\t\t\tBISECT_BAD=\"new\"\n+\t\t\tBISECT_GOOD=\"old\" ;;\n+\t\tesac ;;\n+\tesac\n+}\n+\n+bisect_voc () {\n+\tcase \"$1\" in\n+\tbad) echo \"bad|old\" ;;\n+\tgood) echo \"good|new\" ;;\n+\tesac\n+}\n+\n case \"$#\" in\n 0)\n \tusage ;;\n *)\n \tcmd=\"$1\"\n+\tget_terms\n \tshift\n \tcase \"$cmd\" in\n \thelp)\n \t\tgit bisect -h ;;\n \tstart)\n \t\tbisect_start \"$@\" ;;\n-\tbad|good)\n+\tbad|good|new|old)\n \t\tbisect_state \"$cmd\" \"$@\" ;;\n \tskip)\n \t\tbisect_skip \"$@\" ;;\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 06b4868..2163010 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -710,6 +710,8 @@ test_expect_success 'bisect: demonstrate identification of damage boundary' \"\n \t\ttest \\$rc = 0' &&\n \tcheck_same BROKEN_HASH6 bisect/bad &&\n \tgit bisect reset\n+\tgit checkout master\n+\n \"\n \n cat > expected.bisect-log <<EOF\n@@ -759,4 +761,42 @@ test_expect_success '\"git bisect bad HEAD\" behaves as \"git bisect bad\"' '\n \tgit bisect reset\n '\n \n+test_expect_success 'bisect starts with only one new' '\n+\tgit bisect reset &&\n+\tgit bisect start &&\n+\tgit bisect new $HASH4 &&\n+\tgit bisect next\n+'\n+test_expect_success 'bisect does not start with only one old' '\n+\tgit bisect reset &&\n+\tgit bisect start &&\n+\tgit bisect old $HASH1 || return 1\n+\ttest_must_fail git bisect next\n+\n+'\n+\n+test_expect_success 'bisect start with one new and old' '\n+\tgit bisect reset &&\n+\tgit bisect start &&\n+\tgit bisect old $HASH1 &&\n+\tgit bisect new $HASH4 &&\n+\tgit bisect new &&\n+\tgit bisect new >bisect_result &&\n+\tgrep \"$HASH2 is the first new commit\" bisect_result &&\n+\tgit bisect log > log_to_replay.txt &&\n+\tgit bisect reset\n+'\n+\n+test_expect_success 'bisect replay with old and new' '\n+\tgit bisect replay log_to_replay.txt > bisect_result &&\n+\tgrep \"$HASH2 is the first new commit\" bisect_result &&\n+\tgit bisect reset\n+'\n+\n+test_expect_success 'bisect cannot mix old/new and good/bad' '\n+\tgit bisect start &&\n+\tgit bisect bad $HASH4 &&\n+\ttest_must_fail git bisect old $HASH1\n+'\n+\n test_done\n-- \n2.4.1.173.gd47f443.dirty\n"},{"id":"262932","messageId":"1433404753-12039-2-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","threadId":"39520","inReplyTo":"1433404753-12039-1-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","subject":"[PATCH 2/3] bisect: use name_(bad|good) instead of bisect_(bad|good)","fromName":"Antoine Delaite","fromEmail":"antoine.delaite@ensimag.grenoble-inp.fr","sentAt":"2015-06-04T07:59:12Z","receivedAt":"2015-06-04T07:59:12Z","isPatch":true,"sender":{"key":"antoine.delaite@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12510393?v=4"},"body":"From: Christian Couder <chriscool@tuxfamily.org>\n\nSigned-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>\n---\n bisect.c | 35 +++++++++++++++++------------------\n 1 file changed, 17 insertions(+), 18 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex d6c19fd..68417bb 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -21,8 +21,8 @@ static const char *argv_checkout[] = {\"checkout\", \"-q\", NULL, \"--\", NULL};\n static const char *argv_show_branch[] = {\"show-branch\", NULL, NULL};\n static const char *argv_update_ref[] = {\"update-ref\", \"--no-deref\", \"BISECT_HEAD\", NULL, NULL};\n \n-static const char *bisect_bad;\n-static const char *bisect_good;\n+static const char *name_bad;\n+static const char *name_good;\n \n /* Remember to update object flag allocation in object.h */\n #define COUNTED\t\t(1u<<16)\n@@ -406,7 +406,7 @@ struct commit_list *find_bisection(struct commit_list *list,\n static int register_ref(const char *refname, const unsigned char *sha1,\n \t\t\tint flags, void *cb_data)\n {\n-\tif (!strcmp(refname, bisect_bad)) {\n+\tif (!strcmp(refname, name_bad)) {\n \t\tcurrent_bad_oid = xmalloc(sizeof(*current_bad_oid));\n \t\thashcpy(current_bad_oid->hash, sha1);\n \t} else if (starts_with(refname, \"good-\") ||\n@@ -638,7 +638,7 @@ static void exit_if_skipped_commits(struct commit_list *tried,\n \t\treturn;\n \n \tprintf(\"There are only 'skip'ped commits left to test.\\n\"\n-\t       \"The first %s commit could be any of:\\n\", bisect_bad);\n+\t       \"The first %s commit could be any of:\\n\", name_bad);\n \tprint_commit_list(tried, \"%s\\n\", \"%s\\n\");\n \tif (bad)\n \t\tprintf(\"%s\\n\", oid_to_hex(bad));\n@@ -736,7 +736,7 @@ static void handle_bad_merge_base(void)\n \tif (is_expected_rev(current_bad_oid)) {\n \t\tchar *bad_hex = oid_to_hex(current_bad_oid);\n \t\tchar *good_hex = join_sha1_array_hex(&good_revs, ' ');\n-\t\tif (!strcmp(bisect_bad,\"bad\")) {\n+\t\tif (!strcmp(name_bad,\"bad\")) {\n \t\t\tfprintf(stderr, \"The merge base %s is bad.\\n\"\n \t\t\t\t\"This means the bug has been fixed \"\n \t\t\t\t\"between %s and [%s].\\n\",\n@@ -753,8 +753,7 @@ static void handle_bad_merge_base(void)\n \tfprintf(stderr, \"Some %s revs are not ancestor of the %s rev.\\n\"\n \t\t\"git bisect cannot work properly in this case.\\n\"\n \t\t\"Maybe you mistook %s and %s revs?\\n\",\n-\t\tbisect_good, bisect_bad, bisect_good,\n-\t\tbisect_bad);\n+\t\tname_good, name_bad, name_good, name_bad);\n \texit(1);\n }\n \n@@ -769,7 +768,7 @@ static void handle_skipped_merge_base(const unsigned char *mb)\n \t\t\"So we cannot be sure the first %s commit is \"\n \t\t\"between %s and %s.\\n\"\n \t\t\"We continue anyway.\",\n-\t\tbad_hex, good_hex, bisect_bad, mb_hex, bad_hex);\n+\t\tbad_hex, good_hex, name_bad, mb_hex, bad_hex);\n \tfree(good_hex);\n }\n \n@@ -850,7 +849,7 @@ static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)\n \tint fd;\n \n \tif (!current_bad_oid)\n-\t\tdie(\"a %s revision is needed\", bisect_bad);\n+\t\tdie(\"a %s revision is needed\", name_bad);\n \n \t/* Check if file BISECT_ANCESTORS_OK exists. */\n \tif (!stat(filename, &st) && S_ISREG(st.st_mode))\n@@ -913,13 +912,13 @@ void read_bisect_terms(void)\n \tFILE *fp = fopen(filename, \"r\");\n \n \tif (!fp) {\n-\t\tbisect_bad = \"bad\";\n-\t\tbisect_good = \"good\";\n+\t\tname_bad = \"bad\";\n+\t\tname_good = \"good\";\n \t} else {\n \tstrbuf_getline(&str, fp, '\\n');\n-\tbisect_bad = strbuf_detach(&str, NULL);\n+\tname_bad = strbuf_detach(&str, NULL);\n \tstrbuf_getline(&str, fp, '\\n');\n-\tbisect_good = strbuf_detach(&str, NULL);\n+\tname_good = strbuf_detach(&str, NULL);\n \t}\n \tstrbuf_release(&str);\n \tfclose(fp);\n@@ -965,8 +964,8 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \n \t\tprintf(\"%s was both %s and %s\\n\",\n \t\t       oid_to_hex(current_bad_oid),\n-\t\t       bisect_good,\n-\t\t       bisect_bad);\n+\t\t       name_good,\n+\t\t       name_bad);\n \t\texit(1);\n \t}\n \n@@ -982,7 +981,7 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \tif (!hashcmp(bisect_rev, current_bad_oid->hash)) {\n \t\texit_if_skipped_commits(tried, current_bad_oid);\n \t\tprintf(\"%s is the first %s commit\\n\", bisect_rev_hex,\n-\t\t\tbisect_bad);\n+\t\t\tname_bad);\n \t\tshow_diff_tree(prefix, revs.commits->item);\n \t\t/* This means the bisection process succeeded. */\n \t\texit(10);\n@@ -994,8 +993,8 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \t       \"(roughly %d step%s)\\n\", nr, (nr == 1 ? \"\" : \"s\"),\n \t       steps, (steps == 1 ? \"\" : \"s\"));\n \n-\tfree((char*)bisect_bad);\n-\tfree((char*)bisect_good);\n+\tfree((char*)name_bad);\n+\tfree((char*)name_good);\n \n \treturn bisect_checkout(bisect_rev_hex, no_checkout);\n }\n-- \n2.4.1.173.gd47f443.dirty\n"},{"id":"262933","messageId":"1433404753-12039-3-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","threadId":"39520","inReplyTo":"1433404753-12039-1-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","subject":"[PATCH 3/3] bisect: fix indentation","fromName":"Antoine Delaite","fromEmail":"antoine.delaite@ensimag.grenoble-inp.fr","sentAt":"2015-06-04T07:59:13Z","receivedAt":"2015-06-04T07:59:13Z","isPatch":true,"sender":{"key":"antoine.delaite@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12510393?v=4"},"body":"From: Christian Couder <chriscool@tuxfamily.org>\n\nSigned-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>\n---\n bisect.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 68417bb..87a5f6d 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -915,10 +915,10 @@ void read_bisect_terms(void)\n \t\tname_bad = \"bad\";\n \t\tname_good = \"good\";\n \t} else {\n-\tstrbuf_getline(&str, fp, '\\n');\n-\tname_bad = strbuf_detach(&str, NULL);\n-\tstrbuf_getline(&str, fp, '\\n');\n-\tname_good = strbuf_detach(&str, NULL);\n+\t\tstrbuf_getline(&str, fp, '\\n');\n+\t\tname_bad = strbuf_detach(&str, NULL);\n+\t\tstrbuf_getline(&str, fp, '\\n');\n+\t\tname_good = strbuf_detach(&str, NULL);\n \t}\n \tstrbuf_release(&str);\n \tfclose(fp);\n-- \n2.4.1.173.gd47f443.dirty\n"},{"id":"262937","messageId":"vpqiob3yjeh.fsf@anie.imag.fr","threadId":"39520","inReplyTo":"1433404753-12039-1-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","subject":"Re: [WIP PATCH 1/3] git bisect old/new","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-04T08:20:22Z","receivedAt":"2015-06-04T08:20:22Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr> writes:\n\n> @@ -732,18 +736,25 @@ static void handle_bad_merge_base(void)\n>  \tif (is_expected_rev(current_bad_oid)) {\n>  \t\tchar *bad_hex = oid_to_hex(current_bad_oid);\n>  \t\tchar *good_hex = join_sha1_array_hex(&good_revs, ' ');\n> -\n> -\t\tfprintf(stderr, \"The merge base %s is bad.\\n\"\n> -\t\t\t\"This means the bug has been fixed \"\n> -\t\t\t\"between %s and [%s].\\n\",\n> -\t\t\tbad_hex, bad_hex, good_hex);\n> -\n> +\t\tif (!strcmp(bisect_bad,\"bad\")) {\n\nspace after comma.\n\n> @@ -954,6 +994,9 @@ int bisect_next_all(const char *prefix, int no_checkout)\n>  \t       \"(roughly %d step%s)\\n\", nr, (nr == 1 ? \"\" : \"s\"),\n>  \t       steps, (steps == 1 ? \"\" : \"s\"));\n>  \n> +\tfree((char*)bisect_bad);\n> +\tfree((char*)bisect_good);\n\nAlready noted last time I think: these must not be freed as they are\npointing to static strings.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"262939","messageId":"CAP8UFD3K8NFWgiuusi47Puy4q8JZPD7X9wOj7H7yq+NYui0tsQ@mail.gmail.com","threadId":"39520","inReplyTo":"1433404753-12039-1-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","subject":"Re: [WIP PATCH 1/3] git bisect old/new","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-06-04T08:29:13Z","receivedAt":"2015-06-04T08:29:13Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Jun 4, 2015 at 9:59 AM, Antoine Delaite\n<antoine.delaite@ensimag.grenoble-inp.fr> wrote:\n> From: Christian Couder <chriscool@tuxfamily.org>\n>\n> When not looking for a regression during a bisect but for a fix or a\n> change in another given property, it can be confusing to use 'good'\n> and 'bad'.\n>\n> This patch introduce `git bisect new` and `git bisect old` as an\n> alternative to 'bad' and good': the commits which have the most\n> recent version of the property must be marked as `new` and the ones\n> with the older version as `old`.\n>\n> The output will be the first commit after the change in the property.\n> During a new/old bisect session you cannot use bad/good commands and\n> vice-versa.\n>\n> `git bisect replay` works fine for old/new bisect sessions.\n>\n> Some commands are still not available for old/new:\n>\n>      * git bisect start [<new> [<old>...]] is not possible: the\n>        commits will be treated as bad and good.\n>      * git rev-list --bisect does not treat the revs/bisect/new and\n>        revs/bisect/old-SHA1 files.\n>      * git bisect visualize seem to work partially: the tags are\n>        displayed correctly but the tree is not limited to the bisect\n>        section.\n>\n> Related discussions:\n>\n>         - http://thread.gmane.org/gmane.comp.version-control.git/86063\n>                 introduced bisect fix unfixed to find fix.\n>         - http://thread.gmane.org/gmane.comp.version-control.git/182398\n>                 discussion around bisect yes/no or old/new.\n>         - http://thread.gmane.org/gmane.comp.version-control.git/199758\n>                 last discussion and reviews\n>\n> Signed-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>\n> Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>\n> Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>\n> Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>\n> Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>\n> Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>\n> Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>\n> ---\n> We took account of most of the \"easy\" reviews that were discuted two years ago.\n> We hope we didn't missed any.\n> http://thread.gmane.org/gmane.comp.version-control.git/199758\n>\n> We corrected various issues that were not reported:\n> -one that caused a \"fatal ... not a valid ref\" at the end of the bisection.\n> -the autostart was causing issues, the following lines were working :\n> git bisect new HEAD\n> git bisect bad HEAD\n> git bisect good aGoodCommit\n>\n> The hard review which we were thinking on was the issue of the maintaining\n> of old/new and allow easy support of new \"tags\" like yes/no in the future.\n> I tried to remove the maximum of bad/good and old/new which were hard wrote in\n> the code but I'm not completly satisfied. This patch is clearly a v1.\n\nThanks for working on this. Here are some suggestions that I should\nprobably have told you before, but didn't:\n\n- Take ownership of all the patches.\n- Patch 3/3 renames some variables in bisect.c, do the same thing in\ngit-bisect.sh for consistency.\n- Squash all the patches together.\n- Try to find a way to break down the resulting patch into a logical\npatch series which adds tests at each logical step. This might be\ndifficult. You might want to add features to git bisect--helper first\nfor example, then test those features, then add features to git bisect\nand then test those features.\n\nBest,\nChristian.\n\n> We're currently working on:\n>\n> * rebasing the history of the patch\n> * git rev-list --bisect does not treat the revs/bisect/new and\n> revs/bisect/old-SHA1 files.\n> * git bisect visualize seem to work partially: the tags are displayed\n> correctly but the tree is not limited to the bisect section.\n> * adding tests about old/new\n>\n> Some other problems that might also be considerred later :\n> * change/add valid terms (e.g \"unfixed/fixed\" instead of \"old/new\")\n> *\n> * git bisect start [<new> [<old>...]] is not possible: the commits\n> will be treated as bad and good.\n>\n"},{"id":"262972","messageId":"xmqqh9qnuz6z.fsf@gitster.dls.corp.google.com","threadId":"39520","inReplyTo":"1433404753-12039-1-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","subject":"Re: [WIP PATCH 1/3] git bisect old/new","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-04T18:05:08Z","receivedAt":"2015-06-04T18:05:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr> writes:\n\n> From: Christian Couder <chriscool@tuxfamily.org>\n>\n> When not looking for a regression during a bisect but for a fix or a\n> change in another given property, it can be confusing to use 'good'\n> and 'bad'.\n>\n> This patch introduce `git bisect new` and `git bisect old` as an\n> alternative to 'bad' and good': the commits which have the most\n> recent version of the property must be marked as `new` and the ones\n> with the older version as `old`.\n\nThe phrase \"the most recent version of\" invites an unnecessary\nconfusion.  \"git bisect\" is a tool to find a single bit flip and if\nthe behaviour changed twice in the history from A to B to C, you\ncannot look for the transition between A to B or B to C.  You need\nto say \"I classify behaviour B and C to be both good (new) and I\nknow it used to have behaviour A which was bad (old)\".\n\n    You can mark the commits with the \"new\" property as \"new\", and\n    the \"old\" property as \"old\"; \"good\" being \"new\" and \"bad\" being\n    \"old\" becomes a mere special case of this.  The \"new\" property\n    in the traditional bisection is \"commit has the bug we are\n    hunting\", while \"old\" is \"commit lacks that bug\".\n\n> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\n> index 4cb52a7..12c7711 100644\n> --- a/Documentation/git-bisect.txt\n> +++ b/Documentation/git-bisect.txt\n> ...\n> @@ -104,6 +104,33 @@ For example, `git bisect reset HEAD` will leave you on the current\n>  bisection commit and avoid switching commits at all, while `git bisect\n>  reset bisect/bad` will check out the first bad revision.\n>  \n> +\n> +Alternative terms: bisect new and bisect old\n> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> +\n> +If you are not at ease with the terms \"bad\" and \"good\", perhaps\n> +because you are looking for the commit that introduced a fix, you can\n> +alternatively use \"new\" and \"old\" instead.\n> +But note that you cannot mix \"bad\" and good\" with \"new\" and \"old\".\n\nThere is a dq missing somewhere on this line.\n\n> +\n> +------------------------------------------------\n> +git bisect new [<rev>]\n> +------------------------------------------------\n> +\n> +Marks the commit as new, which means the version is fixed.\n\nI am not sure \", which means the version is fixed.\" is a good idea.\n\nThe whole point of introducing \"old\" vs \"new\", the reason why we may\nwant to do this patch series, is to reduce confusion by removing the\nvalue judgement associated with \"good\" vs \"bad\".  That is what made\nthe bisect confusing when the old ones were bad and new ones were\ngood.  Saying \"fixed\" implicitly contrasts it with \"broken\", and\nintroduces the value judgement on the states again.\n\n\tSide note: yes, if the reader is careful, \"fixed\" needs to\n\thave previous states to be \"broken\", so in that sense,\n\t\"fixed\" can be read about time-sequence and not about value\n\tjudgement.  But let's try to be understandable by even\n\tcasual readers.\n\nWe would instead want to emphasize that what we are judging is not\nvalues but time sequence.  You would want to say \"It used to be X\nbut it no longer is X\", without saying if X is a good thing or not,\ne.g.\n\n\tMarks the commit as new, e.g. \"the bug is no longer there\",\n        if you are looking for a commit that fixed a bug, or \"the\n        feature that used to work is now broken at this point\", if\n        you are looking for a commit that introduced a bug.\n\n> +------------------------------------------------\n> +git bisect old [<rev>...]\n> +------------------------------------------------\n> +\n> +Marks the commit as old, which means the bug is present.\n\nThis is much worse than the previous one. Unless you force the\nreader to _assume_ that you are hunting for a fix, \"bug is present\"\ncan mean \"bug is still present\", or \"bug was introduced somewhere\nalong the line and now it is there\".  And a casual mention of\n\"perhaps because you are looking for a fix\" at the beginning of the\nsection is not strong enough hint that these examples are _only_\nabout \"hunting for bugfix\".\n\nI'd stop here for now, but a casual skimming told me that the same\nissue exists throughout the remainder of the doc.\n\nThanks.\n"}]}