# git bisect code 125 - "WFT?" 8 messages from 2011-03-16 to 2011-03-19. Participants: Piotr Krukowiecki, Junio C Hamano, Johannes Sixt, Jeff King. Thread: https://gitlist.dev/t/26756 ## Piotr Krukowiecki, 2011-03-16 20:44 Subject: git bisect code 125 - "WFT?" Message-ID: URL: https://gitlist.dev/e/AANLkTikZ3Po-YdhO-qCn5usVkt4J196eFF6YdbAeMG_X%40mail.gmail.com ``` Hi, in git bisect exit code 1-124 and 126-127 means "source is bad" (0 means "ok", 125 means "can't be tested" and anything other is "stop"). My first reaction was "WTF" - you have a special value in the middle of a range?? It got a bit clearer after reading the original patch[1] and man bash. It seems this value is last "free" value for the user to use, at least in bash - values above 125 may be used by the shell. Bash uses code 126 if command is not executable, and 127 if command is not found. I think it would be better to use 126 and 127 as either "can't be tested" or "stop" (125 should be left as is): * It would not leave a WTF gap in the "bad" range * If you get "command not executable" or "command not found" it's more probable something is broken than the code is bad, and you should fix your script Opinions? Would it be possible to change the meaning of the codes now (in 1.8.0)? [1] http://article.gmane.org/gmane.comp.version-control.git/62390 -- Piotr Krukowiecki ``` ## Junio C Hamano, 2011-03-16 21:36 Subject: Re: git bisect code 125 - "WFT?" Message-ID: <7v1v267no9.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v1v267no9.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Piotr Krukowiecki writes: > Opinions? Would it be possible to change the meaning of the codes now > (in 1.8.0)? How about just documenting why it is a bad idea to use 126 or 127 as you found out somewhere, and stopping there, iow, without changing the code to use 126/127 that we consider it is a bad idea to use and avoided using so far? ``` ## Piotr Krukowiecki, 2011-03-16 22:06 Subject: Re: git bisect code 125 - "WFT?" Message-ID: URL: https://gitlist.dev/e/AANLkTikRttGnxex1CYSQnSg4PgctFj0-qNjf5un%2BfL0W%40mail.gmail.com In-Reply-To: <7v1v267no9.fsf@alter.siamese.dyndns.org> ``` On Wed, Mar 16, 2011 at 10:36 PM, Junio C Hamano wrote: > Piotr Krukowiecki writes: > >> Opinions? Would it be possible to change the meaning of the codes now >> (in 1.8.0)? > > How about just documenting why it is a bad idea to use 126 or 127 as you > found out somewhere, and stopping there, iow, without changing the code to > use 126/127 that we consider it is a bad idea to use and avoided using so > far? Documenting it won't help. If you get 126 code, you won't know if user returned it to mark the code as bad, or if bash returned it to say that it can't execute a command. Of course if changing the meaning is out of option it's better to document then not to document. -- Piotr Krukowiecki ``` ## Johannes Sixt, 2011-03-17 06:55 Subject: Re: git bisect code 125 - "WFT?" Message-ID: <4D81B04A.1010802@viscovery.net> URL: https://gitlist.dev/e/4D81B04A.1010802%40viscovery.net In-Reply-To: ``` Am 3/16/2011 23:06, schrieb Piotr Krukowiecki: > On Wed, Mar 16, 2011 at 10:36 PM, Junio C Hamano wrote: >> Piotr Krukowiecki writes: >> >>> Opinions? Would it be possible to change the meaning of the codes now >>> (in 1.8.0)? >> >> How about just documenting why it is a bad idea to use 126 or 127 as you >> found out somewhere, and stopping there, iow, without changing the code to >> use 126/127 that we consider it is a bad idea to use and avoided using so >> far? > > Documenting it won't help. If you get 126 code, you won't know if user > returned it to mark the code as bad, or if bash returned it to say > that it can't > execute a command. Huh? Why should the user's script return 126 or 127, particularly if the documentation says "don't do that"? Moreover, any decent (shell) programmer will know that these two values are reserved by POSIX for particular purposes (they are _not_ specific to bash): http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01_01 -- Hannes ``` ## Jeff King, 2011-03-17 07:27 Subject: Re: git bisect code 125 - "WFT?" Message-ID: <20110317072723.GH11931@sigill.intra.peff.net> URL: https://gitlist.dev/e/20110317072723.GH11931%40sigill.intra.peff.net In-Reply-To: <4D81B04A.1010802@viscovery.net> ``` On Thu, Mar 17, 2011 at 07:55:06AM +0100, Johannes Sixt wrote: > Am 3/16/2011 23:06, schrieb Piotr Krukowiecki: > > On Wed, Mar 16, 2011 at 10:36 PM, Junio C Hamano wrote: > >> Piotr Krukowiecki writes: > >> > >>> Opinions? Would it be possible to change the meaning of the codes now > >>> (in 1.8.0)? > >> > >> How about just documenting why it is a bad idea to use 126 or 127 as you > >> found out somewhere, and stopping there, iow, without changing the code to > >> use 126/127 that we consider it is a bad idea to use and avoided using so > >> far? > > > > Documenting it won't help. If you get 126 code, you won't know if user > > returned it to mark the code as bad, or if bash returned it to say > > that it can't > > execute a command. > > Huh? Why should the user's script return 126 or 127, particularly if the > documentation says "don't do that"? Moreover, any decent (shell) > programmer will know that these two values are reserved by POSIX for > particular purposes (they are _not_ specific to bash): > > http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01_01 I think the argument is not that the user would want to return those codes, but that we can protect a poorly written test script from itself by including those exit codes in the list of "indeterminate result" codes. IOW, currently this: git bisect run 'make && ./test-pogram' will happily generate a bogus bisection. And obviously that's a trivial example, but one can imagine a much larger script with a missing command in it. There are two problems with that argument that I see, though: 1. It only protects some very specific cases, and they're not even interesting cases. Say your bisection runs a test script that looks like this: cmd1 missing-cmd cmd2 we _still_ won't see it as an indeterminate result, because the missing cmd's exit code is lost. So you would have to write: cmd1 && missing-cmd && cmd2 but that doesn't really help much. If you are going to be that careful, what you really want is something like: cmd1 || exit 125 missing-cmd || exit 125 cmd2 || exit 125 some-final-command-to-check-the-state So it can help, but I don't think it really helps in real-world cases. 2. If we do detect such a mishap, I'm not sure that "indeterminate result" is necessarily the best result, as that will just keep trying more and more commits without success. It is more likely a sign of a poorly written test script, and the best thing we could do is die and say "your test script looks buggy". -Peff ``` ## Junio C Hamano, 2011-03-17 17:56 Subject: Re: git bisect code 125 - "WFT?" Message-ID: <7vei654omv.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vei654omv.fsf%40alter.siamese.dyndns.org In-Reply-To: <20110317072723.GH11931@sigill.intra.peff.net> ``` Jeff King writes: > 2. If we do detect such a mishap, I'm not sure that "indeterminate > result" is necessarily the best result, as that will just keep > trying more and more commits without success. It is more likely a > sign of a poorly written test script, and the best thing we could > do is die and say "your test script looks buggy". Exactly. I agree "Aborting the bisect as run-script is a crap" is the right thing to do here. ``` ## Piotr Krukowiecki, 2011-03-19 15:18 Subject: [PATCH 1/2] git bisect run: exit code 126 and 127 abort the run Message-ID: <4D84C960.3010401@gmail.com> URL: https://gitlist.dev/e/4D84C960.3010401%40gmail.com In-Reply-To: <7vei654omv.fsf@alter.siamese.dyndns.org> ``` Those codes are reserved by POSIX to indicate "command not executable" and "command not found": http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_02 Bisect used to treat them as codes returned by user to mark a "bad" code. With that approach it was not possible to differentiate between codes returned by the user and codes returned by shell (which is likely a sign of a poorly written test script). Another minor problem was lack of consistency in exit codes. A "bad" code was marked by any value in range 1-124,126-127 and the gap in the middle looked weird. Change the meaning of exit codes 126 and 127 to "abort the bisect process" to fixes the above problems. Signed-off-by: Piotr Krukowiecki --- git-bisect.sh | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) W dniu 17.03.2011 18:56, Junio C Hamano pisze: > Jeff King writes: > >> 2. If we do detect such a mishap, I'm not sure that "indeterminate >> result" is necessarily the best result, as that will just keep >> trying more and more commits without success. It is more likely a >> sign of a poorly written test script, and the best thing we could >> do is die and say "your test script looks buggy". > > Exactly. I agree "Aborting the bisect as run-script is a crap" is the > right thing to do here. Here's a patch. Next one changes git-bisect.txt There's also Documentation/git-bisect-lk2009.txt which talks about exit codes and should be changed somehow. As I understand it's a quote from an email so I don't know if it should be edited in place, or should a note be added at beginning? (And again my patch has commit message longer than the changes... Now I understand why in-code documentation is lacking - after writing lengthy commit message documenting the code is just too much ;) ) diff --git a/git-bisect.sh b/git-bisect.sh index c21e33c..9ca4852 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -376,9 +376,9 @@ bisect_run () { res=$? # Check for really bad run error. - if [ $res -lt 0 -o $res -ge 128 ]; then + if [ $res -lt 0 -o $res -ge 126 ]; then echo >&2 "bisect run failed:" - echo >&2 "exit code $res from '$@' is < 0 or >= 128" + echo >&2 "exit code $res from '$@' is < 0 or >= 126" exit $res fi -- 1.7.1 -- Piotr Krukowiecki ``` ## Piotr Krukowiecki, 2011-03-19 15:20 Subject: [PATCH 2/2] bisect documentation: exit codes 126,127 abort bisect Message-ID: <4D84C9AF.2020900@gmail.com> URL: https://gitlist.dev/e/4D84C9AF.2020900%40gmail.com In-Reply-To: <7vei654omv.fsf@alter.siamese.dyndns.org> ``` Update documentation after meaning of those exit codes changed from "mark as bad code" to "abort bisect run". Signed-off-by: Piotr Krukowiecki --- Documentation/git-bisect.txt | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt index a1e47d6..70d8807 100644 --- a/Documentation/git-bisect.txt +++ b/Documentation/git-bisect.txt @@ -232,17 +232,16 @@ $ git bisect run my_script arguments Note that the script (`my_script` in the above example) should exit with code 0 if the current source code is good, and exit with a -code between 1 and 127 (inclusive), except 125, if the current -source code is bad. +code between 1 and 124 (inclusive) if the current source code is bad. + +Exit code 125 should be used when the current source code cannot be +tested. If the script exits with this code, the current revision will +be skipped (see `git bisect skip` above). Any other exit code will abort the bisect process. It should be noted that a program that terminates via "exit(-1)" leaves $? = 255, (see the exit(3) manual page), as the value is chopped with "& 0377". -The special exit code 125 should be used when the current source code -cannot be tested. If the script exits with this code, the current -revision will be skipped (see `git bisect skip` above). - You may often find that during a bisect session you want to have temporary modifications (e.g. s/#define DEBUG 0/#define DEBUG 1/ in a header file, or "revision that does not have this commit needs this -- 1.7.1 -- Piotr Krukowiecki ```