Skip to content

Commit cb2c7da

Browse files
gitstergitster
committed
Merge branch 'cc/bisect'
* cc/bisect: bisect: remove "checkout_done" variable used when checking merge bases bisect: only check merge bases when needed bisect: test merge base if good rev is not an ancestor of bad rev
2 parents 4c048e3 + 6a54d97 commit cb2c7da

File tree

2 files changed

+260
-27
lines changed

2 files changed

+260
-27
lines changed

‎git-bisect.sh‎

Lines changed: 146 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,25 @@ bisect_write() {
172172
test -n "$nolog" || echo "git bisect $state $rev" >>"$GIT_DIR/BISECT_LOG"
173173
}
174174

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+
175194
bisect_state() {
176195
bisect_autostart
177196
state=$1
@@ -181,7 +200,8 @@ bisect_state() {
181200
1,bad|1,good|1,skip)
182201
rev=$(git rev-parse --verify HEAD) ||
183202
die "Bad rev input: HEAD"
184-
bisect_write "$state" "$rev" ;;
203+
bisect_write "$state" "$rev"
204+
check_expected_revs "$rev" ;;
185205
2,bad|*,good|*,skip)
186206
shift
187207
eval=''
@@ -191,7 +211,8 @@ bisect_state() {
191211
die "Bad rev input: $rev"
192212
eval="$eval bisect_write '$state' '$sha'; "
193213
done
194-
eval "$eval" ;;
214+
eval "$eval"
215+
check_expected_revs "$@" ;;
195216
*,bad)
196217
die "'git bisect bad' can take only one argument." ;;
197218
*)
@@ -243,33 +264,18 @@ bisect_auto_next() {
243264
bisect_next_check && bisect_next || :
244265
}
245266

246-
eval_rev_list() {
247-
_eval="$1"
248-
249-
eval $_eval
250-
res=$?
251-
252-
if [ $res -ne 0 ]; then
253-
echo >&2 "'git rev-list --bisect-vars' failed:"
254-
echo >&2 "maybe you mistake good and bad revs?"
255-
exit $res
256-
fi
257-
258-
return $res
259-
}
260-
261267
filter_skipped() {
262268
_eval="$1"
263269
_skip="$2"
264270

265271
if [ -z "$_skip" ]; then
266-
eval_rev_list "$_eval"
272+
eval "$_eval"
267273
return
268274
fi
269275

270276
# Let's parse the output of:
271277
# "git rev-list --bisect-vars --bisect-all ..."
272-
eval_rev_list "$_eval" | while read hash line
278+
eval "$_eval" | while read hash line
273279
do
274280
case "$VARS,$FOUND,$TRIED,$hash" in
275281
# We display some vars.
@@ -332,20 +338,133 @@ exit_if_skipped_commits () {
332338
fi
333339
}
334340

341+
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+
}
349+
350+
is_among() {
351+
_rev="$1"
352+
_list="$2"
353+
case "$_list" in *$_rev*) return 0 ;; esac
354+
return 1
355+
}
356+
357+
handle_bad_merge_base() {
358+
_badmb="$1"
359+
_good="$2"
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].
364+
EOF
365+
exit 3
366+
else
367+
cat >&2 <<EOF
368+
Some good revs are not ancestor of the bad rev.
369+
git bisect cannot work properly in this case.
370+
Maybe you mistake good and bad revs?
371+
EOF
372+
exit 1
373+
fi
374+
}
375+
376+
handle_skipped_merge_base() {
377+
_mb="$1"
378+
_bad="$2"
379+
_good="$3"
380+
cat >&2 <<EOF
381+
Warning: the merge base between $_bad and [$_good] must be skipped.
382+
So we cannot be sure the first bad commit is between $_mb and $_bad.
383+
We continue anyway.
384+
EOF
385+
}
386+
387+
#
388+
# "check_merge_bases" checks that merge bases are not "bad".
389+
#
390+
# - If one is "good", that's good, we have nothing to do.
391+
# - If one is "bad", it means the user assumed something wrong
392+
# and we must exit.
393+
# - If one is "skipped", we can't know but we should warn.
394+
# - If we don't know, we should check it out and ask the user to test.
395+
#
396+
# In the last case we will return 1, and otherwise 0.
397+
#
398+
check_merge_bases() {
399+
_bad="$1"
400+
_good="$2"
401+
_skip="$3"
402+
for _mb in $(git merge-base --all $_bad $_good)
403+
do
404+
if is_among "$_mb" "$_good"; then
405+
continue
406+
elif test "$_mb" = "$_bad"; then
407+
handle_bad_merge_base "$_bad" "$_good"
408+
elif is_among "$_mb" "$_skip"; then
409+
handle_skipped_merge_base "$_mb" "$_bad" "$_good"
410+
else
411+
bisect_checkout "$_mb" "a merge base must be tested"
412+
return 1
413+
fi
414+
done
415+
return 0
416+
}
417+
418+
#
419+
# "check_good_are_ancestors_of_bad" checks that all "good" revs are
420+
# ancestor of the "bad" rev.
421+
#
422+
# If that's not the case, we need to check the merge bases.
423+
# If a merge base must be tested by the user we return 1 and
424+
# otherwise 0.
425+
#
426+
check_good_are_ancestors_of_bad() {
427+
test -f "$GIT_DIR/BISECT_ANCESTORS_OK" &&
428+
return
429+
430+
_bad="$1"
431+
_good=$(echo $2 | sed -e 's/\^//g')
432+
_skip="$3"
433+
434+
# Bisecting with no good rev is ok
435+
test -z "$_good" && return
436+
437+
_side=$(git rev-list $_good ^$_bad)
438+
if test -n "$_side"; then
439+
# Return if a checkout was done
440+
check_merge_bases "$_bad" "$_good" "$_skip" || return
441+
fi
442+
443+
: > "$GIT_DIR/BISECT_ANCESTORS_OK"
444+
445+
return 0
446+
}
447+
335448
bisect_next() {
336449
case "$#" in 0) ;; *) usage ;; esac
337450
bisect_autostart
338451
bisect_next_check good
339452

