Le vendredi 11 juillet 2008, Christian Couder a écrit : > Le jeudi 10 juillet 2008, Junio C Hamano a écrit : > > Christian Couder <chriscool@xxxxxxxxxxxxx> writes: > > > Yeah, in that case... > > > > > >> The whole idea of "bisect" relies on that idea, that any ancestor of > > >> a good commit is good. Otherwise you'd have to check the commits > > >> one by one, not in a bisecting manner. > > > > Didn't we already discuss this at length? > > Yes, the thread is there: > > http://thread.gmane.org/gmane.comp.version-control.git/86951 > > > > No, you just need to check that the merge bases between the bad rev > > > on one side and each good rev on the other side are good too. And if > > > that is the case, then you can be sure that bisection will point to a > > > first bad commit. > > > > > > So the choice is between a simple and fast but not fully reliable > > > bisect, or a more complex and slower but fully reliable bisect. > > > > I have not looked at your implementation, but I do think: > > > > - The current one is not "fully reliable"; the user needs to know what > > he is doing. You might call it "prone to user errors". > > I agree. > > > - "Test this merge-base before going forward, please" will add > > typically only one round of check (if you have more merge bases between > > good and bad, you need to test all of them are good to be sure), so it > > is not "slower nor more complex". > > By "slower" I meant that it would need more rounds of check on average. > By "more complex" I meant that more code is needed. > > And I think you are right, all the merge bases need to be tested so I > will send a patch on top of the patch discussed here. Ok, here is an untested patch that should check all merge bases. I don't have time right now to add tests and a good commit message but I will do that when I come back from vacancy in about one week. So please consider it as just a RFC. Thanks, Christian. -----8<------------ [PATCH] bisect: check all merge bases instead of only one Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx> --- git-bisect.sh | 29 +++++++++++++++-------------- 1 files changed, 15 insertions(+), 14 deletions(-) diff --git a/git-bisect.sh b/git-bisect.sh index 50f912f..ca16609 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -375,7 +375,6 @@ Warning: the merge base between $_bad and $_g must be skipped. So we cannot be sure the first bad commit is between $_mb and $_bad. We continue anyway. EOF - mark_merge_base_ok "$_bad" "$_g" } check_merge_bases() { @@ -384,19 +383,21 @@ check_merge_bases() { _skip="$3" for _g in $_good; do is_merge_base_ok "$_bad" "$_g" && continue - _mb=$(git merge-base $_g $_bad) - if test "$_mb" = "$_g" || is_among "$_mb" "$_good"; then - mark_merge_base_ok "$_bad" "$_g" - elif test "$_mb" = "$_bad"; then - handle_bad_merge_base "$_bad" "$_g" - elif is_among "$_mb" "$_skip"; then - handle_skipped_merge_base "$_bad" "$_g" "_mb" - else - mark_testing_merge_base "$_mb" - checkout "$_mb" "a merge base must be tested" - checkout_done=1 - break - fi + for _mb in $(git merge-base --all $_g $_bad); do + if test "$_mb" = "$_g" || is_among "$_mb" "$_good"; then + continue + elif test "$_mb" = "$_bad"; then + handle_bad_merge_base "$_bad" "$_g" + elif is_among "$_mb" "$_skip"; then + handle_skipped_merge_base "$_bad" "$_g" "_mb" + else + mark_testing_merge_base "$_mb" + checkout "$_mb" "a merge base must be tested" + checkout_done=1 + return + fi + done + mark_merge_base_ok "$_bad" "$_g" done } -- 1.5.6.2.221.gf54e0.dirty -- 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
