Re: [RFC] Add bad-branch-first option for git-bisect
- From
- Shuang He <shuang.he@intel.com>
- Date
- Jan 26, 2011, 07:22 UTC
- Message-ID
- <4D3FCBB3.2090508@intel.com>
- In-Reply-To
- <AANLkTin1rS-ZBDx4j-UNFH4z9tnTiv5LBodLO-G2U2UF@mail.gmail.com>
On 2011/1/25 17:20, Christian Couder wrote:
Show 16 quoted lines
> 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.
Show 31 quoted lines
> >>> 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.
Show 57 quoted lines
>>>> 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 caseThanks
--Shuang> Thanks, > Christian.