453+
# Get bad, good and skipped revs
454+
bad=$(git rev-parse --verify refs/bisect/bad) &&
455+
good=$(git for-each-ref --format='^%(objectname)' \
456+
"refs/bisect/good-*" | tr '\012' ' ') &&
340457
skip=$(git for-each-ref --format='%(objectname)' \
341-
"refs/bisect/skip-*" | tr '\012' ' ') || exit
458+
"refs/bisect/skip-*" | tr '\012' ' ') &&
459+
460+
# Maybe some merge bases must be tested first
461+
check_good_are_ancestors_of_bad "$bad" "$good" "$skip"
462+
# Return now if a checkout has already been done
463+
test "$?" -eq "1" && return
342464

465+
# Get bisection information
343466
BISECT_OPT=''
344467
test -n "$skip" && BISECT_OPT='--bisect-all'
345-
346-
bad=$(git rev-parse --verify refs/bisect/bad) &&
347-
good=$(git for-each-ref --format='^%(objectname)' \
348-
"refs/bisect/good-*" | tr '\012' ' ') &&
349468
eval="git rev-list --bisect-vars $BISECT_OPT $good $bad --" &&
350469
eval="$eval $(cat "$GIT_DIR/BISECT_NAMES")" &&
351470
eval=$(filter_skipped "$eval" "$skip") &&
@@ -366,9 +485,7 @@ bisect_next() {
366485
# commit is also a "skip" commit (see above).
367486
exit_if_skipped_commits "$bisect_rev"
368487

369-
echo "Bisecting: $bisect_nr revisions left to test after this"
370-
git checkout -q "$bisect_rev" || exit
371-
git show-branch "$bisect_rev"
488+
bisect_checkout "$bisect_rev" "$bisect_nr revisions left to test after this"
372489
}
373490

