{"thread":{"id":"15392","subject":"[PATCH] bisect: remove \"checkout_done\" variable used when checking merge bases","startedAt":"2008-09-06T05:27:03Z","lastAt":"2008-09-06T05:27:03Z","messageCount":1,"participants":["Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"89893","messageId":"20080906072703.98a0475d.chriscool@tuxfamily.org","threadId":"15392","inReplyTo":null,"subject":"[PATCH] bisect: remove \"checkout_done\" variable used when checking merge bases","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-09-06T05:27:03Z","receivedAt":"2008-09-06T05:27:03Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Using return values from the following functions:\n\n- check_merge_bases\n- check_good_are_ancestors_of_bad\n\nseems simpler.\n\nWhile at it, let's add some comments to better document the above\nfunctions.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n git-bisect.sh |   32 +++++++++++++++++++++++++++-----\n 1 files changed, 27 insertions(+), 5 deletions(-)\n\n\t> > PS: After thinking about it, I wonder if we should\n\t> > remove $checkout_done entirely and use the return\n\t> > value from \"check_merge_bases\" and\n\t> > \"check_good_are_ancestors_of_bad\" to know if a\n\t> > checkout was done.\n\t>\n\t> Yup, that might make more sense. \n\n\tSo here is such a patch.\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 69a9a56..79de701 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -384,6 +384,17 @@ We continue anyway.\n EOF\n }\n \n+#\n+# \"check_merge_bases\" checks that merge bases are not \"bad\".\n+#\n+# - If one is \"good\", that's good, we have nothing to do.\n+# - If one is \"bad\", it means the user assumed something wrong\n+# and we must exit.\n+# - If one is \"skipped\", we can't know but we should warn.\n+# - If we don't know, we should check it out and ask the user to test.\n+#\n+# In the last case we will return 1, and otherwise 0.\n+#\n check_merge_bases() {\n \t_bad=\"$1\"\n \t_good=\"$2\"\n@@ -398,12 +409,20 @@ check_merge_bases() {\n \t\t\thandle_skipped_merge_base \"$_mb\" \"$_bad\" \"$_good\"\n \t\telse\n \t\t\tbisect_checkout \"$_mb\" \"a merge base must be tested\"\n-\t\t\tcheckout_done=1\n-\t\t\treturn\n+\t\t\treturn 1\n \t\tfi\n \tdone\n+\treturn 0\n }\n \n+#\n+# \"check_good_are_ancestors_of_bad\" checks that all \"good\" revs are\n+# ancestor of the \"bad\" rev.\n+#\n+# If that's not the case, we need to check the merge bases.\n+# If a merge base must be tested by the user we return 1 and\n+# otherwise 0.\n+#\n check_good_are_ancestors_of_bad() {\n \ttest -f \"$GIT_DIR/BISECT_ANCESTORS_OK\" &&\n \t\treturn\n@@ -417,11 +436,13 @@ check_good_are_ancestors_of_bad() {\n \n \t_side=$(git rev-list $_good ^$_bad)\n \tif test -n \"$_side\"; then\n+\t\t# Return if a checkout was done\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+\treturn 0\n }\n \n bisect_next() {\n@@ -437,8 +458,9 @@ bisect_next() {\n \t\t\"refs/bisect/skip-*\" | tr '\\012' ' ') &&\n \n \t# Maybe some merge bases must be tested first\n-\tcheck_good_are_ancestors_of_bad \"$bad\" \"$good\" \"$skip\" || exit\n-\ttest \"$checkout_done\" -eq \"1\" && checkout_done='' && return\n+\tcheck_good_are_ancestors_of_bad \"$bad\" \"$good\" \"$skip\"\n+\t# Return now if a checkout has already been done\n+\ttest \"$?\" -eq \"1\" && return\n \n \t# Get bisection information\n \tBISECT_OPT=''\n-- \n1.6.0.1.320.gd5f2f.dirty\n"}]}