Re: Git bisect run should check for the existence of the script

[Junio C Hamano <gitster@pobox.com>]

Jean Abou Samra <jean@abou-samra.fr> writes:

> Le 15/07/2020 à 16:55, Junio C Hamano a écrit :
>> Jean Abou Samra <jean@abou-samra.fr> writes:
>>
>>> $ git bisect run ./non-existent.sh
>>> running ./non-existent.sh
>>> /usr/lib/git-core/git-bisect: 247: ./non-existent.sh: not found
> ...
>> On the other hand, the "./non-existent.sh" script could be part of
>> the tracked contents (i.e. some revisions have it and the working
>> tree has it when they get checked out, some revisions don't and the
>> working tree does not have it), and the user is trying to find the
>> first revision that stopped having a working script in its tree.  In
>> such a case, the script that does not exist and the script that fails
>> need to be treated the same way by "git bisect run" as failures.
>>
>> So... I dunno.
>
> Perhapsa --not-found-as-failure option could help?

Do you mean a new option must be passed if the end-user expects the
script to always exist across revisions, or the script is not
tracked to begin with?  It feels somewhat backwards and the effort
by the end-user to always type the option is better spent to make
sure there is no typo on the command line.

Besides, we need to take into account that "bisect run" takes an
arbitrary shell snippet, e.g. the user may try to switch between two
scripts, test-1 and test-2, based on some condition, and wrote this:

  $ git bisect run "if some-condition; then ./test-1; else ./test-3; fi"

but made a typo in the name of test-2.  It would be noticed after
attempting to run the "if ... fi" as a scriptlet by the returned
status from it being 127 (command not found), which is not treated
any specially by "git bisect run".  So a typo in the script name in
the above example cannot be distinguished from this error where the
user wanted to run test-1 under some condition, but wanted to
declare that the revision is bad if that condition did not hold,
i.e.

  $ git bisect run "if some-condition; then ./test-1; else (exit 127); fi"

Even if we somehow could tell these two apart, we cannot pick the
first scriptlet apart and guess which substring in it were meant as
a filename with a typo.

It may be possible to forbid the general use of exit code 127 and
instead reserve it to signal "we know that the test script is so
broken (this includes the case where it does not even exist) that
using it with 'bisect run' is meaningless; please stop immediately."
and nothing else, just like exit code 125 is reserved for "this
revision is not testable and cannot say good or bad", but making
such a change retroactively is not to be done very lightly.

So, again, I dunno.

 git-bisect.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 7a8f796251..5c03bf8533 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -122,7 +122,7 @@ bisect_run () {
 		res=$?
 
 		# Check for really bad run error.
-		if [ $res -lt 0 -o $res -ge 128 ]
+		if [ $res -lt 0 -o $res -ge 128 -o $res -eq 127 ]
 		then
 			eval_gettextln "bisect run failed:
 exit code \$res from '\$command' is < 0 or >= 128" >&2