{"thread":{"id":"26756","subject":"git bisect code 125 - \"WFT?\"","startedAt":"2011-03-16T20:44:03Z","lastAt":"2011-03-19T15:20:15Z","messageCount":8,"participants":["Piotr Krukowiecki","Junio C Hamano","Johannes Sixt","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"163511","messageId":"AANLkTikZ3Po-YdhO-qCn5usVkt4J196eFF6YdbAeMG_X@mail.gmail.com","threadId":"26756","inReplyTo":null,"subject":"git bisect code 125 - \"WFT?\"","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2011-03-16T20:44:03Z","receivedAt":"2011-03-16T20:44:03Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"Hi,\n\nin git bisect exit code 1-124 and 126-127 means \"source is bad\"\n(0 means \"ok\", 125 means \"can't be tested\" and anything other is \"stop\").\n\nMy first reaction was \"WTF\" - you have a special value in the middle\nof a range??\n\nIt got a bit clearer after reading the original patch[1] and man bash. It seems\nthis value is last \"free\" value for the user to use, at least in bash - values\nabove 125 may be used by the shell.\n\nBash uses code 126 if command is not executable, and 127 if command is\nnot found.\n\nI think it would be better to use 126 and 127 as either \"can't be tested\" or\n\"stop\" (125 should be left as is):\n\n * It would not leave a WTF gap in the \"bad\" range\n\n * If you get \"command not executable\" or \"command not found\" it's more\n probable something is broken than the code is bad, and you should fix\n your script\n\nOpinions? Would it be possible to change the meaning of the codes now\n(in 1.8.0)?\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/62390\n\n-- \nPiotr Krukowiecki\n"},{"id":"163516","messageId":"7v1v267no9.fsf@alter.siamese.dyndns.org","threadId":"26756","inReplyTo":"AANLkTikZ3Po-YdhO-qCn5usVkt4J196eFF6YdbAeMG_X@mail.gmail.com","subject":"Re: git bisect code 125 - \"WFT?\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-16T21:36:38Z","receivedAt":"2011-03-16T21:36:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Piotr Krukowiecki writes:\n\n> Opinions? Would it be possible to change the meaning of the codes now\n> (in 1.8.0)?\n\nHow about just documenting why it is a bad idea to use 126 or 127 as you\nfound out somewhere, and stopping there, iow, without changing the code to\nuse 126/127 that we consider it is a bad idea to use and avoided using so\nfar?\n"},{"id":"163518","messageId":"AANLkTikRttGnxex1CYSQnSg4PgctFj0-qNjf5un+fL0W@mail.gmail.com","threadId":"26756","inReplyTo":"7v1v267no9.fsf@alter.siamese.dyndns.org","subject":"Re: git bisect code 125 - \"WFT?\"","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2011-03-16T22:06:32Z","receivedAt":"2011-03-16T22:06:32Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Wed, Mar 16, 2011 at 10:36 PM, Junio C Hamano wrote:\n> Piotr Krukowiecki writes:\n>\n>> Opinions? Would it be possible to change the meaning of the codes now\n>> (in 1.8.0)?\n>\n> How about just documenting why it is a bad idea to use 126 or 127 as you\n> found out somewhere, and stopping there, iow, without changing the code to\n> use 126/127 that we consider it is a bad idea to use and avoided using so\n> far?\n\nDocumenting it won't help. If you get 126 code, you won't know if user\nreturned it to mark the code as bad, or if bash returned it to say\nthat it can't\nexecute a command.\n\nOf course if changing the meaning is out of option it's better to document\nthen not to document.\n\n-- \nPiotr Krukowiecki\n"},{"id":"163530","messageId":"4D81B04A.1010802@viscovery.net","threadId":"26756","inReplyTo":"AANLkTikRttGnxex1CYSQnSg4PgctFj0-qNjf5un+fL0W@mail.gmail.com","subject":"Re: git bisect code 125 - \"WFT?\"","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-03-17T06:55:06Z","receivedAt":"2011-03-17T06:55:06Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/16/2011 23:06, schrieb Piotr Krukowiecki:\n> On Wed, Mar 16, 2011 at 10:36 PM, Junio C Hamano wrote:\n>> Piotr Krukowiecki writes:\n>>\n>>> Opinions? Would it be possible to change the meaning of the codes now\n>>> (in 1.8.0)?\n>>\n>> How about just documenting why it is a bad idea to use 126 or 127 as you\n>> found out somewhere, and stopping there, iow, without changing the code to\n>> use 126/127 that we consider it is a bad idea to use and avoided using so\n>> far?\n> \n> Documenting it won't help. If you get 126 code, you won't know if user\n> returned it to mark the code as bad, or if bash returned it to say\n> that it can't\n> execute a command.\n\nHuh? Why should the user's script return 126 or 127, particularly if the\ndocumentation says \"don't do that\"? Moreover, any decent (shell)\nprogrammer will know that these two values are reserved by POSIX for\nparticular purposes (they are _not_ specific to bash):\n\nhttp://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01_01\n\n-- Hannes\n"},{"id":"163535","messageId":"20110317072723.GH11931@sigill.intra.peff.net","threadId":"26756","inReplyTo":"4D81B04A.1010802@viscovery.net","subject":"Re: git bisect code 125 - \"WFT?\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-17T07:27:23Z","receivedAt":"2011-03-17T07:27:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 17, 2011 at 07:55:06AM +0100, Johannes Sixt wrote:\n\n> Am 3/16/2011 23:06, schrieb Piotr Krukowiecki:\n> > On Wed, Mar 16, 2011 at 10:36 PM, Junio C Hamano wrote:\n> >> Piotr Krukowiecki writes:\n> >>\n> >>> Opinions? Would it be possible to change the meaning of the codes now\n> >>> (in 1.8.0)?\n> >>\n> >> How about just documenting why it is a bad idea to use 126 or 127 as you\n> >> found out somewhere, and stopping there, iow, without changing the code to\n> >> use 126/127 that we consider it is a bad idea to use and avoided using so\n> >> far?\n> > \n> > Documenting it won't help. If you get 126 code, you won't know if user\n> > returned it to mark the code as bad, or if bash returned it to say\n> > that it can't\n> > execute a command.\n> \n> Huh? Why should the user's script return 126 or 127, particularly if the\n> documentation says \"don't do that\"? Moreover, any decent (shell)\n> programmer will know that these two values are reserved by POSIX for\n> particular purposes (they are _not_ specific to bash):\n> \n> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01_01\n\nI think the argument is not that the user would want to return those\ncodes, but that we can protect a poorly written test script from itself\nby including those exit codes in the list of \"indeterminate result\"\ncodes.\n\nIOW, currently this:\n\n git bisect run 'make && ./test-pogram'\n\nwill happily generate a bogus bisection. And obviously that's a trivial\nexample, but one can imagine a much larger script with a missing command\nin it.\n\nThere are two problems with that argument that I see, though:\n\n 1. It only protects some very specific cases, and they're not even\n interesting cases. Say your bisection runs a test script that looks\n like this:\n\n cmd1\n missing-cmd\n cmd2\n\n we _still_ won't see it as an indeterminate result, because the\n missing cmd's exit code is lost. So you would have to write:\n\n cmd1 &&\n missing-cmd &&\n cmd2\n\n but that doesn't really help much. If you are going to be that\n careful, what you really want is something like:\n\n cmd1 || exit 125\n missing-cmd || exit 125\n cmd2 || exit 125\n some-final-command-to-check-the-state\n\n So it can help, but I don't think it really helps in real-world\n cases.\n\n 2. If we do detect such a mishap, I'm not sure that \"indeterminate\n result\" is necessarily the best result, as that will just keep\n trying more and more commits without success. It is more likely a\n sign of a poorly written test script, and the best thing we could\n do is die and say \"your test script looks buggy\".\n\n-Peff\n"},{"id":"163567","messageId":"7vei654omv.fsf@alter.siamese.dyndns.org","threadId":"26756","inReplyTo":"20110317072723.GH11931@sigill.intra.peff.net","subject":"Re: git bisect code 125 - \"WFT?\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-17T17:56:24Z","receivedAt":"2011-03-17T17:56:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> 2. If we do detect such a mishap, I'm not sure that \"indeterminate\n> result\" is necessarily the best result, as that will just keep\n> trying more and more commits without success. It is more likely a\n> sign of a poorly written test script, and the best thing we could\n> do is die and say \"your test script looks buggy\".\n\nExactly. I agree \"Aborting the bisect as run-script is a crap\" is the\nright thing to do here.\n"},{"id":"163753","messageId":"4D84C960.3010401@gmail.com","threadId":"26756","inReplyTo":"7vei654omv.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/2] git bisect run: exit code 126 and 127 abort the run","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2011-03-19T15:18:56Z","receivedAt":"2011-03-19T15:18:56Z","isPatch":true,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"Those codes are reserved by POSIX to indicate\n\"command not executable\" and \"command not found\":\nhttp://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_02\n\nBisect used to treat them as codes returned by user\nto mark a \"bad\" code.\n\nWith that approach it was not possible to differentiate\nbetween codes returned by the user and codes returned by\nshell (which is likely a sign of a poorly written test\nscript).\n\nAnother minor problem was lack of consistency in exit codes.\nA \"bad\" code was marked by any value in range 1-124,126-127\nand the gap in the middle looked weird.\n\nChange the meaning of exit codes 126 and 127 to\n\"abort the bisect process\" to fixes the above problems.\n\nSigned-off-by: Piotr Krukowiecki \n---\n git-bisect.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\nW dniu 17.03.2011 18:56, Junio C Hamano pisze:\n> Jeff King writes:\n> \n>> 2. If we do detect such a mishap, I'm not sure that \"indeterminate\n>> result\" is necessarily the best result, as that will just keep\n>> trying more and more commits without success. It is more likely a\n>> sign of a poorly written test script, and the best thing we could\n>> do is die and say \"your test script looks buggy\".\n> \n> Exactly. I agree \"Aborting the bisect as run-script is a crap\" is the\n> right thing to do here.\n\nHere's a patch. Next one changes git-bisect.txt\n\nThere's also Documentation/git-bisect-lk2009.txt which talks about exit\ncodes and should be changed somehow. As I understand it's a quote from an\nemail so I don't know if it should be edited in place, or should a note\nbe added at beginning?\n\n\n(And again my patch has commit message longer than the changes...\n Now I understand why in-code documentation is lacking - after writing\n lengthy commit message documenting the code is just too much ;) )\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex c21e33c..9ca4852 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -376,9 +376,9 @@ bisect_run () {\n res=$?\n \n # Check for really bad run error.\n- if [ $res -lt 0 -o $res -ge 128 ]; then\n+ if [ $res -lt 0 -o $res -ge 126 ]; then\n \t echo >&2 \"bisect run failed:\"\n-\t echo >&2 \"exit code $res from '$@' is < 0 or >= 128\"\n+\t echo >&2 \"exit code $res from '$@' is < 0 or >= 126\"\n \t exit $res\n fi\n \n-- \n1.7.1\n\n-- \nPiotr Krukowiecki\n"},{"id":"163755","messageId":"4D84C9AF.2020900@gmail.com","threadId":"26756","inReplyTo":"7vei654omv.fsf@alter.siamese.dyndns.org","subject":"[PATCH 2/2] bisect documentation: exit codes 126,127 abort bisect","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2011-03-19T15:20:15Z","receivedAt":"2011-03-19T15:20:15Z","isPatch":true,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"Update documentation after meaning of those exit codes changed\nfrom \"mark as bad code\" to \"abort bisect run\".\n\nSigned-off-by: Piotr Krukowiecki \n---\n Documentation/git-bisect.txt | 11 +++++------\n 1 files changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex a1e47d6..70d8807 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -232,17 +232,16 @@ $ git bisect run my_script arguments\n \n Note that the script (`my_script` in the above example) should\n exit with code 0 if the current source code is good, and exit with a\n-code between 1 and 127 (inclusive), except 125, if the current\n-source code is bad.\n+code between 1 and 124 (inclusive) if the current source code is bad.\n+\n+Exit code 125 should be used when the current source code cannot be\n+tested. If the script exits with this code, the current revision will\n+be skipped (see `git bisect skip` above).\n \n Any other exit code will abort the bisect process. It should be noted\n that a program that terminates via \"exit(-1)\" leaves $? = 255, (see the\n exit(3) manual page), as the value is chopped with \"& 0377\".\n \n-The special exit code 125 should be used when the current source code\n-cannot be tested. If the script exits with this code, the current\n-revision will be skipped (see `git bisect skip` above).\n-\n You may often find that during a bisect session you want to have\n temporary modifications (e.g. s/#define DEBUG 0/#define DEBUG 1/ in a\n header file, or \"revision that does not have this commit needs this\n-- \n1.7.1\n\n-- \nPiotr Krukowiecki\n"}]}