Re: [RFC] bisect: Introduce skip-when to automatically skip commits

[Olliver Schinagl <oliver@schinagl.nl>]

Hey Phillip,

On 07-04-2024 16:09, phillip.wood123@gmail.com wrote:
> On 06/04/2024 20:17, Olliver Schinagl wrote:
>> Hey Phillip,
>>
>> On 06-04-2024 15:50, Phillip Wood wrote:
>>> Hi Olliver
>>>
>>> On 06/04/2024 11:06, Olliver Schinagl wrote:
>>>> On 06-04-2024 03:08, Junio C Hamano wrote:
>>>>> Olliver Schinagl <oliver@schinagl.nl> writes:
>>> If you search builtin/bisect.c you'll see some existing callers of 
>>> strbuf_read_file() that read other files like BISECT_START. Those 
>>> callers should give you an idea of how to use it.
>>
>> Yeah, I found after Junio's hint :) What threw me off, as I wrote 
>> earlier, get_terms(). I wonder now, why is get_terms() implemented as 
>> it is, and should it not use the same functions? Or is it because 
>> terms is a multi-line file, whereas the others are all single line (I 
>> didn't look, though I see addline functions for the strbuf functions. 
>> Should this be refactored?
> 
> get_terms() wants to read the first line into `term_bad` and the second 
> line into `term_good` so it makes sense that it uses two calls to 
> `strbuf_getline()` to do that. It does not want to read the whole file 
> into a single buffer as we do here.

Right, but I why not use strbuf_getline()?

> 
>> So with the name, I started to think some more about it, and after 
>> playing with some names, I settled on 'bisect-post-checkout'. Things 
>> then sort of fell more into place. It is still a hook/commandline 
>> option, but it's a much smaller change (since we don't have any 
>> special code to check the exit code anymore) as we can (obviously) run 
>> `git bisect skip` instead of `exit 125` as well of course.
> 
> Does that mean you will be starting "git bisect skip" from the script 
> run by the current "git bisect" process. I don't think calling git 
> recursively like that is a good idea as you'll potentially end up with a 
> bunch of "git bisect" processes all waiting for their post checkout 
> script to finish running.

Well the process is inherently recursive, though that's up to the user 
depending on what they put in their script of course. I don't think git 
is 'waiting' is it? In that, git bisect runs the command, the command 
runs git bisect, git bisect stores the commit hash in the skip file and 
'exists', which goes then back to the bisect job, which then continues 
as it normally would.

So technically, we're not doing anything bad in git, but a user might do 
something bad.

Thank you,

Olliver

> 
> Best Wishes
> 
> Phillip