374491
bisect_visualize() {
@@ -415,6 +532,8 @@ bisect_clean_state() {
415532
do
416533
git update-ref -d $ref $hash || exit
417534
done
535+
rm -f "$GIT_DIR/BISECT_EXPECTED_REV" &&
536+
rm -f "$GIT_DIR/BISECT_ANCESTORS_OK" &&
418537
rm -f "$GIT_DIR/BISECT_LOG" &&
419538
rm -f "$GIT_DIR/BISECT_NAMES" &&
420539
rm -f "$GIT_DIR/BISECT_RUN" &&

‎t/t6030-bisect-porcelain.sh‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,120 @@ test_expect_success 'bisect does not create a "bisect" branch' '
350350
git branch -D bisect
351351
'
352352

353+
# This creates a "side" branch to test "siblings" cases.
354+
#
355+
# H1-H2-H3-H4-H5-H6-H7 <--other
356+
# \
357+
# S5-S6-S7 <--side
358+
#
359+
test_expect_success 'side branch creation' '
360+
git bisect reset &&
361+
git checkout -b side $HASH4 &&
362+
add_line_into_file "5(side): first line on a side branch" hello2 &&
363+
SIDE_HASH5=$(git rev-parse --verify HEAD) &&
364+
add_line_into_file "6(side): second line on a side branch" hello2 &&
365+
SIDE_HASH6=$(git rev-parse --verify HEAD) &&
366+
add_line_into_file "7(side): third line on a side branch" hello2 &&
367+
SIDE_HASH7=$(git rev-parse --verify HEAD)
368+
'
369+
370+
test_expect_success 'good merge base when good and bad are siblings' '
371+
git bisect start "$HASH7" "$SIDE_HASH7" > my_bisect_log.txt &&
372+
grep "merge base must be tested" my_bisect_log.txt &&
373+
grep $HASH4 my_bisect_log.txt &&
374+
git bisect good > my_bisect_log.txt &&
375+
test_must_fail grep "merge base must be tested" my_bisect_log.txt &&
376+
grep $HASH6 my_bisect_log.txt &&
377+
git bisect reset
378+
'
379+
test_expect_success 'skipped merge base when good and bad are siblings' '
380+
git bisect start "$SIDE_HASH7" "$HASH7" > my_bisect_log.txt &&
381+
grep "merge base must be tested" my_bisect_log.txt &&
382+
grep $HASH4 my_bisect_log.txt &&
383+
git bisect skip > my_bisect_log.txt 2>&1 &&
384+
grep "Warning" my_bisect_log.txt &&
385+
grep $SIDE_HASH6 my_bisect_log.txt &&
386+
git bisect reset
387+
'
388+
389+
test_expect_success 'bad merge base when good and bad are siblings' '
390+
git bisect start "$HASH7" HEAD > my_bisect_log.txt &&
391+
grep "merge base must be tested" my_bisect_log.txt &&
392+
grep $HASH4 my_bisect_log.txt &&
393+
test_must_fail git bisect bad > my_bisect_log.txt 2>&1 &&
394+
grep "merge base $HASH4 is bad" my_bisect_log.txt &&
395+
grep "fixed between $HASH4 and \[$SIDE_HASH7\]" my_bisect_log.txt &&
396+
git bisect reset
397+
'
398+
399+
# This creates a few more commits (A and B) to test "siblings" cases
400+
# when a good and a bad rev have many merge bases.
401+
#
402+
# We should have the following:
403+
#
404+
# H1-H2-H3-H4-H5-H6-H7
405+
# \ \ \
406+
# S5-A \
407+
# \ \
408+
# S6-S7----B
409+
#
410+
# And there A and B have 2 merge bases (S5 and H5) that should be
411+
# reported by "git merge-base --all A B".
412+
#
413+
test_expect_success 'many merge bases creation' '
414+
git checkout "$SIDE_HASH5" &&
415+
git merge -m "merge HASH5 and SIDE_HASH5" "$HASH5" &&
416+
A_HASH=$(git rev-parse --verify HEAD) &&
417+
git checkout side &&
418+
git merge -m "merge HASH7 and SIDE_HASH7" "$HASH7" &&
419+
B_HASH=$(git rev-parse --verify HEAD) &&
420+
git merge-base --all "$A_HASH" "$B_HASH" > merge_bases.txt &&
421+
test $(wc -l < merge_bases.txt) = "2" &&
422+
grep "$HASH5" merge_bases.txt &&
423+
grep "$SIDE_HASH5" merge_bases.txt
424+
'
425+
426+
test_expect_success 'good merge bases when good and bad are siblings' '
427+
git bisect start "$B_HASH" "$A_HASH" > my_bisect_log.txt &&
428+
grep "merge base must be tested" my_bisect_log.txt &&
429+
git bisect good > my_bisect_log2.txt &&
430+
grep "merge base must be tested" my_bisect_log2.txt &&
431+
{
432+
{
433+
grep "$SIDE_HASH5" my_bisect_log.txt &&
434+
grep "$HASH5" my_bisect_log2.txt
435+
} || {
436+
grep "$SIDE_HASH5" my_bisect_log2.txt &&
437+
grep "$HASH5" my_bisect_log.txt
438+
}
439+
} &&
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+
353467
#
354468
#
355469
test_done

0 commit comments

Comments
 (0)