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

> phillip.wood123@gmail.com writes:
>
>>>> 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()?
>> Because you want the whole file, not just one line as the script name
>> could potentially contain a newline
> It is technically true, but it somehow sounds like an implausible
> scenario to me.  The real reason why read_file() is preferrable is
> because you do not have to write, and we do not want to see you write,
> the whole "open (and handle error), read, chomp, and return" sequence.
>
> I would even suspect that get_terms() is a poorly written
> anti-pattern.  If I were adding that function to the system today, I
> wouldn't be surprised if I did read_file() the whole thing and
> worked in-core to split two items out.

>
>> If I understand correctly we're encouraging the user to run "git
>> bisect skip" from the post checkout script. Doesn't that mean we'll
>> end up with a set of processes that look like
>>
>> 	- git bisect start
>> 	  - post checkout script
>>              - git bisect skip
>>                - post checkout script
>>                  - git bisect skip
>>                    ...
>>
>> as the "git bisect start" is waiting for the post checkout script to
>> finish running, but that script is waiting for "git bisect skip" to
>> finish running and so on. Each of those processes takes up system
>> resources, similar to how a recursive function can exhaust the
>> available stack space by calling itself over and over again.
> True.  What such a post-checkout script can do is to only mark the
> HEAD as "untestable", just like a run script given to "bisect run"
> signals that fact by returnint 125.  And at that point, I doubt it
> makes sense to add such a post-checkout script for the purpose of
> allowing "bisect skip".
>
> Having said that, a post-checkout script and pre-resume script may
> have a huge value in helping those whose tests cannot be automated
> (in other words, they cannot do "git bisect run") when they need to
> tweak the working tree during bisection.  We all have seen, during a
> bisection session that spans a segment of history that has another
> bug that affects our test *but* is orthogonal to the bug we are
> chasing, that we "cherry-pick --no-commit" the fix for that other
> problem inside "git bisect run" script.  It might look something
> like
>
>      #!/bin/sh
>      if git merge-base --is-ancestor $the_other_bug HEAD
>      then
> 	# we need the fix
> 	git cherry-pick --no-commit $fix_for_the_other_bug ||
> 	exit 125
>      fi
>
>      make test
>      status=$?
>      git reset --hard ;# undo the cherry-pick
>      exit $status
>
> But to those whose test is not a good match to "git bisect run", if
> we had a mechanism to tweak the checked out working tree after the
> "bisect next" (which is an internal mechanism that "bisect good",
> "bisect bad", and "bisect skip" share to give you the next HEAD and
> the working tree to test) checks out the working tree before it
> gives the control back to you, we could split the above script into
> two parts and throw the "conditionally cherry-pick the fix" part
> into that mechanism.  We'd need to have a companion script to "redo
> the damage" (the "reset --hard" in the above illustration) if this
> were to work seamlessly.  That obviously is totally orthogonal to
> what we are discussing in this thread, but may make a good #leftoverbits
> material (but not for novices).