@matheusbsbs / matheus-bagliano / commits / c9c4e2d5a2
Fork 0

bisect: only check merge bases when needed

When one good revision is not an ancestor of the bad revision, the merge bases between the good and the bad revision should be checked to make sure that they are also good revisions. A previous patch takes care of that, but it may check the merge bases more often than really needed. In fact the previous patch did not try to optimize this as much as possible because it is not so simple. So this is the purpose of this patch. One may think that when all the merge bases have been checked then we can save a flag, so that we don't need to check the merge bases again during the bisect process. The problem is that the user may choose to checkout and test something completely different from what the bisect process suggested. In this case we have to check the merge bases again, because there may be new merge bases relevant to the bisect process. That's why, in this patch, when we detect that the user tested something else than what the bisect process suggested, we remove the flag that says that we don't need to check the merge bases again. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Christian Couder committed Aug 22, 2008 at 05:52 UTC c9c4e2d5a25bbf637780f9b83e74c2a26fb957f5
2 files changed +58 -14
git-bisect.sh
+34 -14
@@ -172,6 +172,25 @@ bisect_write() {
172 test -n "$nolog" || echo "git bisect $state $rev" >>"$GIT_DIR/BISECT_LOG"
173 }
174
175 +is_expected_rev() {
176 + test -f "$GIT_DIR/BISECT_EXPECTED_REV" &&
177 + test "$1" = $(cat "$GIT_DIR/BISECT_EXPECTED_REV")
178 +}
179 +
180 +mark_expected_rev() {
181 + echo "$1" > "$GIT_DIR/BISECT_EXPECTED_REV"
182 +}
183 +
184 +check_expected_revs() {
185 + for _rev in "$@"; do
186 + if ! is_expected_rev "$_rev"; then
187 + rm -f "$GIT_DIR/BISECT_ANCESTORS_OK"
188 + rm -f "$GIT_DIR/BISECT_EXPECTED_REV"
189 + return
190 + fi
191 + done
192 +}
193 +
194 bisect_state() {
195 bisect_autostart
196 state=$1
@@ -181,7 +200,8 @@ bisect_state() {
200 1,bad|1,good|1,skip)
201 rev=$(git rev-parse --verify HEAD) ||
202 die "Bad rev input: HEAD"
184 - bisect_write "$state" "$rev" ;;
203 + bisect_write "$state" "$rev"
204 + check_expected_revs "$rev" ;;
205 2,bad|*,good|*,skip)
206 shift
207 eval=''
@@ -191,7 +211,8 @@ bisect_state() {
211 die "Bad rev input: $rev"
212 eval="$eval bisect_write '$state' '$sha'; "
213 done
194 - eval "$eval" ;;
214 + eval "$eval"
215 + check_expected_revs "$@" ;;
216 *,bad)
217 die "'git bisect bad' can take only one argument." ;;
218 *)
@@ -321,6 +342,7 @@ bisect_checkout() {
342 _rev="$1"
343 _msg="$2"
344 echo "Bisecting: $_msg"
345 + mark_expected_rev "$_rev"
346 git checkout -q "$_rev" || exit
347 git show-branch "$_rev"
348 }
@@ -332,18 +354,10 @@ is_among() {
354 return 1
355 }
356
335 -is_testing_merge_base() {
336 - grep "^testing $1$" "$GIT_DIR/BISECT_MERGE_BASES" >/dev/null 2>&1
337 -}
338 -
339 -mark_testing_merge_base() {
340 - echo "testing $1" >> "$GIT_DIR/BISECT_MERGE_BASES"
341 -}
342 -
357 handle_bad_merge_base() {
358 _badmb="$1"
359 _good="$2"
346 - if is_testing_merge_base "$_badmb"; then
360 + if is_expected_rev "$_badmb"; then
361 cat >&2 <<EOF
362 The merge base $_badmb is bad.
363 This means the bug has been fixed between $_badmb and [$_good].
@@ -383,7 +397,6 @@ check_merge_bases() {
397 elif is_among "$_mb" "$_skip"; then
398 handle_skipped_merge_base "$_mb" "$_bad" "$_good"
399 else
386 - mark_testing_merge_base "$_mb"
400 bisect_checkout "$_mb" "a merge base must be tested"
401 checkout_done=1
402 return
@@ -392,6 +405,9 @@ check_merge_bases() {
405 }
406
407 check_good_are_ancestors_of_bad() {
408 + test -f "$GIT_DIR/BISECT_ANCESTORS_OK" &&
409 + return
410 +
411 _bad="$1"
412 _good=$(echo $2 | sed -e 's/\^//g')
413 _skip="$3"
@@ -401,8 +417,11 @@ check_good_are_ancestors_of_bad() {
417
418 _side=$(git rev-list $_good ^$_bad)
419 if test -n "$_side"; then
404 - check_merge_bases "$_bad" "$_good" "$_skip"
420 + check_merge_bases "$_bad" "$_good" "$_skip" || return
421 + test "$checkout_done" -eq "1" && return
422 fi
423 +
424 + : > "$GIT_DIR/BISECT_ANCESTORS_OK"
425 }
426
427 bisect_next() {
@@ -491,7 +510,8 @@ bisect_clean_state() {
510 do
511 git update-ref -d $ref $hash || exit
512 done
494 - rm -f "$GIT_DIR/BISECT_MERGE_BASES" &&
513 + rm -f "$GIT_DIR/BISECT_EXPECTED_REV" &&
514 + rm -f "$GIT_DIR/BISECT_ANCESTORS_OK" &&
515 rm -f "$GIT_DIR/BISECT_LOG" &&
516 rm -f "$GIT_DIR/BISECT_NAMES" &&
517 rm -f "$GIT_DIR/BISECT_RUN" &&
t/t6030-bisect-porcelain.sh
+24
@@ -440,6 +440,30 @@ test_expect_success 'good merge bases when good and bad are siblings' '
440 git bisect reset
441 '
442
443 +check_trace() {
444 + grep "$1" "$GIT_TRACE" | grep "\^$2" | grep "$3" >/dev/null
445 +}
446 +
447 +test_expect_success 'optimized merge base checks' '
448 + GIT_TRACE="$(pwd)/trace.log" &&
449 + export GIT_TRACE &&
450 + git bisect start "$HASH7" "$SIDE_HASH7" > my_bisect_log.txt &&
451 + grep "merge base must be tested" my_bisect_log.txt &&
452 + grep "$HASH4" my_bisect_log.txt &&
453 + check_trace "rev-list" "$HASH7" "$SIDE_HASH7" &&
454 + git bisect good > my_bisect_log2.txt &&
455 + test -f ".git/BISECT_ANCESTORS_OK" &&
456 + test "$HASH6" = $(git rev-parse --verify HEAD) &&
457 + : > "$GIT_TRACE" &&
458 + git bisect bad > my_bisect_log3.txt &&
459 + test_must_fail check_trace "rev-list" "$HASH6" "$SIDE_HASH7" &&
460 + git bisect good "$A_HASH" > my_bisect_log4.txt &&
461 + grep "merge base must be tested" my_bisect_log4.txt &&
462 + test_must_fail test -f ".git/BISECT_ANCESTORS_OK" &&
463 + check_trace "rev-list" "$HASH6" "$A_HASH" &&
464 + unset GIT_TRACE
465 +'
466 +
467 #
468 #
469 test_done