git bisect code 125 - "WFT?"

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

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

Am 3/16/2011 23:06, schrieb Piotr Krukowiecki:

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

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

Exactly. I agree "Aborting the bisect as run-script is a crap" is the right thing to do here.

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.

---
 git-bisect.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

W dniu 17.03.2011 18:56, Junio C Hamano pisze:

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 ;) )

index c21e33c..9ca4852 100755
@@ -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

Update documentation after meaning of those exit codes changed from "mark as bad code" to "abort bisect run".

---
 Documentation/git-bisect.txt |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

index a1e47d6..70d8807 100644
@@ -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