{"thread":{"id":"28526","subject":"RFC: reverse bisect","startedAt":"2011-09-29T14:20:29Z","lastAt":"2011-10-12T20:14:10Z","messageCount":17,"participants":["Michal Vyskocil","Sverre Rabbelier","Johannes Sixt","Jeff King","Frans Klaver","Junio C Hamano","Christian Couder","Andrew Ardill"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"176486","messageId":"20110929142027.GA4936@zelva.suse.cz","threadId":"28526","inReplyTo":null,"subject":"RFC: reverse bisect","fromName":"Michal Vyskocil","fromEmail":"mvyskocil@suse.cz","sentAt":"2011-09-29T14:20:29Z","receivedAt":"2011-09-29T14:20:29Z","isPatch":false,"sender":{"key":"mvyskocil@suse.cz","avatar":null},"body":"Hi all,\n\nFollowing proposed patch try to implement reverse mode for git bisect.\n\nThe git bisect command is written in the regression-finding in mind. IOW\nit expects the good commit is older than the later one, which caused a\nregression.\n\nHowever common usage for (at least) package maintainer is not to find a\nregression and fix it. The main task it to identify a bugfix!. In this\ncase git bisect is still helpfull as it reduces a time a lot, but user\nneeds to exchange the good<->bad in his mind, which is confusing and in\ncase there are delays in the work, it's trivial to forgot that I have to\ntype git bisect good, when I'm in the bad revision.\n\nThis simple patch try to address the problem of poor package\nmaintainer's brain and introduces --reverse argument for the git bisect\nstart command.\n\nIn this mode, bisect internally exchange the behavior of good/bad\nitself, so there's no need to do it manually. I did some basic testing\nand\n\ngit bisect start --reverse HEAD~999 HEAD\ngit bisect good/bad/skip/run\n\nreally works well, allowing user to identify a first good commit instead\nof the first bad one. I did not test other commands like visualize or\nreplay.\n\nWhat do you think about it? Do you see other problems I'm not aware of?\n---\n bisect.c      |    2 +-\n git-bisect.sh |   49 ++++++++++++++++++++++++++++++++++++++++++++-----\n 2 files changed, 45 insertions(+), 6 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex c7b7d79..33aaeaa 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -768,7 +768,7 @@ static void handle_bad_merge_base(void)\n \n \tfprintf(stderr, \"Some good revs are not ancestor of the bad 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\"Try --reverse to switch the bisect logic.\\n\");\n \texit(1);\n }\n \ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 2524060..5c95f25 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -33,6 +33,25 @@ OPTIONS_SPEC=\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 \n+bisect_reverse_mode() {\n+\ttest -f \"$GIT_DIR/BISECT_REVERSE\"\n+}\n+\n+bisect_reverse_state() {\n+\n+\tif bisect_reverse_mode; then\n+\t\tif test \"$1\" = \"good\"; then\n+\t\t\techo \"bad\"\n+\t\t\treturn 0\n+\t\telif test \"$1\" = \"bad\"; then\n+\t\t\techo \"good\"\n+\t\t\treturn 0\n+\t\tfi\n+\tfi\n+\n+\techo $1\n+}\n+\n bisect_head()\n {\n \tif test -f \"$GIT_DIR/BISECT_HEAD\"\n@@ -69,6 +88,11 @@ bisect_start() {\n \t# Check for one bad and then some good revisions.\n \t#\n \thas_double_dash=0\n+\t#\n+\t# Exchange the internal meainng of good/bad allowing bisect to find\n+\t# a commit fixing a bug, not \"only\" the one causes a regression\n+\t#\n+\treverse_mode=1\n \tfor arg; do\n \t\tcase \"$arg\" in --) has_double_dash=1; break ;; esac\n \tdone\n@@ -91,6 +115,9 @@ bisect_start() {\n \t\t--no-checkout)\n \t\t\tmode=--no-checkout\n \t\t\tshift ;;\n+\t\t--reverse)\n+\t\t\treverse_mode=0\n+\t\t\tshift ;;\n \t\t--*)\n \t\t\tdie \"$(eval_gettext \"unrecognised option: '\\$arg'\")\" ;;\n \t\t*)\n@@ -99,10 +126,17 @@ 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-\t\t\tcase $bad_seen in\n-\t\t\t0) state='bad' ; bad_seen=1 ;;\n-\t\t\t*) state='good' ;;\n-\t\t\tesac\n+\t\t\tif test $reverse_mode -ne 0; then\n+\t\t\t\tcase $bad_seen in\n+\t\t\t\t0) state='bad' ; bad_seen=1 ;;\n+\t\t\t\t*) state='good' ;;\n+\t\t\t\tesac\n+\t\t\telse\n+\t\t\t\tcase $bad_seen in\n+\t\t\t\t0) state='good' ; bad_seen=1 ;;\n+\t\t\t\t*) state='bad' ;;\n+\t\t\t\tesac\n+\t\t\tfi\n \t\t\teval=\"$eval bisect_write '$state' '$rev' 'nolog' &&\"\n \t\t\tshift\n \t\t\t;;\n@@ -170,6 +204,9 @@ bisect_start() {\n \tgit rev-parse --sq-quote \"$@\" >\"$GIT_DIR/BISECT_NAMES\" &&\n \teval \"$eval true\" &&\n \techo \"git bisect start$orig_args\" >>\"$GIT_DIR/BISECT_LOG\" || exit\n+\tif test $reverse_mode -eq 0; then\n+\t\t/bin/touch \"$GIT_DIR/BISECT_REVERSE\" || exit\n+\tfi\n \t#\n \t# Check if we can proceed to the next bisect state.\n \t#\n@@ -225,7 +262,7 @@ bisect_skip() {\n \n bisect_state() {\n \tbisect_autostart\n-\tstate=$1\n+\tstate=$(bisect_reverse_state $1)\n \tcase \"$#,$state\" in\n \t0,*)\n \t\tdie \"$(gettext \"Please call 'bisect_state' with at least one argument.\")\" ;;\n@@ -377,6 +414,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_REVERSE\" &&\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@@ -402,6 +440,7 @@ bisect_replay () {\n \t\t\tcmd=\"bisect_start $rev\"\n \t\t\teval \"$cmd\" ;;\n \t\tgood|bad|skip)\n+\t\t\tcommand=$(bisect_reverse_state $1)\n \t\t\tbisect_write \"$command\" \"$rev\" ;;\n \t\t*)\n \t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n-- \n1.7.6.3\n\n"},{"id":"176489","messageId":"CAGdFq_gSD6SyoLgPOuQcBu9bwpK=VN18ezambN1HHD_AtmFzLA@mail.gmail.com","threadId":"28526","inReplyTo":"20110929142027.GA4936@zelva.suse.cz","subject":"Re: RFC: reverse bisect","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-09-29T14:42:32Z","receivedAt":"2011-09-29T14:42:32Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Sep 29, 2011 at 16:20, Michal Vyskocil <mvyskocil@suse.cz> wrote:\n>        fprintf(stderr, \"Some good revs are not ancestor of the bad rev.\\n\"\n>                \"git bisect cannot work properly in this case.\\n\"\n> -               \"Maybe you mistake good and bad revs?\\n\");\n> +               \"Try --reverse to switch the bisect logic.\\n\");\n\nHeh, glad to see the \"Maybe you mistake\" phrasing removed if nothing else :P.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"176493","messageId":"4E849C5B.7050201@kdbg.org","threadId":"28526","inReplyTo":"20110929142027.GA4936@zelva.suse.cz","subject":"Re: RFC: reverse bisect","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2011-09-29T16:27:07Z","receivedAt":"2011-09-29T16:27:07Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 29.09.2011 16:20, schrieb Michal Vyskocil:\n> git bisect start --reverse HEAD~999 HEAD\n\nWith the regular meaning of the start subcommand, the revs given are\nordered: bad good good...\n\nWith the reversed meaning, this would have to become: good bad bad...\n\nThis would have to be mentioned clearly in the documentation.\n\n> git bisect good/bad/skip/run\n\nLast time this came up on the list I suggested to add the following\ncommands:\n\n   git bisect regression  # a synonym for git bisect start\n   git bisect improvement # your --reverse\n\n-- Hannes\n"},{"id":"176552","messageId":"20110930040924.GA28724@sigill.intra.peff.net","threadId":"28526","inReplyTo":"4E849C5B.7050201@kdbg.org","subject":"Re: RFC: reverse bisect","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-30T04:09:24Z","receivedAt":"2011-09-30T04:09:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 29, 2011 at 06:27:07PM +0200, Johannes Sixt wrote:\n\n> > git bisect good/bad/skip/run\n> \n> Last time this came up on the list I suggested to add the following\n> commands:\n> \n>    git bisect regression  # a synonym for git bisect start\n>    git bisect improvement # your --reverse\n\nThat makes some sense to me. But I do wonder if you could simply get rid\nof the connotations of \"good\" and \"bad\" entirely, by thinking of it as\nsimply looking for a commit that introduced some property. Like:\n\n  # find a bug\n  git bisect start\n  git bisect yes ;# has the bug\n  git bisect no ;# does not have the bug\n  git bisect skip ;# no idea\n\n  # find a feature being implemented\n  git bisect start\n  git bisect yes ;# has the feature\n  git bisect no ;# does not have the feature\n  git bisect skip ;# no idea\n\nIOW, I feel like we are having to handle this weird negation only\nbecause we have assigned a value judgement to the tests. That instead of\nsaying \"yes, we have this bug\", we say \"bad\", which only makes sense if\nyou are looking for a bad thing.\n\nYou can still produce a negation in your mind, of course, by asking\n\"when did this property go away\".  But that is usually about a bug being\nfixed, so the right answer is generally not a set of command line\noptions, but to stop asking \"when did bug X go away\", and instead ask\n\"when did the fix for bug X appear\".\n\nOne catch is that the run command assumes a successful exit is \"good\",\nand anything else is \"bad\". Which makes:\n\n  git bisect run make test\n\ngood for finding regressions, but is a little counterintuitive for the\nyes/no thing (a successful exit means \"no\").\n\n-Peff\n"},{"id":"176553","messageId":"op.v2lwa4rp0aolir@keputer.lokaal","threadId":"28526","inReplyTo":"20110930040924.GA28724@sigill.intra.peff.net","subject":"Re: RFC: reverse bisect","fromName":"Frans Klaver","fromEmail":"fransklaver@gmail.com","sentAt":"2011-09-30T05:31:41Z","receivedAt":"2011-09-30T05:31:41Z","isPatch":false,"sender":{"key":"fransklaver@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1876483?v=4"},"body":"On Fri, 30 Sep 2011 06:09:24 +0200, Jeff King <peff@peff.net> wrote:\n\n> One catch is that the run command assumes a successful exit is \"good\",\n> and anything else is \"bad\". Which makes:\n>\n>   git bisect run make test\n>\n> good for finding regressions, but is a little counterintuitive for the\n> yes/no thing (a successful exit means \"no\").\n\nThen you would require a script that inverts the result, no? From my\npoint of view it's either that or add an option telling bisect run how\nto interpret the results. In the latter case you could still consider\nadding the regression/improvement qualification to bisect start. It might\nhelp getting the mind set right.\n\nFrans\n"},{"id":"176558","messageId":"20110930082914.GD27152@zelva.suse.cz","threadId":"28526","inReplyTo":"4E849C5B.7050201@kdbg.org","subject":"Re: RFC: reverse bisect","fromName":"Michal Vyskocil","fromEmail":"mvyskocil@suse.cz","sentAt":"2011-09-30T08:29:14Z","receivedAt":"2011-09-30T08:29:14Z","isPatch":false,"sender":{"key":"mvyskocil@suse.cz","avatar":null},"body":"On Thu, Sep 29, 2011 at 06:27:07PM +0200, Johannes Sixt wrote:\n> Am 29.09.2011 16:20, schrieb Michal Vyskocil:\n> > git bisect start --reverse HEAD~999 HEAD\n> \n> With the regular meaning of the start subcommand, the revs given are\n> ordered: bad good good...\n> \n> With the reversed meaning, this would have to become: good bad bad...\n> \n> This would have to be mentioned clearly in the documentation.\n> \n> > git bisect good/bad/skip/run\n> \n> Last time this came up on the list I suggested to add the following\n> commands:\n> \n>    git bisect regression  # a synonym for git bisect start\n>    git bisect improvement # your --reverse\n\nGood point! As you mentioned, the --switch already reverse the meanings\nof arguments as you mentioned. Using a new command will be less\nconfusing for users.\n\nMichal Vyskocil\n"},{"id":"176568","messageId":"20110930114220.GA742@zelva.suse.cz","threadId":"28526","inReplyTo":"20110929142027.GA4936@zelva.suse.cz","subject":"[RFC/PATCH]: reverse bisect v 2.0","fromName":"Michal Vyskocil","fromEmail":"mvyskocil@suse.cz","sentAt":"2011-09-30T11:42:21Z","receivedAt":"2011-09-30T11:42:21Z","isPatch":true,"sender":{"key":"mvyskocil@suse.cz","avatar":null},"body":"The bugfix command works like the previous git bisect start --reverse.\nIt switch the meaning of good(s) and bad from the default regression\nsearch approach to the bugfix one.\n\nI don't like adding more new subcommands into bisect, so I decided to\nnot add ideas I have found on this mailinglist, like 'git bisect\nregression' or 'yes', 'no', 'fixed', 'unfixed' or whatever.\n\nThe git bisect start/bugfix good/bad/skip log replay and vizualize were\ntested (however on simple linear example).\n\nThe missing points:\n * git-bisect--helper has the \"bad\" hardcoded, so the commit fixing a\n   bug is reffered as a bad one\n * in git bisect vizualize, the good commit is shown under refs/bad in\n   gitk (however that's the same problem if user reverse the usage of\n   good/bad itself).\n * documentation and tests of course\n\nRegards\nMichal Vyskocil\n---\n bisect.c      |    2 +-\n git-bisect.sh |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++------\n 2 files changed, 60 insertions(+), 8 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex c7b7d79..2eb34db 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -768,7 +768,7 @@ static void handle_bad_merge_base(void)\n \n \tfprintf(stderr, \"Some good revs are not ancestor of the bad 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\"Try git bisect bugfix to switch the default bisect logic.\\n\");\n \texit(1);\n }\n \ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 2524060..6959cf8 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -4,7 +4,9 @@ USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'\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+\treset bisect state and start bisection to find a regression.\n+git bisect bugfix [--no-checkout] [<good> [<bad>...]] [--] [<pathspec>...]\n+\treset bisect state and start bisection to find a bugfix.\n git bisect bad [<rev>]\n \tmark <rev> a known-bad revision.\n git bisect good [<rev>...]\n@@ -33,6 +35,29 @@ OPTIONS_SPEC=\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 \n+setup_bugfix_mode() {\n+\t/bin/touch \"$GIT_DIR/BISECT_BUGFIX\"\n+}\n+\n+bisect_bugfix_mode() {\n+\ttest -f \"$GIT_DIR/BISECT_BUGFIX\"\n+}\n+\n+bisect_check_state() {\n+\n+\tif bisect_bugfix_mode; then\n+\t\tif test \"$1\" = \"good\"; then\n+\t\t\techo \"bad\"\n+\t\t\treturn 0\n+\t\telif test \"$1\" = \"bad\"; then\n+\t\t\techo \"good\"\n+\t\t\treturn 0\n+\t\tfi\n+\tfi\n+\n+\techo $1\n+}\n+\n bisect_head()\n {\n \tif test -f \"$GIT_DIR/BISECT_HEAD\"\n@@ -69,6 +94,15 @@ bisect_start() {\n \t# Check for one bad and then some good revisions.\n \t#\n \thas_double_dash=0\n+\t#\n+\t# Exchange the internal meaning of good/bad allowing bisect to find\n+\t# a commit fixing a bug, not \"only\" the one causes a regression\n+\t#\n+\tcmd=\"start\"\n+\tif test -n \"$1\" && test \"$1\" = \"bugfix\"; then\n+\t\tcmd=\"bugfix\"\n+\t\tshift 1\n+\tfi\n \tfor arg; do\n \t\tcase \"$arg\" in --) has_double_dash=1; break ;; esac\n \tdone\n@@ -99,10 +133,17 @@ 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-\t\t\tcase $bad_seen in\n-\t\t\t0) state='bad' ; bad_seen=1 ;;\n-\t\t\t*) state='good' ;;\n-\t\t\tesac\n+\t\t\t#if test $cmd = \"bisect\"; then\n+\t\t\t\tcase $bad_seen in\n+\t\t\t\t0) state='bad' ; bad_seen=1 ;;\n+\t\t\t\t*) state='good' ;;\n+\t\t\t\tesac\n+\t\t\t#else\n+\t\t\t#\tcase $bad_seen in\n+\t\t\t#\t0) state='good' ; bad_seen=1 ;;\n+\t\t\t#\t*) state='bad' ;;\n+\t\t\t#\tesac\n+\t\t\t#fi\n \t\t\teval=\"$eval bisect_write '$state' '$rev' 'nolog' &&\"\n \t\t\tshift\n \t\t\t;;\n@@ -169,7 +210,10 @@ bisect_start() {\n \t} &&\n \tgit rev-parse --sq-quote \"$@\" >\"$GIT_DIR/BISECT_NAMES\" &&\n \teval \"$eval true\" &&\n-\techo \"git bisect start$orig_args\" >>\"$GIT_DIR/BISECT_LOG\" || exit\n+\techo \"git bisect $cmd$orig_args\" >>\"$GIT_DIR/BISECT_LOG\" || exit\n+\tif test $cmd = \"bugfix\"; then\n+\t\tsetup_bugfix_mode || exit\n+\tfi\n \t#\n \t# Check if we can proceed to the next bisect state.\n \t#\n@@ -225,7 +269,7 @@ bisect_skip() {\n \n bisect_state() {\n \tbisect_autostart\n-\tstate=$1\n+\tstate=$(bisect_check_state $1)\n \tcase \"$#,$state\" in\n \t0,*)\n \t\tdie \"$(gettext \"Please call 'bisect_state' with at least one argument.\")\" ;;\n@@ -377,6 +421,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_BUGFIX\" &&\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@@ -401,7 +446,12 @@ bisect_replay () {\n \t\tstart)\n \t\t\tcmd=\"bisect_start $rev\"\n \t\t\teval \"$cmd\" ;;\n+\t\tbugfix)\n+\t\t\tcmd=\"bisect_start 'bugfix' $rev\"\n+\t\t\tsetup_bugfix_mode || exit\n+\t\t\teval \"$cmd\" ;;\n \t\tgood|bad|skip)\n+\t\t\tcommand=$(bisect_check_state $1)\n \t\t\tbisect_write \"$command\" \"$rev\" ;;\n \t\t*)\n \t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n@@ -485,6 +535,8 @@ case \"$#\" in\n \t\tgit bisect -h ;;\n \tstart)\n \t\tbisect_start \"$@\" ;;\n+\tbugfix)\n+\t\tbisect_start \"bugfix\" \"$@\" ;;\n \tbad|good)\n \t\tbisect_state \"$cmd\" \"$@\" ;;\n \tskip)\n-- \n1.7.6.3\n\n"},{"id":"176583","messageId":"7v62k9q4oq.fsf@alter.siamese.dyndns.org","threadId":"28526","inReplyTo":"20110930114220.GA742@zelva.suse.cz","subject":"Re: [RFC/PATCH]: reverse bisect v 2.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-30T18:13:25Z","receivedAt":"2011-09-30T18:13:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"[administrivia: added people involved in the main discussion thread back\non CC line, and also added CCouder who seems to be fond of the command]\n\nMichal Vyskocil <mvyskocil@suse.cz> writes:\n\n> The bugfix command works like the previous git bisect start --reverse.\n\nDoes any released version of git have \"bisect start --reverse\"?\n\n$ git bisect start --reverse\nunrecognised option: '--reverse'\n\nLet's suppose that we had a \"git frotz\" Porcelain subcommand, that used to\nsay \"xyzzy\" when all is well, but says \"nitfol\" these days. I released a\n(bad) script that parses the output from the subcommand, and want to say\nsomething like \"This script only works with Git version after X\", and to\nfind out the value of X, I need to bisect the history to find when \"frotz\"\nstarted to say \"nitfol\".\n\nI am not trying to find a commit that introduced a bug/regression to cause\nthe recent \"frotz\" to say \"nitfol\". Neither am I trying to find a fix that\ncorrected the earlier bogus output \"xyzzy\". No value judgement is involved\nin this scenario.\n\nI wonder if something along the following line would make the usage more\npleasant and self-explanatory:\n\n    $ git bisect start --used-to='git frotz says xyzzy' v0.99 master\n    Bisecting: 171 revisions left to test after this (roughly 8 steps)\n    $ ... build and then test ...\n    $ git bisect tested\n    You are trying to check: git frotz says xyzzy\n    Does the result satisify what you are trying to find [y/n]? yes\n\nSaying 'yes' would be like saying 'good' and 'no' would be like saying\n'bad' here.\n\nWhen trying to find regression, you would say:\n\n    $ git bisect start --used-to='it works' v0.99 master\n\nand you say 'yes' if it works (equivalent to 'good'), and 'no' if it does\nnot (equivalent to 'bad').\n\nWhen trying to find a fix, you would say:\n\n    $ git bisect start --used-to='checkout $tree $path clobbers $path' v0.99 master\n\nand you say 'yes' if it is still broken at the tested version (equivalent\nto your reversed 'good'), and 'no' if the tested version contains a fix\n(equivalent to your reversed 'bad').\n\nI am not married to the name of the option, but --used-to='...' felt more\nor less self-explanatory name to signal users what to describe in that\ntext. The condition \"You are trying to check:\" cue wants to remind the\nuser is \"we used to do *this* but now we don't\", and the command is trying\nto see when it changed.\n\nThoughts?\n"},{"id":"176732","messageId":"20111003104112.GE16078@sigill.intra.peff.net","threadId":"28526","inReplyTo":"7v62k9q4oq.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH]: reverse bisect v 2.0","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-03T10:41:12Z","receivedAt":"2011-10-03T10:41:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 30, 2011 at 11:13:25AM -0700, Junio C Hamano wrote:\n\n> I wonder if something along the following line would make the usage more\n> pleasant and self-explanatory:\n> \n>     $ git bisect start --used-to='git frotz says xyzzy' v0.99 master\n>     Bisecting: 171 revisions left to test after this (roughly 8 steps)\n>     $ ... build and then test ...\n>     $ git bisect tested\n>     You are trying to check: git frotz says xyzzy\n>     Does the result satisify what you are trying to find [y/n]? yes\n\nI like this idea a lot. My \"yes/no\" thing was a \"if I were designing\nbisect from scratch today...\" suggestion, but having something like\n--used-to makes it a natural addition to the regular good/bad interface.\nAnd I really like the prompt to help people remember what it is they're\ndeclaring each time.\n\nHowever, --used-to feels a bit backwards to me. I think of it as\n\"--has-property\" or something similar. That is, you are looking for when\nsomething appeared (be it a bug, a feature, or whatever). But I guess it\ndepends on what you are bisecting. In my case, \"yes\" would be the\ncurrent \"bad\", and \"no\" would be the current \"good\".\n\nSo maybe provide both --used-to and --has-property (which I really\ndislike as a name, but I can't think of anything better at the moment).\nAnd then we can interpret \"yes\" and \"no\" accordingly, depending which\none the user used (and while that _sounds_ confusing, it won't be to the\nuser; we'll be prompting them with \"you're looking for ...\", so their\nanswer will be very natural).\n\nBut with both of them, the user is free to phrase it in whatever way\nfeels natural.\n\n> When trying to find regression, you would say:\n> \n>     $ git bisect start --used-to='it works' v0.99 master\n\nJust for completeness, under my proposal this could also be:\n\n  $ git bisect start --has-property='git frotz is broken' v0.99 master\n\nWhich is probably slightly less natural. But:\n\n> When trying to find a fix, you would say:\n> \n>     $ git bisect start --used-to='checkout $tree $path clobbers $path' v0.99 master\n\nThis one would be a bit nicer:\n\n  $ git bisect start --has-property='checkout works' v0.99 master\n\n-Peff\n\nPS Side note: do we really need it to be interactive? I would think\n   you could do:\n\n      $ git bisect start --used-to='git frotz says xyzzy'\n      Bisecting: 171 revisions left to test after this (roughly 8 steps)\n      Checking whether 'git frotz says xyzzy'.\n      $ ... build and test ...\n      $ git bisect yes\n      Bisecting: 89 revisions left to test after this (roughly 7 steps)\n      Checking whether 'git frotz says xyzzy'.\n\n   It means the user is reminded, then tests, then responds. But I think\n   it would be enough of a reminder. The important thing is that we keep\n   mentioning it at each bisection step. Maybe it depends on how long\n   your build/test step is (I tend to work on things like git, where\n   that is a 30-second procedure, not a kernel with a long build and a\n   reboot in between :) ).\n"},{"id":"176752","messageId":"7vfwjaggd2.fsf@alter.siamese.dyndns.org","threadId":"28526","inReplyTo":"20111003104112.GE16078@sigill.intra.peff.net","subject":"Re: [RFC/PATCH]: reverse bisect v 2.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-03T17:00:25Z","receivedAt":"2011-10-03T17:00:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Sep 30, 2011 at 11:13:25AM -0700, Junio C Hamano wrote:\n>\n>> I wonder if something along the following line would make the usage more\n>> pleasant and self-explanatory:\n>> \n>>     $ git bisect start --used-to='git frotz says xyzzy' v0.99 master\n>>     Bisecting: 171 revisions left to test after this (roughly 8 steps)\n>>     $ ... build and then test ...\n>>     $ git bisect tested\n>>     You are trying to check: git frotz says xyzzy\n>>     Does the result satisify what you are trying to find [y/n]? yes\n>\n> I like this idea a lot. My \"yes/no\" thing was a \"if I were designing\n> bisect from scratch today...\" suggestion, but having something like\n> --used-to makes it a natural addition to the regular good/bad interface.\n> And I really like the prompt to help people remember what it is they're\n> declaring each time.\n\nI forgot to clarify that \"tested\" was only to help users who wanted\nreminder; if the user is confident with the usual \"yes/no\", the\ninteractivity is not required.\n\n> However, --used-to feels a bit backwards to me. I think of it as\n> \"--has-property\" or something similar.\n\nI do not think --used-to='frotz says xyzzy' is a good phrasing at all; it\nis grammatically incorrect. But --has-property has one large downside.  At\nleast --used-to makes it clear that the user is supposed to decribe the\nproperty of the tree in the past.\n\nLet's step back a bit to understand why I think this is not optimal.\n\nThe low-level \"bisect\" machinery is about bisecting a history whose\nancient commits have a property (e.g. \"used to work\" or \"used to be\nbroken\") and more recent ones does not have the property to find one\ncommit where the property is lost. Conceptually, this \"property\" does not\nhave any value judgement associated with it, but historically, we called\nthat property \"good\" (and lack of it \"bad\"), which caused confusion at the\nsurface level when looking for a recent fix. The user has to say \"ah, this\nis still broken and does not contain the fix---good\" with a twisted mind.\n\nTo dissociate the value judgement from the \"property\", it is a good idea\nto allow users to operate the bisect machinery without using \"good/bad\",\nand \"yes/no\" is a good pair of words to use. However, the user still needs\nto be aware of the fact that the \"property\" must exist in older history\nand must be lacking in newer history.\n\n\"--has-property\" strips the value judgement but does not surface this\ncrucial point to the UI. When the user wants to find a recent fix, the\nproperty can be spelled in various ways: \"it is broken\", \"it no longer is\nbroken\", \"it is fixed\", \"it works\", etc.. We need to map the 'yes' answer\nto 'bad' if --has-property=works is given and the user is looking for a\nfix, not a regression. But --has-property does not explicitly tell the\nuser that the property must exist in the past and absent in the present.\n\nMy --used-to was an attempt to surface this \"we are looking for a commit\nthat changed the condition that used to hold, but no longer holds for\nrecent commits\" to the UI level and force \"yes == observation consistent\nwith the older tree\" and \"no == new tree\" interpretation, without a need\nto map yes/no to good/bad or bad/good depending on what is hunted.\n\nThe alternative approach of \"bisect start --reverse\" was an attempt to\nallow swapping this mapping. When reverse is in effect, \"good\" maps to\n\"observation consistent with the newer tree\". I do not _mind_ mapping if\nit turns out to be absolutely necessary, but I wanted to see if we can do\nthis without one.\n\n> something appeared (be it a bug, a feature, or whatever). But I guess it\n> depends on what you are bisecting. In my case, \"yes\" would be the\n> current \"bad\", and \"no\" would be the current \"good\".\n\nI think we are aiming for the same thing, but I think you are assuming\nthat a naive user would always describe the condition that used to hold\nwith --has-property without being told; I do not think that assumption\nholds.\n"},{"id":"176831","messageId":"20111004103056.GB11236@sigill.intra.peff.net","threadId":"28526","inReplyTo":"7vfwjaggd2.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH]: reverse bisect v 2.0","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-04T10:30:56Z","receivedAt":"2011-10-04T10:30:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 03, 2011 at 10:00:25AM -0700, Junio C Hamano wrote:\n\n> > I like this idea a lot. My \"yes/no\" thing was a \"if I were designing\n> > bisect from scratch today...\" suggestion, but having something like\n> > --used-to makes it a natural addition to the regular good/bad interface.\n> > And I really like the prompt to help people remember what it is they're\n> > declaring each time.\n> \n> I forgot to clarify that \"tested\" was only to help users who wanted\n> reminder; if the user is confident with the usual \"yes/no\", the\n> interactivity is not required.\n\nThat makes sense to me. I think in either case, it would be nice to\nmention the --used-to text when we take each step. We're already\noutputting some status information there (like how many commits left).\n\n> > However, --used-to feels a bit backwards to me. I think of it as\n> > \"--has-property\" or something similar.\n> \n> I do not think --used-to='frotz says xyzzy' is a good phrasing at all; it\n> is grammatically incorrect. But --has-property has one large downside.  At\n> least --used-to makes it clear that the user is supposed to decribe the\n> property of the tree in the past.\n> \n> Let's step back a bit to understand why I think this is not optimal.\n\nWhat you say makes sense, but isn't it just a problem of the name? IOW,\na much better name than \"--has-property\" would be \"--started-to\". That\nwould imply the exact same cutoff as --used-to, but negate only the\nyes/no bit.\n\nSo you could say:\n\n  # find a bug:\n  git bisect start --used-to='work with --foo=bar'\n\n  # or if you are looking for a specific undesirable behavior, you might\n  # write:\n  git bisect start --used-to='not segfault with --foo=bar'\n\n  # but now you have a negation in your condition. So it might be more\n  # natural to write it as:\n  git bisect start --started-to='segfault with --foo=bar'\n\n  # Or we can find a feature\n  git bisect start --used-to='not respect core.foo'\n\n  # but again, we have a negation. Instead:\n  git bisect start --started-to='respect core.foo'\n\nAnd the --started-to would literally be implemented as flipping the\nmeaning of \"git bisect yes\" and \"git bisect no\", and nothing more. IOW,\nit's just another way of spelling \"git bisect --reverse\".\n\nI know you wanted to emphasize the \"older tree has this property\" of\n--used-to, but I think it is clear with --started-to that the older tree\nobviously obviously had the negation of the property.\n\n-Peff\n"},{"id":"176852","messageId":"7vfwj8dbn0.fsf@alter.siamese.dyndns.org","threadId":"28526","inReplyTo":"20111004103056.GB11236@sigill.intra.peff.net","subject":"Re: [RFC/PATCH]: reverse bisect v 2.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-04T15:22:59Z","receivedAt":"2011-10-04T15:22:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Oct 03, 2011 at 10:00:25AM -0700, Junio C Hamano wrote:\n>\n> And the --started-to would literally be implemented as flipping the\n> meaning of \"git bisect yes\" and \"git bisect no\", and nothing more. IOW,\n> it's just another way of spelling \"git bisect --reverse\".\n\nYes, if we wanted to also implement the flipping of the mapping between\nyes/no and good/bad, I do not have any problem with --used-to/--started-to\npair of options.\n\nThanks.\n"},{"id":"176886","messageId":"201110050034.46334.chriscool@tuxfamily.org","threadId":"28526","inReplyTo":"7vfwj8dbn0.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH]: reverse bisect v 2.0","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2011-10-04T22:34:45Z","receivedAt":"2011-10-04T22:34:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nOn Tuesday 04 October 2011 17:22:59 Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> > On Mon, Oct 03, 2011 at 10:00:25AM -0700, Junio C Hamano wrote:\n> > \n> > And the --started-to would literally be implemented as flipping the\n> > meaning of \"git bisect yes\" and \"git bisect no\", and nothing more. IOW,\n> > it's just another way of spelling \"git bisect --reverse\".\n> \n> Yes, if we wanted to also implement the flipping of the mapping between\n> yes/no and good/bad, I do not have any problem with --used-to/--started-to\n> pair of options.\n\nIf we decide to go with yes/no, an option like:\n\n--yes-means=<it behaves like this>\n\nseems to me easier to understand. Though I recognize that it doesn't tell that \nthe behavior changed.\n\nAnd before responding to this thread I wanted to have another look at the \nunfinished patch that Dscho sent a few years ago, but I did not have much time \nthese past few days, and I could not find it anymore.\n\nThanks,\nChristian.\n"},{"id":"176892","messageId":"7v62k4ban7.fsf@alter.siamese.dyndns.org","threadId":"28526","inReplyTo":"201110050034.46334.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH]: reverse bisect v 2.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-04T23:27:24Z","receivedAt":"2011-10-04T23:27:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> If we decide to go with yes/no, an option like:\n>\n> --yes-means=<it behaves like this>\n>\n> seems to me easier to understand. Though I recognize that it doesn't tell that \n> the behavior changed.\n\nWhat problem are you trying to solve?\n\nAs I already said while discussing --has-property vs --used-to, I think\nthat --yes-means shares the exact same downside as --has-property.\n\nThe --used-to proposal was to make sure that people who are bisecting to\nfind fixes would not by mistake say\n\n\tgit bisect start --used-to='work fine'\n\nas it is very clear that bisecting in a history with something that used\nto work fine is _not_ hunting for a fix.\n\nWhen the users say\n\n\tgit bisect start --yes-means='frotz is broken'\n\nis it clear to them that they are supposed to define what used to be the\ncase (in which case \"yes\" is always mapped to \"good\")?  If you are trying\nto allow them to say either old or new behaviour with --yes-means, how\ndoes your \"bisect\" know if it needs to map \"yes\" to \"good\" (regression\nhunting), or \"yes\" to \"bad\" (looking for a fix)?\n"},{"id":"177129","messageId":"CAH5451kUf=vPfgOOusmJjfbiyueX9VByJLzZ9WbyqLd0z78wxA@mail.gmail.com","threadId":"28526","inReplyTo":"7v62k4ban7.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH]: reverse bisect v 2.0","fromName":"Andrew Ardill","fromEmail":"andrew.ardill@gmail.com","sentAt":"2011-10-07T01:57:30Z","receivedAt":"2011-10-07T01:57:30Z","isPatch":true,"sender":{"key":"andrew.ardill@gmail.com","avatar":"https://gravatar.com/avatar/da14cb7c091dd44dc6c63a4d3361b149acaf25226dc78eb4131a17b93d9b0993?d=mp&s=160"},"body":"Hi, I have given this some thought and have a slightly different\nproposal for the options to use. I have written a commit message that\nI think explains the need for the improvement, and justifies my\nproposal. The options I propose are '--intoduced' and '--removed'.\n\n--->8---\n\nBisect is used to look for a commit that causes a specific change.\nSuch a change can be classified by the user as either introducing\nsomething, or removing something, and this distinction is arbitrary. A\nchange could be said to introduce a bug fix, or remove a bug - both\nformulations have inherently the same meaning, but search behaviour\nchanges depending on which one we use.\nSuppose I want to find the commit that removed a bug. If I examine a\nsnapshot and it contains the bug, the bug has not yet been *removed*\nand I must look in the future for its removal. Conversely, if I\nexamine a snapshot and the bug fix exists, the bug fix must have been\n*introduced* at some earlier point and so I must search backwards.\n\nConfusion exists at this point because at each verification step a\nquestion like 'Does this snapshot have X?' is asked, when what we\neventually want to know is 'When was X introduced?' or 'When was X\nremoved?'. Previously there has been no way to specify if we are\nlooking for X being introduced or removed; it was assumed that we were\nlooking for when something was introduced, for example when a bug was\nintroduced.\n\nTeach git bisect if we are looking for when something was introduced\nor when something was removed.\n\nExamples.\nSearch for a feature add:\n   $ git bisect start --introduced='feature: git frotz says xyzzy' v0.99 master\n   Bisecting: 171 revisions left to test after this (roughly 8 steps)\n   $ ... build and then test ...\n   $ git bisect tested\n   Does this snapshot have 'feature: git frotz says xyzzy' [y/n]? yes\n# already added, look backwards\n\nSearch for a feature regression:\n   $ git bisect start --removed='feature: git frotz says xyzzy' v0.99 master\n   Bisecting: 171 revisions left to test after this (roughly 8 steps)\n   $ ... build and then test ...\n   $ git bisect tested\n   Does this snapshot have 'feature: git frotz says xyzzy' [y/n]? yes\n# not removed yet, look forwards\n\n--->8---\n\nRegards,\n\nAndrew Ardill\n"},{"id":"177426","messageId":"7vr52ibydy.fsf@alter.siamese.dyndns.org","threadId":"28526","inReplyTo":"CAH5451kUf=vPfgOOusmJjfbiyueX9VByJLzZ9WbyqLd0z78wxA@mail.gmail.com","subject":"Re: [RFC/PATCH]: reverse bisect v 2.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-12T04:57:13Z","receivedAt":"2011-10-12T04:57:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Ardill <andrew.ardill@gmail.com> writes:\n\n> Examples.\n> Search for a feature add:\n>    $ git bisect start --introduced='feature: git frotz says xyzzy' v0.99 master\n>    Bisecting: 171 revisions left to test after this (roughly 8 steps)\n>    $ ... build and then test ...\n>    $ git bisect tested\n>    Does this snapshot have 'feature: git frotz says xyzzy' [y/n]? yes\n> # already added, look backwards\n>\n> Search for a feature regression:\n>    $ git bisect start --removed='feature: git frotz says xyzzy' v0.99 master\n>    Bisecting: 171 revisions left to test after this (roughly 8 steps)\n>    $ ... build and then test ...\n>    $ git bisect tested\n>    Does this snapshot have 'feature: git frotz says xyzzy' [y/n]? yes\n> # not removed yet, look forwards\n\nWith an obvious addition of non-interactive short-cut subcommands \"git\nbisect yes\" and \"git bisect no\", I think --removed= is a much better\nwording than --used-to= I suggested in the discussion.\n\nI however am still worried about the flipping of the mapping between\n<good,bad> and <yes,no> this design requires. What are we going to do to\nthe labels of low-level machinery (i.e $GIT_DIR/refs/bisect/bad and\n$GIT/refs/bisect/good)? They appear in \"bisect visualize\" and I was hoping\nthat it would be simpler in the code if we do not have to change them in\nsuch a way that depends on this introduced/removed switch, and that was\nthe reason why I was trying to see if we can solve this without the\nswitchable mapping between <good,bad> and <yes,no>.\n\nMore specifically, I was hoping that we can rename \"good\" to \"old\" and\n\"bad\" to \"new\" unconditionally and be done with it. We would ask the user\n\"What did the code used to do in the olden days?\" and \"Does this version\nbehave the same as it used to?\". The possible answers the user can give\nare \"git bisect old\" (it behaves the same as the older versions) and \"git\nbisect new\" (it behaves the same as the newer versions). Then we do not\nhave to worry about having to flip the meaning of <yes> and <no> at the UI\nlevel.\n"},{"id":"177481","messageId":"20111012201410.GB1502@sigill.intra.peff.net","threadId":"28526","inReplyTo":"7vr52ibydy.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH]: reverse bisect v 2.0","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-12T20:14:10Z","receivedAt":"2011-10-12T20:14:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 11, 2011 at 09:57:13PM -0700, Junio C Hamano wrote:\n\n> With an obvious addition of non-interactive short-cut subcommands \"git\n> bisect yes\" and \"git bisect no\", I think --removed= is a much better\n> wording than --used-to= I suggested in the discussion.\n\nAgreed.\n\n> I however am still worried about the flipping of the mapping between\n> <good,bad> and <yes,no> this design requires. What are we going to do to\n> the labels of low-level machinery (i.e $GIT_DIR/refs/bisect/bad and\n> $GIT/refs/bisect/good)? They appear in \"bisect visualize\" and I was hoping\n> that it would be simpler in the code if we do not have to change them in\n> such a way that depends on this introduced/removed switch, and that was\n> the reason why I was trying to see if we can solve this without the\n> switchable mapping between <good,bad> and <yes,no>.\n\nHmm. I hadn't thought about the labels. In a yes/no situation, though,\ncouldn't you use the labels as the user sees them?\n\nThen it is simply a matter of flipping yes/no inside the bisect script\nwhenever we interact with the user (i.e., \"git bisect yes\") or when we\ninteract with the on-disk labels.\n\nCertainly it's more complex than not allowing reversing, though.\n\n> More specifically, I was hoping that we can rename \"good\" to \"old\" and\n> \"bad\" to \"new\" unconditionally and be done with it. We would ask the user\n> \"What did the code used to do in the olden days?\" and \"Does this version\n> behave the same as it used to?\". The possible answers the user can give\n> are \"git bisect old\" (it behaves the same as the older versions) and \"git\n> bisect new\" (it behaves the same as the newer versions). Then we do not\n> have to worry about having to flip the meaning of <yes> and <no> at the UI\n> level.\n\nHmm. I think this is not quite as nice, but it is way simpler. It may be\nworth trying for a bit to see how people like it. If they don't, the\ncost of failure is that we have to maintain \"old/new\" forever, even\nafter we implement a yes/no reversible scheme. But maintaining the\nold/new mapping from yes/no would not be any harder than the good/bad\nmapping, which we would need to do anyway.\n\nSo it sounds like a reasonable first step.\n\n-Peff\n"}]}