{"thread":{"id":"44873","subject":"[PATCH] Documentation/bisect: improve on (bad|new) and (good|bad)","startedAt":"2017-01-13T14:51:31Z","lastAt":"2017-01-17T19:58:19Z","messageCount":5,"participants":["Christian Couder","Junio C Hamano","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"309354","messageId":"20170113144405.3963-1-chriscool@tuxfamily.org","threadId":"44873","inReplyTo":null,"subject":"[PATCH] Documentation/bisect: improve on (bad|new) and (good|bad)","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-13T14:44:05Z","receivedAt":"2017-01-13T14:51:31Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"The following part of the description:\n\ngit bisect (bad|new) [<rev>]\ngit bisect (good|old) [<rev>...]\n\nmay be a bit confusing, as a reader may wonder if instead it should be:\n\ngit bisect (bad|good) [<rev>]\ngit bisect (old|new) [<rev>...]\n\nOf course the difference between \"[<rev>]\" and \"[<rev>...]\" should hint\nthat there is a good reason for the way it is.\n\nBut we can further clarify and complete the description by adding\n\"<term-new>\" and \"<term-old>\" to the \"bad|new\" and \"good|old\"\nalternatives.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Documentation/git-bisect.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 2bb9a577a2..bdd915a66b 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 start [--term-{old,good}=<term> --term-{new,bad}=<term>]\n \t\t  [--no-checkout] [<bad> [<good>...]] [--] [<paths>...]\n- git bisect (bad|new) [<rev>]\n- git bisect (good|old) [<rev>...]\n+ git bisect (bad|new|<term-new>) [<rev>]\n+ git bisect (good|old|<term-old>) [<rev>...]\n  git bisect terms [--term-good | --term-bad]\n  git bisect skip [(<rev>|<range>)...]\n  git bisect reset [<commit>]\n-- \n2.11.0.313.g11b7cc88e6.dirty\n\n"},{"id":"309387","messageId":"xmqqinpihiwz.fsf@gitster.mtv.corp.google.com","threadId":"44873","inReplyTo":"20170113144405.3963-1-chriscool@tuxfamily.org","subject":"Re: [PATCH] Documentation/bisect: improve on (bad|new) and (good|bad)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-13T19:14:04Z","receivedAt":"2017-01-13T19:14:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> The following part of the description:\n>\n> git bisect (bad|new) [<rev>]\n> git bisect (good|old) [<rev>...]\n>\n> may be a bit confusing, as a reader may wonder if instead it should be:\n>\n> git bisect (bad|good) [<rev>]\n> git bisect (old|new) [<rev>...]\n>\n> Of course the difference between \"[<rev>]\" and \"[<rev>...]\" should hint\n> that there is a good reason for the way it is.\n>\n> But we can further clarify and complete the description by adding\n> \"<term-new>\" and \"<term-old>\" to the \"bad|new\" and \"good|old\"\n> alternatives.\n>\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n>  Documentation/git-bisect.txt | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n\nThanks.  The patch looks good.\n\nA related tangent.  \n\nLast night, I was trying to think if there is a fundamental reason\nwhy \"bad/new/term-new\" cannot take more than one <rev>s on the newer\nside of the bisection, and couldn't quite think of any before\nfalling asleep.\n\nCurrently we keep track of a single bisect/bad, while marking all the\nrevs given as good previously as bisect/good-<SHA-1>.\n\nBecause the next \"bad\" is typically chosen from the region of the\ncommit DAG that is bounded by bad and good commits, i.e. \"rev-list\nbisect/bad --not bisect/good-*\", the current bisect/bad will always\nbe an ancestor of all bad commits that used to be bisect/bad, and\nkeeping previous bisect/bad as bisect/bad-<SHA-1> won't change the\nregion of the commit DAG yet to be explored.\n\nAs a reason why we need to use only a single bisect/bad, the above\ndescription is understandable.  But as a reason why we cannot have\nmore than one, it is tautological.  It merely says \"if we start from\nonly one and dig history to find older culprit, we need only one\nbad\".\n\nI fell asleep last night without thinking further than that.\n\nI think the answer to the question \"why do we think we need a single\nbisect/bad?\" is \"because bisection is about assuming that there is\nonly one commit that flips the tree state from 'old' to 'new' and\nfinding that single commit\".  That would mean that even if we had\nbisect/bad-A and bisect/bad-B, e.g.\n\n                          o---o---o---bad-A\n                         /\n    -----Good---o---o---o\n                         \\\n                          o---o---o---bad-B\n\n\nwhere 'o' are all commits whose goodness is not yet known, because\nbisection is valid only when we are hunting for a single commit that\nflips the state from good to bad, that commit MUST be at or before\nthe merge base of bad-A and bad-B.  So even if we allowed\n\n\t$ git bisect bad bad-A bad-B\n\non the command line, we won't have to set bisect/bad-A and\nbisect/bad-B.  We only need a single bisect/bad that points at the\nmerge base of these two.\n\nBut what if bad-A and bad-B have more than one merge bases?  We\nwon't know which side the badness came from.\n\n                          o---o---o---bad-A\n                         /     \\ / \n    -----Good---o---o---o       / \n                         \\     / \\\n                          o---o---o---bad-B\n\nBeing able to bisect the region of DAG bound by \"^Good bad-A bad-B\"\nmay have value in such a case.  I dunno.\n\n"},{"id":"309440","messageId":"CAP8UFD31Hb_4phoHPL51Rq0bfW3_xnL9LMosdH-0h=JX3Uwyvw@mail.gmail.com","threadId":"44873","inReplyTo":"xmqqinpihiwz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Documentation/bisect: improve on (bad|new) and (good|bad)","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-15T14:51:11Z","receivedAt":"2017-01-15T14:51:18Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Jan 13, 2017 at 8:14 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n>> The following part of the description:\n>>\n>> git bisect (bad|new) [<rev>]\n>> git bisect (good|old) [<rev>...]\n>>\n>> may be a bit confusing, as a reader may wonder if instead it should be:\n>>\n>> git bisect (bad|good) [<rev>]\n>> git bisect (old|new) [<rev>...]\n>>\n>> Of course the difference between \"[<rev>]\" and \"[<rev>...]\" should hint\n>> that there is a good reason for the way it is.\n>>\n>> But we can further clarify and complete the description by adding\n>> \"<term-new>\" and \"<term-old>\" to the \"bad|new\" and \"good|old\"\n>> alternatives.\n>>\n>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n>> ---\n>>  Documentation/git-bisect.txt | 4 ++--\n>>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> Thanks.  The patch looks good.\n>\n> A related tangent.\n>\n> Last night, I was trying to think if there is a fundamental reason\n> why \"bad/new/term-new\" cannot take more than one <rev>s on the newer\n> side of the bisection, and couldn't quite think of any before\n> falling asleep.\n>\n> Currently we keep track of a single bisect/bad, while marking all the\n> revs given as good previously as bisect/good-<SHA-1>.\n>\n> Because the next \"bad\" is typically chosen from the region of the\n> commit DAG that is bounded by bad and good commits, i.e. \"rev-list\n> bisect/bad --not bisect/good-*\", the current bisect/bad will always\n> be an ancestor of all bad commits that used to be bisect/bad, and\n> keeping previous bisect/bad as bisect/bad-<SHA-1> won't change the\n> region of the commit DAG yet to be explored.\n>\n> As a reason why we need to use only a single bisect/bad, the above\n> description is understandable.  But as a reason why we cannot have\n> more than one, it is tautological.  It merely says \"if we start from\n> only one and dig history to find older culprit, we need only one\n> bad\".\n>\n> I fell asleep last night without thinking further than that.\n>\n> I think the answer to the question \"why do we think we need a single\n> bisect/bad?\" is \"because bisection is about assuming that there is\n> only one commit that flips the tree state from 'old' to 'new' and\n> finding that single commit\".  That would mean that even if we had\n> bisect/bad-A and bisect/bad-B, e.g.\n>\n>                           o---o---o---bad-A\n>                          /\n>     -----Good---o---o---o\n>                          \\\n>                           o---o---o---bad-B\n>\n>\n> where 'o' are all commits whose goodness is not yet known, because\n> bisection is valid only when we are hunting for a single commit that\n> flips the state from good to bad, that commit MUST be at or before\n> the merge base of bad-A and bad-B.  So even if we allowed\n>\n>         $ git bisect bad bad-A bad-B\n>\n> on the command line, we won't have to set bisect/bad-A and\n> bisect/bad-B.  We only need a single bisect/bad that points at the\n> merge base of these two.\n>\n> But what if bad-A and bad-B have more than one merge bases?  We\n> won't know which side the badness came from.\n>\n>                           o---o---o---bad-A\n>                          /     \\ /\n>     -----Good---o---o---o       /\n>                          \\     / \\\n>                           o---o---o---bad-B\n>\n> Being able to bisect the region of DAG bound by \"^Good bad-A bad-B\"\n> may have value in such a case.  I dunno.\n\nI agree that there could be improvements in this area. Though from my\nexperience with special cases, like when a good commit is not an\nancestor of the bad commit (where there are probably bugs still\nlurking), I think it could be tricky to implement correctly in all\ncases, and it could make it even more difficult, than it sometimes\nalready is, to explain the resulting behavior to users.\n"},{"id":"309477","messageId":"vpqfukjpdnp.fsf@anie.imag.fr","threadId":"44873","inReplyTo":"xmqqinpihiwz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Documentation/bisect: improve on (bad|new) and (good|bad)","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2017-01-16T09:17:14Z","receivedAt":"2017-01-16T09:26:41Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n>> The following part of the description:\n>>\n>> git bisect (bad|new) [<rev>]\n>> git bisect (good|old) [<rev>...]\n>>\n>> may be a bit confusing, as a reader may wonder if instead it should be:\n>>\n>> git bisect (bad|good) [<rev>]\n>> git bisect (old|new) [<rev>...]\n>>\n>> Of course the difference between \"[<rev>]\" and \"[<rev>...]\" should hint\n>> that there is a good reason for the way it is.\n>>\n>> But we can further clarify and complete the description by adding\n>> \"<term-new>\" and \"<term-old>\" to the \"bad|new\" and \"good|old\"\n>> alternatives.\n>>\n>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n>> ---\n>>  Documentation/git-bisect.txt | 4 ++--\n>>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> Thanks.  The patch looks good.\n\nLooks good to me too.\n\n> I think the answer to the question \"why do we think we need a single\n> bisect/bad?\" is \"because bisection is about assuming that there is\n> only one commit that flips the tree state from 'old' to 'new' and\n> finding that single commit\".\n\nI wouldn't say it's about \"assuming\" there's only one commit, but it's\nabout finding *one* such commit, i.e. it works if there are several such\ncommits, but won't find them all.\n\n> But what if bad-A and bad-B have more than one merge bases?  We\n> won't know which side the badness came from.\n>\n>                           o---o---o---bad-A\n>                          /     \\ / \n>     -----Good---o---o---o       / \n>                          \\     / \\\n>                           o---o---o---bad-B\n>\n> Being able to bisect the region of DAG bound by \"^Good bad-A bad-B\"\n> may have value in such a case.  I dunno.\n\nI could help finding several guilty commits, but anyway you can't\nguarantee you'll find them all as soon as you use a binary search: if\nthe history looks like\n\n--- Good --- Bad --- Good --- Good --- Bad --- Good --- Bad\n\nthen without examining all commits, you can't tell how many good->bad\nswitches occured.\n\nBut keeping several bad commits wouldn't help keeping the set of\npotentially guilty commits small: bad commits appear on the positive\nside in \"^Good bad-A bad-B\", so having more bad commits mean having a\nlarger DAG to explore (which is a bit counter-intuitive: without\nthinking about it I'd have said \"more info => less commits to explore\").\n\nSo, if finding all guilty commits is not possible, I'm not sure how\nvaluable it is to try to find several of them.\n\nOTOH, keeping several good commits is needed to find a commit for which\nall parents are good and the commit is bad, i.e. distinguish\n\nGood\n    \\\n     Bad <-- this is the one.\n    /\nGood\n\nand\n\nGood\n    \\\n     Bad <-- need to dig further\n    /\n Bad\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"309543","messageId":"xmqq37ghfoh9.fsf@gitster.mtv.corp.google.com","threadId":"44873","inReplyTo":"vpqfukjpdnp.fsf@anie.imag.fr","subject":"Re: [PATCH] Documentation/bisect: improve on (bad|new) and (good|bad)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-17T19:58:10Z","receivedAt":"2017-01-17T19:58:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n>> But what if bad-A and bad-B have more than one merge bases?  We\n>> won't know which side the badness came from.\n>>\n>>                           o---o---o---bad-A\n>>                          /     \\ / \n>>     -----Good---o---o---o       / \n>>                          \\     / \\\n>>                           o---o---o---bad-B\n>>\n>> Being able to bisect the region of DAG bound by \"^Good bad-A bad-B\"\n>> may have value in such a case.  I dunno.\n>\n> I could help finding several guilty commits, but anyway you can't\n> guarantee you'll find them all as soon as you use a binary search: if\n> the history looks like\n>\n> --- Good --- Bad --- Good --- Good --- Bad --- Good --- Bad\n>\n> then without examining all commits, you can't tell how many good->bad\n> switches occured.\n>\n> But keeping several bad commits wouldn't help keeping the set of\n> potentially guilty commits small: bad commits appear on the positive\n> side in \"^Good bad-A bad-B\", so having more bad commits mean having a\n> larger DAG to explore (which is a bit counter-intuitive: without\n> thinking about it I'd have said \"more info => less commits to explore\").\n>\n> So, if finding all guilty commits is not possible, I'm not sure how\n> valuable it is to try to find several of them.\n\nThe criss-cross merge example, is not trying to find multiple\nsources of badness.  It still assumes [*1*] that there is only one\nevent that introduced the badness observed at bad-A and bad-B, both\nof which inherited the badness from the same such event.  Unlike a\ncase with a single/unique merge-base, we cannot say \"we can start\nfrom the merge-base, as their common badness must be coming from the\nsame place\".  The badness may exist in the first 'o' on the same\nline as bad-A in the above picture, which is an ancestor of one\nmerge-base on that line and does not break the other merge base on\nthe same line as bad-B, for example.\n\n> OTOH, keeping several good commits is needed to find a commit for which\n> all parents are good and the commit is bad.\n\nYes, that is correct.\n\n\n[Footnote]\n\n*1* The assumption is what makes \"bisect\" workable.  If the\n    assumption does not hold, then \"bisect\" would not give a useful\n    answer \"where did I screw up?\".  It gives a fairly useless \"I\n    found one bad commit whose parent is good---there is no\n    guarantee if that has anything to do with the badness you are\n    seeing at the tip\".\n"}]}