On Wed, Jan 26, 2011 at 8:22 AM, Shuang He <shuang.he@xxxxxxxxx> wrote: > On 2011/1/25 17:20, Christian Couder wrote: >> >>> >>> 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. Yeah, this is the way to go. And by the way I am happy to know that you have implemented an automated bisect system. That's great and I hope it already helps. >>>> - 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 ;) No problem. I am not in a hurry at all. In fact I don't have much time these days so I reply very late too. > 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. Are you sure it is needed to be able to change the algorithm at every step? This means that you would like a new "git bisect strategy <strategy>" subcommand ? First I thought that we could just add a "--strategy <strategy>" option to "git bisect start". But anyway, I think it should be easy to add afterward, and it can be done in a separated patch that can be discussed on its own. > 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 Yes, there are some hard coded commits that depend on the algorithm. Anyway I did not look in depth at your patch yet, and as I said it would be better if you could split it into a patch series where a "firstparentsonly" algorithm is implemented first. This way it will be easier to review, and we can start to integrate some non controversial features, and then discuss the other ones on their own merit. Thanks in advance, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html