Re: [RFC] Add bad-branch-first option for git-bisect

[Shuang He <shuang.he@intel.com>]

On 2011/1/25 17:20, Christian Couder wrote:
> On Mon, Jan 24, 2011 at 11:30 AM, Shuang He<shuang.he@intel.com>  wrote:
>> On 2011/1/24 17:53, Christian Couder wrote:
>>> Hi,
>>>
>>> On Mon, Jan 24, 2011 at 3:03 AM, Shuang He<shuang.he@intel.com>    wrote:
>>>> Hi
>>>>      The default git-bisect algorithm will jump around the commit tree,
>>>> on the purpose of taking least steps to find the first culprit commit.
>>>> We may find it sometime would locate a old culprit commit that we're not
>>>> concerned about anymore.
>>> Yes, it can be a problem.
>> I'm honored to be given so much comment :)
>> Thank you
> I am honored by your interest in git bisect and the fact that you
> provided a patch :-)
> Thanks!

I'm glad to see that git community is so hot.

>
>>> If the quality of these branches is too bad, I think they should not
>>> have been merged in the first place.
>>> If they are not merged (and not marked as good), then git bisect will
>>> not look at them, since it will look only at commits that are
>>> ancestors of the bad commit it is given.
>>>
>>> Or if one is merged but it causes too many problems, then perhaps a
>>> replacement commit could be used to unmerge the branch.
>>>
>>> Another possibility is to have in a file a list of commits that are
>>> the last commits on these branches before the merge commits, and do a:
>>>
>>> git bisect good $(cat good_commits_file.txt)
>>>
>>> at the beginning of each bisection.
>>>
>>> So I think the long term solution in this case is not what your are
>>> suggesting.
>> Yeah, I agree that the issue I addressed above will not be a problem if all
>> those branches are maintained very well.
>> Actually we've implemented a automated bisect system for Intel Linux
>> Graphics Driver Project, and so we'd like the system
>> helps us to locate issue in an more automatic way when branches are not
>> maintained as good as expected.
> I think there is always a price to pay when you bisect if the branches
> are not well maintained.
> Maybe your algorithm could help in some cases, but my opinion is that
> there will probably still be many problems and a human will often have
> to take a look.
>

Yes, I agree. What we trying to do is just make the machine to do more 
help for human.

>>>>          2. Some of those branches may not synchronized with main
>>>> branch in time.  Say feature1 is broken when feature2 branch is created,
>>>> and
>>>> feature1 is fixed just a moment later after feature2 branch is created,
>>>> and when feature2's development is done, and developer want to merge
>>>> feature2 branch back to master branch, feature2 will be firstly
>>>> synchronized to master branch tip, then merge into master.  For the same
>>>> reason addressed in issue 1, this will also lead git-bisect into wrong
>>>> direction.
>>> I am not sure what you mean by " feature2 will be firstly synchronized
>>> to master branch tip", and I think this should mean a rebase that
>>> would fix the bug if feature1 has already been merged into the master
>>> branch.
>>>
>>> But anyway in this case, I think that git bisect will find that the
>>> first bad commit is the last commit in the branch, just before it was
>>> merged. And by looking at the branch graph it should be quite easy to
>>> understand what happened.
> Now I think I was wrong here, as git bisect will probably find that
> the first commit in the branch (not the last one) is the first bad
> commit.
>
> [...]
>
>>> - the name "bisectbadbranchfirst" seems wrong to me, because git
>>> branches are just some special tags; "firstparentsonly" would be a
>>> better name,
>> It's recursively applying bad branch first algorithm, not just constantly
>> stick to first parent.
>> Given this condition:
>>     A ->  B ->  C ->  D ->  E ->  F ->  G ->  H   (master)
>>          \ a  ->  b ->  c ->  d ->  e /  (feature 1)
>>               \ x ->  y ->  z/      (feature 2)
>> start with H as bad commit, and A as good commit, if y is the target bad
>> commit. bad-branch-first algorithm will do it like this:
>>     1. In first round stick to master branch, so it will locate G as first
>> bad commit
>>     2. In second round stick to feature1 branch, then it will locate d as
>> first bad commit
>>     3. In third round stick to feature2 branch, then it will finally locate y
>> as first bad commit
>> So you could see, it's always sticking to branch where current bad commit
>> sit
> I see. It is interesting, but why not develop a "firstparentsonly"
> algorithm first?
>
> As Avery explains in his email, it is already interesting to have a
> "firstparentsonly" algorithm because some people are only interested
> to know from which branch the bug comes from.
> When they know that, they can just contact the relevant people and be
> done with it.
>
> And when we have a "firstparentsonly" algorithm, then your algorithm
> could be just a script that repeatedly uses git bisect with the
> "firstparentsonly" algorithm. And this script might be integrated in
> the "contrib" directory if it not considered important to be
> integrated as an algorithm into git bisect.

Sorry to reply so late, since I was on a long journey home for Chinese 
New Year vacation ;)
I agree that's also an good option.
Is it acceptable to add option to git-bisect stuff, so user could choose 
which algorithm to use at every step at will.
And we have tested previous attached patch with t6002-rev-list-bisect.sh 
and t6030-bisect-porcelain.sh, and we get:
     with bad-branch-first disabled (which is the default setting):
         t6002-rev-list-bisect.sh: # passed all 45 test(s)
         t6030-bisect-porcelain.sh: # passed all 40 test(s)
     and with bad-branch-first enabled:
         t6002-rev-list-bisect.sh: # passed all 45 test(s)
         t6030-bisect-porcelain.sh: # failed 5 among 40 test(s), and I 
have spent some time digging into those failures ,and it seems they're 
all false negative since they're using hard-coded bisect path to 
validate specific case

Thanks
     --Shuang
> Thanks,
> Christian.