Re: git bisect bad @

From: René Scharfe <l.s.r@web.de>
Date: Jan 13, 2022, 13:55 UTC
Message-ID: <8bc73981-589e-20e5-247b-2f74e166ae1a@web.de>
In-Reply-To: <CAP8UFD3tyBhrOQzg9j4qDAT0Tb8TCTK0=J6ORsiLVuMWn+W9wg@mail.gmail.com>

Am 13.01.22 um 13:28 schrieb Christian Couder:

Show 10 quoted lines

> On Thu, Jan 13, 2022 at 10:32 AM Ramkumar Ramachandra
> <r@artagnon.com> wrote:
>>
>> René Scharfe wrote:
>
>>> Reserving 126 and 127 shouldn't cause too much trouble,
>
> I don't think it's a good idea at this point to reserve the 126 and
> 127 error codes as there might be existing scripts relying on them
> to mean "bad".

Certainly possible -- people get the weirdest ideas.

Show 5 quoted lines

> Perhaps we could introduce a new command line option, for example
> --bad-is-only-1, to specify that the only error code considered bad
> will be 1. Or perhaps a more general --bad-is=<list of ranges>, to
> be able to specify all the values and ranges that should be
> considered bad.

This would only help someone who mistyped the script name or forgot to make it executable if they also used that option. I can't imagine someone planning their mistakes ahead like that. And always using this option would be annoying.

Show 9 quoted lines

>>> but there's also a way to avoid it: bisect run could checkout a
>>> known-good revision first and abort if the script returns
>>> non-zero for any reason, including its non-existence.
>>
>> I can't say I'm overly enthusiastic about this trade-off. I think
>> most people would check their bisect scripts against the good
>> revision by hand before starting bisect: why introduce one
>> redundant step for users like me who tend to bump their heads,
>> because they're a bit rusty with machines?

It would slow the normal operation a bit, but reduce the time to error and its impact significantly.

> I also don't like introducing a redundant step, unless a special
> command line option is introduced for it.

My comment regarding --is-bad applies here as well.

OK, here's another idea: We verify using a known-good commit only if the return code of the first run of the bisect script is 126 or 127. If we get the same value again then we report the script as broken and leave the bisect state unchanged. Otherwise we know it's an old school script, register the first revision as bad and continue without further verification steps.

Show 8 quoted lines

>> Again, I don't know if this is a good idea, but if exit codes from
>> the shell aren't standardized, surely fork() and exec() would have
>> a better spec? So, perhaps remove the little git-bisect.sh and
>> rewrite it in C? I'd be up for this task, if we decide that this is
>> a better way to go.
>
> There has been a lot of effort, especially by Miriam (added in Cc),
> to port git-bisect.sh to C over the years.

The implementation language of git bisect is not immediately relevant here, but that the shell is used to call the user-supplied bisect run script is. If we'd run it directly (without RUN_USING_SHELL) we could distinguish error code 126/127 from execution errors. I assume the option is used to stay compatible with the old shell version of bisect.

René