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

bisect: libify `check_merge_bases` and its dependents

Since we want to get rid of git-bisect.sh, it would be necessary to convert those exit() calls to return statements so that errors can be reported. Emulate try catch in C by converting `exit(<positive-value>)` to `return <negative-value>`. Follow POSIX conventions to return <negative-value> to indicate error. In `check_merge_bases()` there is an early success special case, so we have introduced special error code BISECT_INTERNAL_SUCCESS_MERGE_BASE (-11) which indicates early success. This BISECT_INTERNAL_SUCCESS_MERGE_BASE is converted back to BISECT_OK (0) in `check_good_are_ancestors_of_bad()`. Update all callers to handle the error returns. Mentored-by: Christian Couder <chriscool@tuxfamily.org> Mentored by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com> Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com> Signed-off-by: Miriam Rubio <mirucam@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Pranit Bauva committed Feb 17, 2020 at 09:40 UTC cdd4dc2d6a3e420178100efc780a987495a1a3bd
2 files changed +24 -5
bisect.c
+18 -4
@@ -811,13 +811,18 @@ static void handle_skipped_merge_base(const struct object_id *mb)
811 * "check_merge_bases" checks that merge bases are not "bad" (or "new").
812 *
813 * - If one is "bad" (or "new"), it means the user assumed something wrong
814 - * and we must exit with a non 0 error code.
814 + * and we must return error with a non 0 error code.
815 * - If one is "good" (or "old"), that's good, we have nothing to do.
816 * - If one is "skipped", we can't know but we should warn.
817 * - If we don't know, we should check it out and ask the user to test.
818 + * - If a merge base must be tested, on success return
819 + * BISECT_INTERNAL_SUCCESS_MERGE_BASE (-11) a special condition
820 + * for early success, this will be converted back to 0 in
821 + * check_good_are_ancestors_of_bad().
822 */
819 -static void check_merge_bases(int rev_nr, struct commit **rev, int no_checkout)
823 +static enum bisect_error check_merge_bases(int rev_nr, struct commit **rev, int no_checkout)
824 {
825 + enum bisect_error res = BISECT_OK;
826 struct commit_list *result;
827
828 result = get_merge_bases_many(rev[0], rev_nr - 1, rev + 1);
@@ -832,11 +837,16 @@ static void check_merge_bases(int rev_nr, struct commit **rev, int no_checkout)
837 handle_skipped_merge_base(mb);
838 } else {
839 printf(_("Bisecting: a merge base must be tested\n"));
835 - exit(bisect_checkout(mb, no_checkout));
840 + res = bisect_checkout(mb, no_checkout);
841 + if (!res)
842 + /* indicate early success */
843 + res = BISECT_INTERNAL_SUCCESS_MERGE_BASE;
844 + break;
845 }
846 }
847
848 free_commit_list(result);
849 + return res;
850 }
851
852 static int check_ancestors(struct repository *r, int rev_nr,
@@ -871,6 +881,7 @@ static void check_good_are_ancestors_of_bad(struct repository *r,
881 char *filename = git_pathdup("BISECT_ANCESTORS_OK");
882 struct stat st;
883 int fd, rev_nr;
884 + enum bisect_error res = BISECT_OK;
885 struct commit **rev;
886
887 if (!current_bad_oid)
@@ -885,10 +896,13 @@ static void check_good_are_ancestors_of_bad(struct repository *r,
896 goto done;
897
898 /* Check if all good revs are ancestor of the bad rev. */
899 +
900 rev = get_bad_and_good_commits(r, &rev_nr);
901 if (check_ancestors(r, rev_nr, rev, prefix))
890 - check_merge_bases(rev_nr, rev, no_checkout);
902 + res = check_merge_bases(rev_nr, rev, no_checkout);
903 free(rev);
904 + if (res)
905 + exit(res == BISECT_INTERNAL_SUCCESS_MERGE_BASE ? BISECT_OK : -res);
906
907 /* Create file BISECT_ANCESTORS_OK. */
908 fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);
bisect.h
+6 -1
@@ -39,11 +39,16 @@ struct rev_list_info {
39 * BISECT_FAILED error code: default error code.
40 * BISECT_ONLY_SKIPPED_LEFT error code: only skipped
41 * commits left to be tested.
42 + * BISECT_INTERNAL_SUCCESS_MERGE_BASE early success
43 + * code: found merge base that should be tested.
44 + * Early success code BISECT_INTERNAL_SUCCESS_MERGE_BASE
45 + * should be only an internal code.
46 */
47 enum bisect_error {
48 BISECT_OK = 0,
49 BISECT_FAILED = -1,
46 - BISECT_ONLY_SKIPPED_LEFT = -2
50 + BISECT_ONLY_SKIPPED_LEFT = -2,
51 + BISECT_INTERNAL_SUCCESS_MERGE_BASE = -11
52 };
53
54 enum bisect_error bisect_next_all(struct repository *r,