{"thread":{"id":"51304","subject":"git bisect should return 1 when the first bad commit is found","startedAt":"2019-06-13T17:11:01Z","lastAt":"2019-06-25T05:13:12Z","messageCount":9,"participants":["Pedro Larroy","Junio C Hamano","Jeff King","Christian Couder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"377152","messageId":"CAC_CU1iWBCTt5jLQ0Zp2HoyA0oFgG3shDB2rLfOsmC+x67NdAw@mail.gmail.com","threadId":"51304","inReplyTo":null,"subject":"git bisect should return 1 when the first bad commit is found","fromName":"Pedro Larroy","fromEmail":"pedro.larroy.lists@gmail.com","sentAt":"2019-06-12T22:33:43Z","receivedAt":"2019-06-13T17:11:01Z","isPatch":false,"sender":{"key":"pedro.larroy.lists@gmail.com","avatar":null},"body":"Hi\n\nI'm using git bisect, and for complex reasons I can't use git bisect\nrun to drive the bisection. I don't think git bisect bad/good is\nreturning a different status code when the first bad commit is found\nand this would be very useful to stop the driver of the bisection.\n\nWhat do you think?  Would such a change be acceptable?\n\nThank you!\n\nPedro.\n"},{"id":"377168","messageId":"xmqq5zp9wdjb.fsf@gitster-ct.c.googlers.com","threadId":"51304","inReplyTo":"CAC_CU1iWBCTt5jLQ0Zp2HoyA0oFgG3shDB2rLfOsmC+x67NdAw@mail.gmail.com","subject":"Re: git bisect should return 1 when the first bad commit is found","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-13T18:22:48Z","receivedAt":"2019-06-13T18:22:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pedro Larroy <pedro.larroy.lists@gmail.com> writes:\n\n> I'm using git bisect, and for complex reasons I can't use git bisect\n> run to drive the bisection. I don't think git bisect bad/good is\n> returning a different status code when the first bad commit is found\n> and this would be very useful to stop the driver of the bisection.\n>\n> What do you think?  Would such a change be acceptable?\n\nProbably not.\n\nA non-zero exit code would indicate that \"git bisect\" had trouble\nfinding the next commit to check after being told 'good' (or 'bad'),\nwhich you wouldn't be able to tell from your \"first bad one is\nfound\".\n\nThe 'first bad one' is not necessarily the HEAD when the bisect\nprocedure finds it, so you'd have to read and parse \"x is the first\nsuch commit\" message anyway, no?  And once you start parsing the\nmessage from 'git bisect', you should be able to tell between\n\"please try this one, you have approximately 7 steps left\" and \"we\nfound the first bad one, which is X\", without relying on the exit\nstatus, no?\n\n"},{"id":"377839","messageId":"CAC_CU1hHK5n4GcETbWFLBeqT_Y9LV6N32Rv0F0+OCXH2G0qg8Q@mail.gmail.com","threadId":"51304","inReplyTo":"xmqq5zp9wdjb.fsf@gitster-ct.c.googlers.com","subject":"Re: git bisect should return 1 when the first bad commit is found","fromName":"Pedro Larroy","fromEmail":"pedro.larroy.lists@gmail.com","sentAt":"2019-06-23T20:32:16Z","receivedAt":"2019-06-23T20:32:29Z","isPatch":false,"sender":{"key":"pedro.larroy.lists@gmail.com","avatar":null},"body":"Thanks for your answer.\n\nI was expecting the HEAD to point to the first bad commit.\n\nIn mercurial, the exit status tells you information about the\nbisection process:  https://www.mercurial-scm.org/repo/hg/help/bisect\n\nSure one can parse stdout, it's just more tedious than just checking\nthe return code and having the HEAD left to the original bad commit.\n\nPedro.\n\nOn Thu, Jun 13, 2019 at 11:22 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Pedro Larroy <pedro.larroy.lists@gmail.com> writes:\n>\n> > I'm using git bisect, and for complex reasons I can't use git bisect\n> > run to drive the bisection. I don't think git bisect bad/good is\n> > returning a different status code when the first bad commit is found\n> > and this would be very useful to stop the driver of the bisection.\n> >\n> > What do you think?  Would such a change be acceptable?\n>\n> Probably not.\n>\n> A non-zero exit code would indicate that \"git bisect\" had trouble\n> finding the next commit to check after being told 'good' (or 'bad'),\n> which you wouldn't be able to tell from your \"first bad one is\n> found\".\n>\n> The 'first bad one' is not necessarily the HEAD when the bisect\n> procedure finds it, so you'd have to read and parse \"x is the first\n> such commit\" message anyway, no?  And once you start parsing the\n> message from 'git bisect', you should be able to tell between\n> \"please try this one, you have approximately 7 steps left\" and \"we\n> found the first bad one, which is X\", without relying on the exit\n> status, no?\n>\n"},{"id":"377843","messageId":"20190623224935.GD1100@sigill.intra.peff.net","threadId":"51304","inReplyTo":"CAC_CU1hHK5n4GcETbWFLBeqT_Y9LV6N32Rv0F0+OCXH2G0qg8Q@mail.gmail.com","subject":"Re: git bisect should return 1 when the first bad commit is found","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-23T22:49:35Z","receivedAt":"2019-06-24T02:49:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 23, 2019 at 01:32:16PM -0700, Pedro Larroy wrote:\n\n> Thanks for your answer.\n> \n> I was expecting the HEAD to point to the first bad commit.\n> \n> In mercurial, the exit status tells you information about the\n> bisection process:  https://www.mercurial-scm.org/repo/hg/help/bisect\n> \n> Sure one can parse stdout, it's just more tedious than just checking\n> the return code and having the HEAD left to the original bad commit.\n\nI think it might be nice for Git to write a well-known refname (like\nBISECT_RESULT or similar) so that you can refer to that instead of\nhaving to read stdout (whether by machine or by a user\ncutting-and-pasting). And I cannot offhand think of a particular reason\nwhy that could not just be HEAD (instead of something bisect-specific)\nafter the bisect finishes.\n\nWe do not promise any particular value in HEAD now. The only downside\nwould be the minor cost to checkout the working tree of the known-bad\ncommit if we are not already there.\n\n-Peff\n"},{"id":"377844","messageId":"CAP8UFD1gWnkJZSzjOxOHdS4R3GCUSDgC9X85y_Yvc0LmyK19Cw@mail.gmail.com","threadId":"51304","inReplyTo":"20190623224935.GD1100@sigill.intra.peff.net","subject":"Re: git bisect should return 1 when the first bad commit is found","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-06-24T07:16:13Z","receivedAt":"2019-06-24T07:16:29Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Jun 24, 2019 at 4:51 AM Jeff King <peff@peff.net> wrote:\n>\n> On Sun, Jun 23, 2019 at 01:32:16PM -0700, Pedro Larroy wrote:\n>\n> > Thanks for your answer.\n> >\n> > I was expecting the HEAD to point to the first bad commit.\n> >\n> > In mercurial, the exit status tells you information about the\n> > bisection process:  https://www.mercurial-scm.org/repo/hg/help/bisect\n\nIt's not clear from he above URL how that differs from what git bisect\ndoes. I only found \"Returns 0 on success\" there which is not very\nexplicit, and we could argue that it's also what git bisect does.\n\n> > Sure one can parse stdout, it's just more tedious than just checking\n> > the return code and having the HEAD left to the original bad commit.\n\nThe git bisect documentation says:\n\n       Eventually there will be no more revisions left to inspect, and\nthe command will print out a description of the first bad commit. The\nreference\n       refs/bisect/bad will be left pointing at that commit.\n\nSo you just need to parse stdout to detect that it found the first bad\ncommit, and then you can use refs/bisect/bad.\n\nIf the return code was used, how would you distinguish between a\nfailure in the command (for example if you give bad information to\n`git bisect good` or `git bisect bad`) and the fact that it has not\nyet found the first bad commit? Anyway you would need to add some\nlogic for that.\n\n> I think it might be nice for Git to write a well-known refname (like\n> BISECT_RESULT or similar) so that you can refer to that instead of\n> having to read stdout (whether by machine or by a user\n> cutting-and-pasting). And I cannot offhand think of a particular reason\n> why that could not just be HEAD (instead of something bisect-specific)\n> after the bisect finishes.\n\nIf it would be HEAD, it would mean that git bisect would potentially\nhave to do one more checkout so that HEAD points to the first bad\ncommit. This checkout would sometimes be useless, so it's more\nefficient to use something like refs/bisect/bad rather than HEAD.\n\n> We do not promise any particular value in HEAD now. The only downside\n> would be the minor cost to checkout the working tree of the known-bad\n> commit if we are not already there.\n\nThough we might not explicitly promise in the doc that HEAD will stay\nat the last commit that was tested, I think that's something people\ncan expect from the way we describe how bisect work. So I don't think\nit would be a good idea to change our behavior.\n"},{"id":"377919","messageId":"xmqqo92mhlla.fsf@gitster-ct.c.googlers.com","threadId":"51304","inReplyTo":"20190623224935.GD1100@sigill.intra.peff.net","subject":"Re: git bisect should return 1 when the first bad commit is found","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-24T18:41:53Z","receivedAt":"2019-06-24T18:42:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sun, Jun 23, 2019 at 01:32:16PM -0700, Pedro Larroy wrote:\n>\n>> Thanks for your answer.\n>> \n>> I was expecting the HEAD to point to the first bad commit.\n>> \n>> In mercurial, the exit status tells you information about the\n>> bisection process:  https://www.mercurial-scm.org/repo/hg/help/bisect\n>> \n>> Sure one can parse stdout, it's just more tedious than just checking\n>> the return code and having the HEAD left to the original bad commit.\n>\n> I think it might be nice for Git to write a well-known refname (like\n> BISECT_RESULT or similar) so that you can refer to that instead of\n> having to read stdout (whether by machine or by a user\n> cutting-and-pasting). And I cannot offhand think of a particular reason\n> why that could not just be HEAD (instead of something bisect-specific)\n> after the bisect finishes.\n\nAs Christian downthread reminds us, that is what the bisect/bad ref\nis (which I totally forgot when I gave the earlier response).  I do\nnot think we need a new ref, but I do not think it is so bad to add\nan option \"git bisect --exit-code ( --good | --bad ) [<commit-ish>]\"\nthat makes the command usually exit with non-zero status.  Unless we\nhave found the final answer successfully, that is, and in that case\nthe command would exit with 0 status to signall \"all done\".\n\nBut that should be an option.\n"},{"id":"377938","messageId":"20190625045304.GA7626@sigill.intra.peff.net","threadId":"51304","inReplyTo":"xmqqo92mhlla.fsf@gitster-ct.c.googlers.com","subject":"Re: git bisect should return 1 when the first bad commit is found","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-25T04:53:04Z","receivedAt":"2019-06-25T04:53:07Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 24, 2019 at 11:41:53AM -0700, Junio C Hamano wrote:\n\n> > I think it might be nice for Git to write a well-known refname (like\n> > BISECT_RESULT or similar) so that you can refer to that instead of\n> > having to read stdout (whether by machine or by a user\n> > cutting-and-pasting). And I cannot offhand think of a particular reason\n> > why that could not just be HEAD (instead of something bisect-specific)\n> > after the bisect finishes.\n> \n> As Christian downthread reminds us, that is what the bisect/bad ref\n> is (which I totally forgot when I gave the earlier response).  I do\n> not think we need a new ref, but I do not think it is so bad to add\n> an option \"git bisect --exit-code ( --good | --bad ) [<commit-ish>]\"\n> that makes the command usually exit with non-zero status.  Unless we\n> have found the final answer successfully, that is, and in that case\n> the command would exit with 0 status to signall \"all done\".\n> \n> But that should be an option.\n\nAh, right, I forgot that we already have such a ref. So that does seem\nlike a reasonable way to access it (and as Christian notes, it's\ndocumented as well).  And I agree with you that any exit-code magic\nshould require the user to ask for it explicitly.\n\nI do think that accessing the bisect/bad ref is a little intimate with\nthe implementation (i.e., it implies knowing that there is only a single\n\"bad\" that we are moving around, unlike \"good\", where we may mark many\nsuch tips).\n\nSo another option is to put the result in a new ref (bisect/result,\nperhaps), but with one twist: delete it at the beginning of a bisection,\nso its presence can be used as a marker that the bisection is complete.\nI.e., you could do \"git rev-parse --verify refs/bisect/result\" as an\nalternative to \"--exit-code\". That gives you the added flexibility of\nasking \"is the bisection done\" separately from \"do this next bisection\nstep\".\n\n-Peff\n"},{"id":"377939","messageId":"20190625045848.GA7571@sigill.intra.peff.net","threadId":"51304","inReplyTo":"CAP8UFD1gWnkJZSzjOxOHdS4R3GCUSDgC9X85y_Yvc0LmyK19Cw@mail.gmail.com","subject":"Re: git bisect should return 1 when the first bad commit is found","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-25T04:58:49Z","receivedAt":"2019-06-25T04:58:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 24, 2019 at 09:16:13AM +0200, Christian Couder wrote:\n\n> So you just need to parse stdout to detect that it found the first bad\n> commit, and then you can use refs/bisect/bad.\n>\n> If the return code was used, how would you distinguish between a\n> failure in the command (for example if you give bad information to\n> `git bisect good` or `git bisect bad`) and the fact that it has not\n> yet found the first bad commit? Anyway you would need to add some\n> logic for that.\n\nRight, thanks for reminding us of that ref. I did leave a suggestion\nelsewhere in the thread that we might be able to avoid the exit-code\npitfalls by using a specific \"result\" ref.\n\n> If it would be HEAD, it would mean that git bisect would potentially\n> have to do one more checkout so that HEAD points to the first bad\n> commit. This checkout would sometimes be useless, so it's more\n> efficient to use something like refs/bisect/bad rather than HEAD.\n\nI'm not too concerned with the cost of checking out a tree. After all,\nwe've just done a bunch of checkouts and probably some operations on the\ntree itself. But I think there's a much more important reason my\nsuggestion may have problems.\n\nSometimes we _can't_ checkout another commit, because part of testing\nthe bisection may involve mucking with the working tree. For instance,\nearlier today I was bisecting a case where git.git did not show the\nbreakage itself, but applying a debugging patch on top did. So I was\nbisecting along the real history, adding new contents, testing, and then\ncleaning up my changes before jumping to the next bisection candidate.\n\nOn the other hand, I have to clean them up anyway before running \"git\nbisect good\" or it cannot jump to the next candidate when we are not\nfinished. Jumping to \"bad\" when we _are_ finished is no different there.\nBut I think it does show that there can be surprising implications to\nmoving HEAD around.\n\n-Peff\n"},{"id":"377940","messageId":"xmqqv9wufdsw.fsf@gitster-ct.c.googlers.com","threadId":"51304","inReplyTo":"20190625045304.GA7626@sigill.intra.peff.net","subject":"Re: git bisect should return 1 when the first bad commit is found","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-25T05:13:03Z","receivedAt":"2019-06-25T05:13:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I do think that accessing the bisect/bad ref is a little intimate with\n> the implementation (i.e., it implies knowing that there is only a single\n> \"bad\" that we are moving around, unlike \"good\", where we may mark many\n> such tips).\n\nI actually do not think it is merely an implementation detail, but a\nrather fundamental part of the \"bisect\" design.  After all, we are\ntrying to find *the* single transition point from \"the old world\" to\n\"the new world\", and the primary assumption that makes \"bisection\"\nwork in the first place is that there is only one such transition\npoint from old to new.  Otherwise, there may be a stretch of history\nwhere one transition from \"old\" to \"new\" is followed by another\ntransition from \"new\" back to \"old\" and then again another one from\n\"old\" to \"new\"; in such a \"stripe snake\" part of the history, \"find\nthe midpoint, if it is from the new world, then narrow to the world\nbefore that midpoint\", which is the central idea of the \"bisection\"\nwould not work, would it?\n\nSo, ... under that \"single transition point\" assumption, how would a\n\"bisect\" that uses more than one \"bad\" would work?  \"bad\" is the one\nthat is known to belong to the \"new\" world that is topologically the\noldest and it moves backward in topology to find the boundary from\nthe \"old\" world.  If there were another \"bad\" (i.e. one that is\nknown to belong the the \"new\" world, that may not be the oldest),\nshouldn't that have the other one as its ancestor, making it\nredundant (i.e. we know we have inherited the badness from the other\none)?\n\nHaving said that ...\n\n> So another option is to put the result in a new ref (bisect/result,\n> perhaps), but with one twist: delete it at the beginning of a bisection,\n> so its presence can be used as a marker that the bisection is complete.\n\n... I kind of like this one exactly for the reasons you give below.\n\n> I.e., you could do \"git rev-parse --verify refs/bisect/result\" as an\n> alternative to \"--exit-code\". That gives you the added flexibility of\n> asking \"is the bisection done\" separately from \"do this next bisection\n> step\".\n>\n> -Peff\n"}]}