{"thread":{"id":"15151","subject":"[PATCH 2/2 v3] bisect: only check merge bases when needed","startedAt":"2008-08-22T03:52:29Z","lastAt":"2008-08-22T03:52:29Z","messageCount":1,"participants":["Christian Couder"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"88080","messageId":"20080822055229.95f9b944.chriscool@tuxfamily.org","threadId":"15151","inReplyTo":null,"subject":"[PATCH 2/2 v3] bisect: only check merge bases when needed","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-08-22T03:52:29Z","receivedAt":"2008-08-22T03:52:29Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"When one good revision is not an ancestor of the bad revision, the\nmerge bases between the good and the bad revision should be checked\nto make sure that they are also good revisions.\n\nA previous patch takes care of that, but it may check the merge bases\nmore often than really needed. In fact the previous patch did not try\nto optimize this as much as possible because it is not so simple. So\nthis is the purpose of this patch.\n\nOne may think that when all the merge bases have been checked then\nwe can save a flag, so that we don't need to check the merge bases\nagain during the bisect process.\n\nThe problem is that the user may choose to checkout and test\nsomething completely different from what the bisect process\nsuggested. In this case we have to check the merge bases again,\nbecause there may be new merge bases relevant to the bisect\nprocess.\n\nThat's why, in this patch, when we detect that the user tested\nsomething else than what the bisect process suggested, we remove\nthe flag that says that we don't need to check the merge bases\nagain.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n git-bisect.sh               |   48 ++++++++++++++++++++++++++++++------------\n t/t6030-bisect-porcelain.sh |   24 +++++++++++++++++++++\n 2 files changed, 58 insertions(+), 14 deletions(-)\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex b314d47..69a9a56 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -172,6 +172,25 @@ bisect_write() {\n \ttest -n \"$nolog\" || echo \"git bisect $state $rev\" >>\"$GIT_DIR/BISECT_LOG\"\n }\n \n+is_expected_rev() {\n+\ttest -f \"$GIT_DIR/BISECT_EXPECTED_REV\" &&\n+\ttest \"$1\" = $(cat \"$GIT_DIR/BISECT_EXPECTED_REV\")\n+}\n+\n+mark_expected_rev() {\n+\techo \"$1\" > \"$GIT_DIR/BISECT_EXPECTED_REV\"\n+}\n+\n+check_expected_revs() {\n+\tfor _rev in \"$@\"; do\n+\t\tif ! is_expected_rev \"$_rev\"; then\n+\t\t\trm -f \"$GIT_DIR/BISECT_ANCESTORS_OK\"\n+\t\t\trm -f \"$GIT_DIR/BISECT_EXPECTED_REV\"\n+\t\t\treturn\n+\t\tfi\n+\tdone\n+}\n+\n bisect_state() {\n \tbisect_autostart\n \tstate=$1\n@@ -181,7 +200,8 @@ bisect_state() {\n \t1,bad|1,good|1,skip)\n \t\trev=$(git rev-parse --verify HEAD) ||\n \t\t\tdie \"Bad rev input: HEAD\"\n-\t\tbisect_write \"$state\" \"$rev\" ;;\n+\t\tbisect_write \"$state\" \"$rev\"\n+\t\tcheck_expected_revs \"$rev\" ;;\n \t2,bad|*,good|*,skip)\n \t\tshift\n \t\teval=''\n@@ -191,7 +211,8 @@ bisect_state() {\n \t\t\t\tdie \"Bad rev input: $rev\"\n \t\t\teval=\"$eval bisect_write '$state' '$sha'; \"\n \t\tdone\n-\t\teval \"$eval\" ;;\n+\t\teval \"$eval\"\n+\t\tcheck_expected_revs \"$@\" ;;\n \t*,bad)\n \t\tdie \"'git bisect bad' can take only one argument.\" ;;\n \t*)\n@@ -321,6 +342,7 @@ bisect_checkout() {\n \t_rev=\"$1\"\n \t_msg=\"$2\"\n \techo \"Bisecting: $_msg\"\n+\tmark_expected_rev \"$_rev\"\n \tgit checkout -q \"$_rev\" || exit\n \tgit show-branch \"$_rev\"\n }\n@@ -332,18 +354,10 @@ is_among() {\n \treturn 1\n }\n \n-is_testing_merge_base() {\n-\tgrep \"^testing $1$\" \"$GIT_DIR/BISECT_MERGE_BASES\" >/dev/null 2>&1\n-}\n-\n-mark_testing_merge_base() {\n-\techo \"testing $1\" >> \"$GIT_DIR/BISECT_MERGE_BASES\"\n-}\n-\n handle_bad_merge_base() {\n \t_badmb=\"$1\"\n \t_good=\"$2\"\n-\tif is_testing_merge_base \"$_badmb\"; then\n+\tif is_expected_rev \"$_badmb\"; then\n \t\tcat >&2 <<EOF\n The merge base $_badmb is bad.\n This means the bug has been fixed between $_badmb and [$_good].\n@@ -383,7 +397,6 @@ check_merge_bases() {\n \t\telif is_among \"$_mb\" \"$_skip\"; then\n \t\t\thandle_skipped_merge_base \"$_mb\" \"$_bad\" \"$_good\"\n \t\telse\n-\t\t\tmark_testing_merge_base \"$_mb\"\n \t\t\tbisect_checkout \"$_mb\" \"a merge base must be tested\"\n \t\t\tcheckout_done=1\n \t\t\treturn\n@@ -392,6 +405,9 @@ check_merge_bases() {\n }\n \n check_good_are_ancestors_of_bad() {\n+\ttest -f \"$GIT_DIR/BISECT_ANCESTORS_OK\" &&\n+\t\treturn\n+\n \t_bad=\"$1\"\n \t_good=$(echo $2 | sed -e 's/\\^//g')\n \t_skip=\"$3\"\n@@ -401,8 +417,11 @@ check_good_are_ancestors_of_bad() {\n \n \t_side=$(git rev-list $_good ^$_bad)\n \tif test -n \"$_side\"; then\n-\t\tcheck_merge_bases \"$_bad\" \"$_good\" \"$_skip\"\n+\t\tcheck_merge_bases \"$_bad\" \"$_good\" \"$_skip\" || return\n+\t\ttest \"$checkout_done\" -eq \"1\" && return\n \tfi\n+\n+\t: > \"$GIT_DIR/BISECT_ANCESTORS_OK\"\n }\n \n bisect_next() {\n@@ -491,7 +510,8 @@ bisect_clean_state() {\n \tdo\n \t\tgit update-ref -d $ref $hash || exit\n \tdone\n-\trm -f \"$GIT_DIR/BISECT_MERGE_BASES\" &&\n+\trm -f \"$GIT_DIR/BISECT_EXPECTED_REV\" &&\n+\trm -f \"$GIT_DIR/BISECT_ANCESTORS_OK\" &&\n \trm -f \"$GIT_DIR/BISECT_LOG\" &&\n \trm -f \"$GIT_DIR/BISECT_NAMES\" &&\n \trm -f \"$GIT_DIR/BISECT_RUN\" &&\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex a1ce95c..c163114 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -440,6 +440,30 @@ test_expect_success 'good merge bases when good and bad are siblings' '\n \tgit bisect reset\n '\n \n+check_trace() {\n+\tgrep \"$1\" \"$GIT_TRACE\" | grep \"\\^$2\" | grep \"$3\" >/dev/null\n+}\n+\n+test_expect_success 'optimized merge base checks' '\n+\tGIT_TRACE=\"$(pwd)/trace.log\" &&\n+\texport GIT_TRACE &&\n+\tgit bisect start \"$HASH7\" \"$SIDE_HASH7\" > my_bisect_log.txt &&\n+\tgrep \"merge base must be tested\" my_bisect_log.txt &&\n+\tgrep \"$HASH4\" my_bisect_log.txt &&\n+\tcheck_trace \"rev-list\" \"$HASH7\" \"$SIDE_HASH7\" &&\n+\tgit bisect good > my_bisect_log2.txt &&\n+\ttest -f \".git/BISECT_ANCESTORS_OK\" &&\n+\ttest \"$HASH6\" = $(git rev-parse --verify HEAD) &&\n+\t: > \"$GIT_TRACE\" &&\n+\tgit bisect bad > my_bisect_log3.txt &&\n+\ttest_must_fail check_trace \"rev-list\" \"$HASH6\" \"$SIDE_HASH7\" &&\n+\tgit bisect good \"$A_HASH\" > my_bisect_log4.txt &&\n+\tgrep \"merge base must be tested\" my_bisect_log4.txt &&\n+\ttest_must_fail test -f \".git/BISECT_ANCESTORS_OK\" &&\n+\tcheck_trace \"rev-list\" \"$HASH6\" \"$A_HASH\" &&\n+\tunset GIT_TRACE\n+'\n+\n #\n #\n test_done\n-- \n1.6.0.128.g1827\n"}]}