{"thread":{"id":"52821","subject":"[Outreachy] [PATCH v4 00/12] Finish converting git bisect to C part 1","startedAt":"2020-02-17T08:41:22Z","lastAt":"2020-02-19T17:40:43Z","messageCount":14,"participants":["Miriam Rubio","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":12},"messages":[{"id":"391899","messageId":"20200217084039.78215-1-mirucam@gmail.com","threadId":"52821","inReplyTo":null,"subject":"[Outreachy] [PATCH v4 00/12] Finish converting git bisect to C part 1","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:27Z","receivedAt":"2020-02-17T08:41:22Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"These patches correspond to a first part of patch series \nof Outreachy project \"Finish converting `git bisect` from shell to C\" \nstarted by Pranit Bauva and Tanushree Tumane\n(https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and\ncontinued by me.\n\nThis first part is formed of preparatory/clean-up patches and all \n`bisect.c` libification work. \n\nI would like to thank René Scharfe and Taylor Blau for their\nsuggestions.\n\nThese patch series emails were generated from:\nhttps://gitlab.com/mirucam/git/commits/git-bisect-work-part1-v4\n\n--- Changes since v3 Finish converting git bisect to C part 1 patch series ---\n\nGeneral changes\n---------------\n\n* Removed `run-command: make exists_in_PATH() non-static` patch of this\npart 1 patch series.\n\nMiriam Rubio (3):\n bisect--helper: convert `vocab_*` char pointers to char arrays\n bisect: use the standard 'if (!var)' way to check for 0\n bisect: add enum to represent bisect returning codes\n\nPranit Bauva (7):\n bisect--helper: return error codes from `cmd_bisect__helper()`\n bisect: libify `exit_if_skipped_commits` to `error_if_skipped*` and\n its dependents\n bisect: libify `bisect_checkout`\n bisect: libify `check_merge_bases` and its dependents\n bisect: libify `check_good_are_ancestors_of_bad` and its dependents\n bisect: libify `handle_bad_merge_base` and its dependents\n bisect: libify `bisect_next_all`\n\nTanushree Tumane (2):\n bisect--helper: change `retval` to `res`\n bisect--helper: introduce new `decide_next()` function\n\n bisect.c | 135 +++++++++++++++++++++++++++------------\n bisect.h | 29 ++++++++-\n builtin/bisect--helper.c | 123 +++++++++++++++++++----------------\n 3 files changed, 189 insertions(+), 98 deletions(-)\n\n-- \n2.25.0\n\n"},{"id":"391900","messageId":"20200217084039.78215-2-mirucam@gmail.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"[PATCH v4 01/12] bisect--helper: convert `vocab_*` char pointers to char arrays","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:28Z","receivedAt":"2020-02-17T08:41:24Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Instead of using a pointer that points at a constant string,\njust give name directly to the constant string; this way, we\ndo not have to allocate a pointer variable in addition to\nthe string we want to use.\n\nLet's convert `vocab_bad` and `vocab_good` char pointers to char arrays.\n\nMentored-by: Christian Couder \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 1718df7f09..36c09b7238 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -52,8 +52,8 @@ static void set_terms(struct bisect_terms *terms, const char *bad,\n \tterms->term_bad = xstrdup(bad);\n }\n \n-static const char *vocab_bad = \"bad|new\";\n-static const char *vocab_good = \"good|old\";\n+static const char vocab_bad[] = \"bad|new\";\n+static const char vocab_good[] = \"good|old\";\n \n /*\n * Check whether the string `term` belongs to the set of strings\n-- \n2.25.0\n\n"},{"id":"391901","messageId":"20200217084039.78215-4-mirucam@gmail.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"[PATCH v4 03/12] bisect: use the standard 'if (!var)' way to check for 0","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:30Z","receivedAt":"2020-02-17T08:41:25Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Instead of using 'var == 0' in an if condition, let's use '!var' and\nmake 'bisect.c' more consistent with the rest of the code.\n\nMentored-by: Christian Couder \nSigned-off-by: Miriam Rubio \n---\n bisect.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex e81c91d02c..83cb5b3a98 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -572,7 +572,7 @@ static int sqrti(int val)\n {\n \tfloat d, x = val;\n \n-\tif (val == 0)\n+\tif (!val)\n \t\treturn 0;\n \n \tdo {\n@@ -869,7 +869,7 @@ static void check_good_are_ancestors_of_bad(struct repository *r,\n \t\tgoto done;\n \n \t/* Bisecting with no good rev is ok. */\n-\tif (good_revs.nr == 0)\n+\tif (!good_revs.nr)\n \t\tgoto done;\n \n \t/* Check if all good revs are ancestor of the bad rev. */\n-- \n2.25.0\n\n"},{"id":"391902","messageId":"20200217084039.78215-3-mirucam@gmail.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"[PATCH v4 02/12] bisect--helper: change `retval` to `res`","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:29Z","receivedAt":"2020-02-17T08:41:26Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Tanushree Tumane \n\nLet's rename variable retval to res, so that variable names\nin bisect--helper.c are more consistent.\n\nAfter this change, there are 110 occurrences of res in the file\nand zero of retval, while there were 26 instances of retval before.\n\nMentored-by: Christian Couder \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 52 ++++++++++++++++++++--------------------\n 1 file changed, 26 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 36c09b7238..21de5c096c 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -206,31 +206,31 @@ static int bisect_write(const char *state, const char *rev,\n \tstruct object_id oid;\n \tstruct commit *commit;\n \tFILE *fp = NULL;\n-\tint retval = 0;\n+\tint res = 0;\n \n \tif (!strcmp(state, terms->term_bad)) {\n \t\tstrbuf_addf(&tag, \"refs/bisect/%s\", state);\n \t} else if (one_of(state, terms->term_good, \"skip\", NULL)) {\n \t\tstrbuf_addf(&tag, \"refs/bisect/%s-%s\", state, rev);\n \t} else {\n-\t\tretval = error(_(\"Bad bisect_write argument: %s\"), state);\n+\t\tres = error(_(\"Bad bisect_write argument: %s\"), state);\n \t\tgoto finish;\n \t}\n \n \tif (get_oid(rev, &oid)) {\n-\t\tretval = error(_(\"couldn't get the oid of the rev '%s'\"), rev);\n+\t\tres = error(_(\"couldn't get the oid of the rev '%s'\"), rev);\n \t\tgoto finish;\n \t}\n \n \tif (update_ref(NULL, tag.buf, &oid, NULL, 0,\n \t\t UPDATE_REFS_MSG_ON_ERR)) {\n-\t\tretval = -1;\n+\t\tres = -1;\n \t\tgoto finish;\n \t}\n \n \tfp = fopen(git_path_bisect_log(), \"a\");\n \tif (!fp) {\n-\t\tretval = error_errno(_(\"couldn't open the file '%s'\"), git_path_bisect_log());\n+\t\tres = error_errno(_(\"couldn't open the file '%s'\"), git_path_bisect_log());\n \t\tgoto finish;\n \t}\n \n@@ -244,7 +244,7 @@ static int bisect_write(const char *state, const char *rev,\n \tif (fp)\n \t\tfclose(fp);\n \tstrbuf_release(&tag);\n-\treturn retval;\n+\treturn res;\n }\n \n static int check_and_set_terms(struct bisect_terms *terms, const char *cmd)\n@@ -294,7 +294,7 @@ static const char need_bisect_start_warning[] =\n static int bisect_next_check(const struct bisect_terms *terms,\n \t\t\t const char *current_term)\n {\n-\tint missing_good = 1, missing_bad = 1, retval = 0;\n+\tint missing_good = 1, missing_bad = 1, res = 0;\n \tconst char *bad_ref = xstrfmt(\"refs/bisect/%s\", terms->term_bad);\n \tconst char *good_glob = xstrfmt(\"%s-*\", terms->term_good);\n \n@@ -308,7 +308,7 @@ static int bisect_next_check(const struct bisect_terms *terms,\n \t\tgoto finish;\n \n \tif (!current_term) {\n-\t\tretval = -1;\n+\t\tres = -1;\n \t\tgoto finish;\n \t}\n \n@@ -329,21 +329,21 @@ static int bisect_next_check(const struct bisect_terms *terms,\n \t\t */\n \t\tyesno = git_prompt(_(\"Are you sure [Y/n]? \"), PROMPT_ECHO);\n \t\tif (starts_with(yesno, \"N\") || starts_with(yesno, \"n\"))\n-\t\t\tretval = -1;\n+\t\t\tres = -1;\n \t\tgoto finish;\n \t}\n \tif (!is_empty_or_missing_file(git_path_bisect_start())) {\n-\t\tretval = error(_(need_bad_and_good_revision_warning),\n+\t\tres = error(_(need_bad_and_good_revision_warning),\n \t\t\t vocab_bad, vocab_good, vocab_bad, vocab_good);\n \t} else {\n-\t\tretval = error(_(need_bisect_start_warning),\n+\t\tres = error(_(need_bisect_start_warning),\n \t\t\t vocab_good, vocab_bad, vocab_good, vocab_bad);\n \t}\n \n finish:\n \tfree((void *) good_glob);\n \tfree((void *) bad_ref);\n-\treturn retval;\n+\treturn res;\n }\n \n static int get_terms(struct bisect_terms *terms)\n@@ -397,7 +397,7 @@ static int bisect_terms(struct bisect_terms *terms, const char *option)\n \n static int bisect_append_log_quoted(const char **argv)\n {\n-\tint retval = 0;\n+\tint res = 0;\n \tFILE *fp = fopen(git_path_bisect_log(), \"a\");\n \tstruct strbuf orig_args = STRBUF_INIT;\n \n@@ -405,25 +405,25 @@ static int bisect_append_log_quoted(const char **argv)\n \t\treturn -1;\n \n \tif (fprintf(fp, \"git bisect start\") < 1) {\n-\t\tretval = -1;\n+\t\tres = -1;\n \t\tgoto finish;\n \t}\n \n \tsq_quote_argv(&orig_args, argv);\n \tif (fprintf(fp, \"%s\\n\", orig_args.buf) < 1)\n-\t\tretval = -1;\n+\t\tres = -1;\n \n finish:\n \tfclose(fp);\n \tstrbuf_release(&orig_args);\n-\treturn retval;\n+\treturn res;\n }\n \n static int bisect_start(struct bisect_terms *terms, int no_checkout,\n \t\t\tconst char **argv, int argc)\n {\n \tint i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;\n-\tint flags, pathspec_pos, retval = 0;\n+\tint flags, pathspec_pos, res = 0;\n \tstruct string_list revs = STRING_LIST_INIT_DUP;\n \tstruct string_list states = STRING_LIST_INIT_DUP;\n \tstruct strbuf start_head = STRBUF_INIT;\n@@ -524,7 +524,7 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\n \t\t\targv_array_pushl(&argv, \"checkout\", start_head.buf,\n \t\t\t\t\t \"--\", NULL);\n \t\t\tif (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {\n-\t\t\t\tretval = error(_(\"checking out '%s' failed.\"\n+\t\t\t\tres = error(_(\"checking out '%s' failed.\"\n \t\t\t\t\t\t \" Try 'git bisect start \"\n \t\t\t\t\t\t \"'.\"),\n \t\t\t\t\t start_head.buf);\n@@ -572,12 +572,12 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\n \n \tif (no_checkout) {\n \t\tif (get_oid(start_head.buf, &oid) < 0) {\n-\t\t\tretval = error(_(\"invalid ref: '%s'\"), start_head.buf);\n+\t\t\tres = error(_(\"invalid ref: '%s'\"), start_head.buf);\n \t\t\tgoto finish;\n \t\t}\n \t\tif (update_ref(NULL, \"BISECT_HEAD\", &oid, NULL, 0,\n \t\t\t UPDATE_REFS_MSG_ON_ERR)) {\n-\t\t\tretval = -1;\n+\t\t\tres = -1;\n \t\t\tgoto finish;\n \t\t}\n \t}\n@@ -589,26 +589,26 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\n \tfor (i = 0; i < states.nr; i++)\n \t\tif (bisect_write(states.items[i].string,\n \t\t\t\t revs.items[i].string, terms, 1)) {\n-\t\t\tretval = -1;\n+\t\t\tres = -1;\n \t\t\tgoto finish;\n \t\t}\n \n \tif (must_write_terms && write_terms(terms->term_bad,\n \t\t\t\t\t terms->term_good)) {\n-\t\tretval = -1;\n+\t\tres = -1;\n \t\tgoto finish;\n \t}\n \n-\tretval = bisect_append_log_quoted(argv);\n-\tif (retval)\n-\t\tretval = -1;\n+\tres = bisect_append_log_quoted(argv);\n+\tif (res)\n+\t\tres = -1;\n \n finish:\n \tstring_list_clear(&revs, 0);\n \tstring_list_clear(&states, 0);\n \tstrbuf_release(&start_head);\n \tstrbuf_release(&bisect_names);\n-\treturn retval;\n+\treturn res;\n }\n \n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n-- \n2.25.0\n\n"},{"id":"391903","messageId":"20200217084039.78215-6-mirucam@gmail.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"[PATCH v4 05/12] bisect: add enum to represent bisect returning codes","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:32Z","receivedAt":"2020-02-17T08:41:28Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Since we want to get rid of git-bisect.sh, it would be necessary to\nconvert those exit() calls to return statements so that errors can be\nreported.\n\nCreate an enum called `bisect_error` with the bisecting return codes\nto use in `bisect.c` libification process.\n\nChange bisect_next_all() to make it return this enum.\n\nMentored-by: Christian Couder \nSigned-off-by: Miriam Rubio \n---\n bisect.c | 2 +-\n bisect.h | 14 +++++++++++++-\n 2 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 83cb5b3a98..e4573c7ba1 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -945,7 +945,7 @@ void read_bisect_terms(const char **read_bad, const char **read_good)\n * If no_checkout is non-zero, the bisection process does not\n * checkout the trial commit but instead simply updates BISECT_HEAD.\n */\n-int bisect_next_all(struct repository *r, const char *prefix, int no_checkout)\n+enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int no_checkout)\n {\n \tstruct rev_info revs;\n \tstruct commit_list *tried;\ndiff --git a/bisect.h b/bisect.h\nindex 4e69a11ea8..c921ead02c 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -31,7 +31,19 @@ struct rev_list_info {\n \tconst char *header_prefix;\n };\n \n-int bisect_next_all(struct repository *r,\n+/*\n+ * enum bisect_error represents the following return codes:\n+ * BISECT_OK: success code. Internally, it means that next\n+ * commit has been found (and possibly checked out) and it\n+ * should be tested.\n+ * BISECT_FAILED error code: default error code.\n+ */\n+enum bisect_error {\n+\tBISECT_OK = 0,\n+\tBISECT_FAILED = -1\n+};\n+\n+enum bisect_error bisect_next_all(struct repository *r,\n \t\t const char *prefix,\n \t\t int no_checkout);\n \n-- \n2.25.0\n\n"},{"id":"391904","messageId":"20200217084039.78215-5-mirucam@gmail.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"[PATCH v4 04/12] bisect--helper: introduce new `decide_next()` function","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:31Z","receivedAt":"2020-02-17T08:41:29Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Tanushree Tumane \n\nLet's refactor code from bisect_next_check() into a new\ndecide_next() helper function.\n\nThis removes some goto statements and makes the code simpler,\nclearer and easier to understand.\n\nWhile at it `bad_ref` and `good_glob` are not const any more\nto void casting them inside `free()`.\n\nMentored-by: Christian Couder \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 66 +++++++++++++++++++++-------------------\n 1 file changed, 34 insertions(+), 32 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 21de5c096c..e21d3d1a4c 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -291,26 +291,14 @@ static const char need_bisect_start_warning[] =\n \t \"You then need to give me at least one %s and %s revision.\\n\"\n \t \"You can use \\\"git bisect %s\\\" and \\\"git bisect %s\\\" for that.\");\n \n-static int bisect_next_check(const struct bisect_terms *terms,\n-\t\t\t const char *current_term)\n+static int decide_next(const struct bisect_terms *terms,\n+\t\t const char *current_term, int missing_good,\n+\t\t int missing_bad)\n {\n-\tint missing_good = 1, missing_bad = 1, res = 0;\n-\tconst char *bad_ref = xstrfmt(\"refs/bisect/%s\", terms->term_bad);\n-\tconst char *good_glob = xstrfmt(\"%s-*\", terms->term_good);\n-\n-\tif (ref_exists(bad_ref))\n-\t\tmissing_bad = 0;\n-\n-\tfor_each_glob_ref_in(mark_good, good_glob, \"refs/bisect/\",\n-\t\t\t (void *) &missing_good);\n-\n \tif (!missing_good && !missing_bad)\n-\t\tgoto finish;\n-\n-\tif (!current_term) {\n-\t\tres = -1;\n-\t\tgoto finish;\n-\t}\n+\t\treturn 0;\n+\tif (!current_term)\n+\t\treturn -1;\n \n \tif (missing_good && !missing_bad &&\n \t !strcmp(current_term, terms->term_good)) {\n@@ -321,7 +309,7 @@ static int bisect_next_check(const struct bisect_terms *terms,\n \t\t */\n \t\twarning(_(\"bisecting only with a %s commit\"), terms->term_bad);\n \t\tif (!isatty(0))\n-\t\t\tgoto finish;\n+\t\t\treturn 0;\n \t\t/*\n \t\t * TRANSLATORS: Make sure to include [Y] and [n] in your\n \t\t * translation. The program will only accept English input\n@@ -329,21 +317,35 @@ static int bisect_next_check(const struct bisect_terms *terms,\n \t\t */\n \t\tyesno = git_prompt(_(\"Are you sure [Y/n]? \"), PROMPT_ECHO);\n \t\tif (starts_with(yesno, \"N\") || starts_with(yesno, \"n\"))\n-\t\t\tres = -1;\n-\t\tgoto finish;\n-\t}\n-\tif (!is_empty_or_missing_file(git_path_bisect_start())) {\n-\t\tres = error(_(need_bad_and_good_revision_warning),\n-\t\t\t vocab_bad, vocab_good, vocab_bad, vocab_good);\n-\t} else {\n-\t\tres = error(_(need_bisect_start_warning),\n-\t\t\t vocab_good, vocab_bad, vocab_good, vocab_bad);\n+\t\t\treturn -1;\n+\t\treturn 0;\n \t}\n \n-finish:\n-\tfree((void *) good_glob);\n-\tfree((void *) bad_ref);\n-\treturn res;\n+\tif (!is_empty_or_missing_file(git_path_bisect_start()))\n+\t\treturn error(_(need_bad_and_good_revision_warning),\n+\t\t\t vocab_bad, vocab_good, vocab_bad, vocab_good);\n+\telse\n+\t\treturn error(_(need_bisect_start_warning),\n+\t\t\t vocab_good, vocab_bad, vocab_good, vocab_bad);\n+}\n+\n+static int bisect_next_check(const struct bisect_terms *terms,\n+\t\t\t const char *current_term)\n+{\n+\tint missing_good = 1, missing_bad = 1;\n+\tchar *bad_ref = xstrfmt(\"refs/bisect/%s\", terms->term_bad);\n+\tchar *good_glob = xstrfmt(\"%s-*\", terms->term_good);\n+\n+\tif (ref_exists(bad_ref))\n+\t\tmissing_bad = 0;\n+\n+\tfor_each_glob_ref_in(mark_good, good_glob, \"refs/bisect/\",\n+\t\t\t (void *) &missing_good);\n+\n+\tfree(good_glob);\n+\tfree(bad_ref);\n+\n+\treturn decide_next(terms, current_term, missing_good, missing_bad);\n }\n \n static int get_terms(struct bisect_terms *terms)\n-- \n2.25.0\n\n"},{"id":"391905","messageId":"20200217084039.78215-7-mirucam@gmail.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"[PATCH v4 06/12] bisect--helper: return error codes from `cmd_bisect__helper()`","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:33Z","receivedAt":"2020-02-17T08:41:30Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nSince we want to get rid of git-bisect.sh, it would be necessary\nto convert bisect.c exit() calls to return statements so\nthat errors can be reported. Let's prepare for that by making\nit possible to return different error codes than just 0 or 1.\n\nDifferent error codes might enable a bisecting script calling the\nbisect command that uses this function to do different things\ndepending on the exit status of the bisect command.\n\nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex e21d3d1a4c..e6bd4d6645 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -713,5 +713,5 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\treturn error(\"BUG: unknown subcommand '%d'\", cmdmode);\n \t}\n \tfree_terms(&terms);\n-\treturn !!res;\n+\treturn abs(res);\n }\n-- \n2.25.0\n\n"},{"id":"391906","messageId":"20200217084039.78215-9-mirucam@gmail.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"[PATCH v4 08/12] bisect: libify `bisect_checkout`","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:35Z","receivedAt":"2020-02-17T08:41:32Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nSince we want to get rid of git-bisect.sh, it would be necessary to\nconvert those exit() calls to return statements so that errors can be\nreported.\n\nEmulate try catch in C by converting `exit()` to\n`return `. Follow POSIX conventions to return\n to indicate error.\n\nTurn `exit()` to `return` calls in `bisect_checkout()`.\nChanges related to return values have no bad side effects on the\ncode that calls `bisect_checkout()`.\n\nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n bisect.c | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 85bda3500b..f6582ddfed 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -704,9 +704,10 @@ static int is_expected_rev(const struct object_id *oid)\n \treturn res;\n }\n \n-static int bisect_checkout(const struct object_id *bisect_rev, int no_checkout)\n+static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int no_checkout)\n {\n \tchar bisect_rev_hex[GIT_MAX_HEXSZ + 1];\n+\tenum bisect_error res = BISECT_OK;\n \n \tmemcpy(bisect_rev_hex, oid_to_hex(bisect_rev), the_hash_algo->hexsz + 1);\n \tupdate_ref(NULL, \"BISECT_EXPECTED_REV\", bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR);\n@@ -716,14 +717,24 @@ static int bisect_checkout(const struct object_id *bisect_rev, int no_checkout)\n \t\tupdate_ref(NULL, \"BISECT_HEAD\", bisect_rev, NULL, 0,\n \t\t\t UPDATE_REFS_DIE_ON_ERR);\n \t} else {\n-\t\tint res;\n \t\tres = run_command_v_opt(argv_checkout, RUN_GIT_CMD);\n \t\tif (res)\n-\t\t\texit(res);\n+\t\t\t/*\n+\t\t\t * Errors in `run_command()` itself, signaled by res < 0,\n+\t\t\t * and errors in the child process, signaled by res > 0\n+\t\t\t * can both be treated as regular BISECT_FAILURE (-1).\n+\t\t\t */\n+\t\t\treturn -abs(res);\n \t}\n \n \targv_show_branch[1] = bisect_rev_hex;\n-\treturn run_command_v_opt(argv_show_branch, RUN_GIT_CMD);\n+\tres = run_command_v_opt(argv_show_branch, RUN_GIT_CMD);\n+\t/*\n+\t * Errors in `run_command()` itself, signaled by res < 0,\n+\t * and errors in the child process, signaled by res > 0\n+\t * can both be treated as regular BISECT_FAILURE (-1).\n+\t */\n+\treturn -abs(res);\n }\n \n static struct commit *get_commit_reference(struct repository *r,\n-- \n2.25.0\n\n"},{"id":"391907","messageId":"20200217084039.78215-8-mirucam@gmail.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"[PATCH v4 07/12] bisect: libify `exit_if_skipped_commits` to `error_if_skipped*` and its dependents","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:34Z","receivedAt":"2020-02-17T08:41:33Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nSince we want to get rid of git-bisect.sh, it would be necessary to\nconvert those exit() calls to return statements so that errors can be\nreported.\n\nEmulate try catch in C by converting `exit()` to\n`return `. Follow POSIX conventions to return\n to indicate error.\n\nUpdate all callers to handle the error returns.\n\nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n bisect.c | 17 +++++++++++------\n bisect.h | 5 ++++-\n 2 files changed, 15 insertions(+), 7 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex e4573c7ba1..85bda3500b 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -661,11 +661,11 @@ static void bisect_common(struct rev_info *revs)\n \t\tmark_edges_uninteresting(revs, NULL, 0);\n }\n \n-static void exit_if_skipped_commits(struct commit_list *tried,\n+static enum bisect_error error_if_skipped_commits(struct commit_list *tried,\n \t\t\t\t const struct object_id *bad)\n {\n \tif (!tried)\n-\t\treturn;\n+\t\treturn BISECT_OK;\n \n \tprintf(\"There are only 'skip'ped commits left to test.\\n\"\n \t \"The first %s commit could be any of:\\n\", term_bad);\n@@ -676,7 +676,8 @@ static void exit_if_skipped_commits(struct commit_list *tried,\n \tif (bad)\n \t\tprintf(\"%s\\n\", oid_to_hex(bad));\n \tprintf(_(\"We cannot bisect more!\\n\"));\n-\texit(2);\n+\n+\treturn BISECT_ONLY_SKIPPED_LEFT;\n }\n \n static int is_expected_rev(const struct object_id *oid)\n@@ -950,6 +951,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tstruct rev_info revs;\n \tstruct commit_list *tried;\n \tint reaches = 0, all = 0, nr, steps;\n+\tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n \n@@ -972,8 +974,9 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \t\t * We should exit here only if the \"bad\"\n \t\t * commit is also a \"skip\" commit.\n \t\t */\n-\t\texit_if_skipped_commits(tried, NULL);\n-\n+\t\tres = error_if_skipped_commits(tried, NULL);\n+\t\tif (res < 0)\n+\t\t\texit(-res);\n \t\tprintf(_(\"%s was both %s and %s\\n\"),\n \t\t oid_to_hex(current_bad_oid),\n \t\t term_good,\n@@ -990,7 +993,9 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tbisect_rev = &revs.commits->item->object.oid;\n \n \tif (oideq(bisect_rev, current_bad_oid)) {\n-\t\texit_if_skipped_commits(tried, current_bad_oid);\n+\t\tres = error_if_skipped_commits(tried, current_bad_oid);\n+\t\tif (res)\n+\t\t\texit(-res);\n \t\tprintf(\"%s is the first %s commit\\n\", oid_to_hex(bisect_rev),\n \t\t\tterm_bad);\n \t\tshow_diff_tree(r, prefix, revs.commits->item);\ndiff --git a/bisect.h b/bisect.h\nindex c921ead02c..19d90e4870 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -37,10 +37,13 @@ struct rev_list_info {\n * commit has been found (and possibly checked out) and it\n * should be tested.\n * BISECT_FAILED error code: default error code.\n+ * BISECT_ONLY_SKIPPED_LEFT error code: only skipped\n+ * commits left to be tested.\n */\n enum bisect_error {\n \tBISECT_OK = 0,\n-\tBISECT_FAILED = -1\n+\tBISECT_FAILED = -1,\n+\tBISECT_ONLY_SKIPPED_LEFT = -2\n };\n \n enum bisect_error bisect_next_all(struct repository *r,\n-- \n2.25.0\n\n"},{"id":"391908","messageId":"20200217084039.78215-11-mirucam@gmail.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"[PATCH v4 10/12] bisect: libify `check_good_are_ancestors_of_bad` and its dependents","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:37Z","receivedAt":"2020-02-17T08:41:37Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nSince we want to get rid of git-bisect.sh, it would be necessary to\nconvert those exit() calls to return statements so that errors can be\nreported.\n\nEmulate try catch in C by converting `exit()` to\n`return `. Follow POSIX conventions to return\n to indicate error.\n\nCode that turns BISECT_INTERNAL_SUCCESS_MERGE_BASE (-11)\n to BISECT_OK (0) from `check_good_are_ancestors_of_bad()` has been moved to\n`cmd_bisect__helper()`.\n\nUpdate all callers to handle the error returns.\n\nMentored-by: Christian Couder \nMentored by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n bisect.c | 41 ++++++++++++++++++++++++++--------------\n builtin/bisect--helper.c | 11 ++++++++++-\n 2 files changed, 37 insertions(+), 15 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 382e0b471f..f5ce3a4b70 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -872,20 +872,23 @@ static int check_ancestors(struct repository *r, int rev_nr,\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, its source code will be\n- * checked out to be tested by the user and we will exit.\n+ * checked out to be tested by the user and we will return.\n */\n-static void check_good_are_ancestors_of_bad(struct repository *r,\n+\n+static enum bisect_error check_good_are_ancestors_of_bad(struct repository *r,\n \t\t\t\t\t const char *prefix,\n \t\t\t\t\t int no_checkout)\n {\n-\tchar *filename = git_pathdup(\"BISECT_ANCESTORS_OK\");\n+\tchar *filename;\n \tstruct stat st;\n \tint fd, rev_nr;\n \tenum bisect_error res = BISECT_OK;\n \tstruct commit **rev;\n \n \tif (!current_bad_oid)\n-\t\tdie(_(\"a %s revision is needed\"), term_bad);\n+\t\treturn error(_(\"a %s revision is needed\"), term_bad);\n+\n+\tfilename = git_pathdup(\"BISECT_ANCESTORS_OK\");\n \n \t/* Check if file BISECT_ANCESTORS_OK exists. */\n \tif (!stat(filename, &st) && S_ISREG(st.st_mode))\n@@ -901,18 +904,26 @@ static void check_good_are_ancestors_of_bad(struct repository *r,\n \tif (check_ancestors(r, rev_nr, rev, prefix))\n \t\tres = check_merge_bases(rev_nr, rev, no_checkout);\n \tfree(rev);\n-\tif (res)\n-\t\texit(res == BISECT_INTERNAL_SUCCESS_MERGE_BASE ? BISECT_OK : -res);\n \n-\t/* Create file BISECT_ANCESTORS_OK. */\n-\tfd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);\n-\tif (fd < 0)\n-\t\twarning_errno(_(\"could not create file '%s'\"),\n-\t\t\t filename);\n-\telse\n-\t\tclose(fd);\n+\tif (!res) {\n+\t\t/* Create file BISECT_ANCESTORS_OK. */\n+\t\tfd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);\n+\t\tif (fd < 0)\n+\t\t\t/*\n+\t\t\t * BISECT_ANCESTORS_OK file is not absolutely necessary,\n+\t\t\t * the bisection process will continue at the next\n+\t\t\t * bisection step.\n+\t\t\t * So, just signal with a warning that something\n+\t\t\t * might be wrong.\n+\t\t\t */\n+\t\t\twarning_errno(_(\"could not create file '%s'\"),\n+\t\t\t\tfilename);\n+\t\telse\n+\t\t\tclose(fd);\n+\t}\n done:\n \tfree(filename);\n+\treturn res;\n }\n \n /*\n@@ -984,7 +995,9 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tif (read_bisect_refs())\n \t\tdie(_(\"reading bisect refs failed\"));\n \n-\tcheck_good_are_ancestors_of_bad(r, prefix, no_checkout);\n+\tres = check_good_are_ancestors_of_bad(r, prefix, no_checkout);\n+\tif (res)\n+\t\treturn res;\n \n \tbisect_rev_setup(r, &revs, prefix, \"%s\", \"^%s\", 1);\n \trevs.limited = 1;\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex e6bd4d6645..c1c40b516d 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -666,7 +666,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \n \tswitch (cmdmode) {\n \tcase NEXT_ALL:\n-\t\treturn bisect_next_all(the_repository, prefix, no_checkout);\n+\t\tres = bisect_next_all(the_repository, prefix, no_checkout);\n+\t\tbreak;\n \tcase WRITE_TERMS:\n \t\tif (argc != 2)\n \t\t\treturn error(_(\"--write-terms requires two arguments\"));\n@@ -713,5 +714,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\treturn error(\"BUG: unknown subcommand '%d'\", cmdmode);\n \t}\n \tfree_terms(&terms);\n+\n+\t/*\n+\t * Handle early success\n+\t * From check_merge_bases > check_good_are_ancestors_of_bad > bisect_next_all\n+\t */\n+\tif (res == BISECT_INTERNAL_SUCCESS_MERGE_BASE)\n+\t\tres = BISECT_OK;\n+\n \treturn abs(res);\n }\n-- \n2.25.0\n\n"},{"id":"391909","messageId":"20200217084039.78215-10-mirucam@gmail.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"[PATCH v4 09/12] bisect: libify `check_merge_bases` and its dependents","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:36Z","receivedAt":"2020-02-17T08:41:38Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nSince we want to get rid of git-bisect.sh, it would be necessary to\nconvert those exit() calls to return statements so that errors can be\nreported.\n\nEmulate try catch in C by converting `exit()` to\n`return `. Follow POSIX conventions to return\n to indicate error.\n\nIn `check_merge_bases()` there is an early success special case,\nso we have introduced special error code\nBISECT_INTERNAL_SUCCESS_MERGE_BASE (-11) which indicates early\nsuccess. This BISECT_INTERNAL_SUCCESS_MERGE_BASE is converted back\nto BISECT_OK (0) in `check_good_are_ancestors_of_bad()`.\n\nUpdate all callers to handle the error returns.\n\nMentored-by: Christian Couder \nMentored by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n bisect.c | 22 ++++++++++++++++++----\n bisect.h | 7 ++++++-\n 2 files changed, 24 insertions(+), 5 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex f6582ddfed..382e0b471f 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -811,13 +811,18 @@ static void handle_skipped_merge_base(const struct object_id *mb)\n * \"check_merge_bases\" checks that merge bases are not \"bad\" (or \"new\").\n *\n * - If one is \"bad\" (or \"new\"), it means the user assumed something wrong\n- * and we must exit with a non 0 error code.\n+ * and we must return error with a non 0 error code.\n * - If one is \"good\" (or \"old\"), that's good, we have nothing to do.\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+ * - If a merge base must be tested, on success return\n+ * BISECT_INTERNAL_SUCCESS_MERGE_BASE (-11) a special condition\n+ * for early success, this will be converted back to 0 in\n+ * check_good_are_ancestors_of_bad().\n */\n-static void check_merge_bases(int rev_nr, struct commit **rev, int no_checkout)\n+static enum bisect_error check_merge_bases(int rev_nr, struct commit **rev, int no_checkout)\n {\n+\tenum bisect_error res = BISECT_OK;\n \tstruct commit_list *result;\n \n \tresult = get_merge_bases_many(rev[0], rev_nr - 1, rev + 1);\n@@ -832,11 +837,16 @@ static void check_merge_bases(int rev_nr, struct commit **rev, int no_checkout)\n \t\t\thandle_skipped_merge_base(mb);\n \t\t} else {\n \t\t\tprintf(_(\"Bisecting: a merge base must be tested\\n\"));\n-\t\t\texit(bisect_checkout(mb, no_checkout));\n+\t\t\tres = bisect_checkout(mb, no_checkout);\n+\t\t\tif (!res)\n+\t\t\t\t/* indicate early success */\n+\t\t\t\tres = BISECT_INTERNAL_SUCCESS_MERGE_BASE;\n+\t\t\tbreak;\n \t\t}\n \t}\n \n \tfree_commit_list(result);\n+\treturn res;\n }\n \n static int check_ancestors(struct repository *r, int rev_nr,\n@@ -871,6 +881,7 @@ static void check_good_are_ancestors_of_bad(struct repository *r,\n \tchar *filename = git_pathdup(\"BISECT_ANCESTORS_OK\");\n \tstruct stat st;\n \tint fd, rev_nr;\n+\tenum bisect_error res = BISECT_OK;\n \tstruct commit **rev;\n \n \tif (!current_bad_oid)\n@@ -885,10 +896,13 @@ static void check_good_are_ancestors_of_bad(struct repository *r,\n \t\tgoto done;\n \n \t/* Check if all good revs are ancestor of the bad rev. */\n+\n \trev = get_bad_and_good_commits(r, &rev_nr);\n \tif (check_ancestors(r, rev_nr, rev, prefix))\n-\t\tcheck_merge_bases(rev_nr, rev, no_checkout);\n+\t\tres = check_merge_bases(rev_nr, rev, no_checkout);\n \tfree(rev);\n+\tif (res)\n+\t\texit(res == BISECT_INTERNAL_SUCCESS_MERGE_BASE ? BISECT_OK : -res);\n \n \t/* Create file BISECT_ANCESTORS_OK. */\n \tfd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);\ndiff --git a/bisect.h b/bisect.h\nindex 19d90e4870..f68ae85376 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -39,11 +39,16 @@ struct rev_list_info {\n * BISECT_FAILED error code: default error code.\n * BISECT_ONLY_SKIPPED_LEFT error code: only skipped\n * commits left to be tested.\n+ * BISECT_INTERNAL_SUCCESS_MERGE_BASE early success\n+ * code: found merge base that should be tested.\n+ * Early success code BISECT_INTERNAL_SUCCESS_MERGE_BASE\n+ * should be only an internal code.\n */\n enum bisect_error {\n \tBISECT_OK = 0,\n \tBISECT_FAILED = -1,\n-\tBISECT_ONLY_SKIPPED_LEFT = -2\n+\tBISECT_ONLY_SKIPPED_LEFT = -2,\n+\tBISECT_INTERNAL_SUCCESS_MERGE_BASE = -11\n };\n \n enum bisect_error bisect_next_all(struct repository *r,\n-- \n2.25.0\n\n"},{"id":"391910","messageId":"20200217084039.78215-12-mirucam@gmail.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"[PATCH v4 11/12] bisect: libify `handle_bad_merge_base` and its dependents","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:38Z","receivedAt":"2020-02-17T08:41:39Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nSince we want to get rid of git-bisect.sh, it would be necessary to\nconvert those exit() calls to return statements so that errors can be\nreported.\n\nEmulate try catch in C by converting `exit()` to\n`return `. Follow POSIX conventions to return\n to indicate error.\n\nUpdate all callers to handle the error returns.\n\nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n bisect.c | 9 +++++----\n bisect.h | 1 +\n 2 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex f5ce3a4b70..837332a428 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -761,7 +761,7 @@ static struct commit **get_bad_and_good_commits(struct repository *r,\n \treturn rev;\n }\n \n-static void handle_bad_merge_base(void)\n+static enum bisect_error handle_bad_merge_base(void)\n {\n \tif (is_expected_rev(current_bad_oid)) {\n \t\tchar *bad_hex = oid_to_hex(current_bad_oid);\n@@ -782,14 +782,14 @@ static void handle_bad_merge_base(void)\n \t\t\t\t\"between %s and [%s].\\n\"),\n \t\t\t\tbad_hex, term_bad, term_good, bad_hex, good_hex);\n \t\t}\n-\t\texit(3);\n+\t\treturn BISECT_MERGE_BASE_CHECK;\n \t}\n \n \tfprintf(stderr, _(\"Some %s revs are not ancestors of the %s rev.\\n\"\n \t\t\"git bisect cannot work properly in this case.\\n\"\n \t\t\"Maybe you mistook %s and %s revs?\\n\"),\n \t\tterm_good, term_bad, term_good, term_bad);\n-\texit(1);\n+\treturn BISECT_FAILED;\n }\n \n static void handle_skipped_merge_base(const struct object_id *mb)\n@@ -830,7 +830,8 @@ static enum bisect_error check_merge_bases(int rev_nr, struct commit **rev, int\n \tfor (; result; result = result->next) {\n \t\tconst struct object_id *mb = &result->item->object.oid;\n \t\tif (oideq(mb, current_bad_oid)) {\n-\t\t\thandle_bad_merge_base();\n+\t\t\tres = handle_bad_merge_base();\n+\t\t\tbreak;\n \t\t} else if (0 <= oid_array_lookup(&good_revs, mb)) {\n \t\t\tcontinue;\n \t\t} else if (0 <= oid_array_lookup(&skipped_revs, mb)) {\ndiff --git a/bisect.h b/bisect.h\nindex f68ae85376..0d9758179f 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -48,6 +48,7 @@ enum bisect_error {\n \tBISECT_OK = 0,\n \tBISECT_FAILED = -1,\n \tBISECT_ONLY_SKIPPED_LEFT = -2,\n+\tBISECT_MERGE_BASE_CHECK = -3,\n \tBISECT_INTERNAL_SUCCESS_MERGE_BASE = -11\n };\n \n-- \n2.25.0\n\n"},{"id":"391911","messageId":"20200217084039.78215-13-mirucam@gmail.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"[PATCH v4 12/12] bisect: libify `bisect_next_all`","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-02-17T08:40:39Z","receivedAt":"2020-02-17T08:41:40Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nSince we want to get rid of git-bisect.sh, it would be necessary to\nconvert those exit() calls to return statements so that errors can be\nreported.\n\nEmulate try catch in C by converting `exit()` to\n`return `. Follow POSIX conventions to return\n to indicate error.\n\nAll the functions calling `bisect_next_all()` are already able to\nhandle return values from it.\n\nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n bisect.c | 29 +++++++++++++++++++----------\n bisect.h | 10 ++++++++--\n 2 files changed, 27 insertions(+), 12 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 837332a428..9154f810f7 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -976,10 +976,10 @@ void read_bisect_terms(const char **read_bad, const char **read_good)\n }\n \n /*\n- * We use the convention that exiting with an exit code 10 means that\n+ * We use the convention that return BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND (-10) means\n * the bisection process finished successfully.\n- * In this case the calling shell script should exit 0.\n- *\n+ * In this case the calling function or command should not turn a\n+ * BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND return code into an error or a non zero exit code.\n * If no_checkout is non-zero, the bisection process does not\n * checkout the trial commit but instead simply updates BISECT_HEAD.\n */\n@@ -1010,23 +1010,25 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \n \tif (!revs.commits) {\n \t\t/*\n-\t\t * We should exit here only if the \"bad\"\n+\t\t * We should return error here only if the \"bad\"\n \t\t * commit is also a \"skip\" commit.\n \t\t */\n \t\tres = error_if_skipped_commits(tried, NULL);\n \t\tif (res < 0)\n-\t\t\texit(-res);\n+\t\t\treturn res;\n \t\tprintf(_(\"%s was both %s and %s\\n\"),\n \t\t oid_to_hex(current_bad_oid),\n \t\t term_good,\n \t\t term_bad);\n-\t\texit(1);\n+\n+\t\treturn BISECT_FAILED;\n \t}\n \n \tif (!all) {\n \t\tfprintf(stderr, _(\"No testable commit found.\\n\"\n \t\t\t\"Maybe you started with bad path parameters?\\n\"));\n-\t\texit(4);\n+\n+\t\treturn BISECT_NO_TESTABLE_COMMIT;\n \t}\n \n \tbisect_rev = &revs.commits->item->object.oid;\n@@ -1034,12 +1036,19 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\n \tif (oideq(bisect_rev, current_bad_oid)) {\n \t\tres = error_if_skipped_commits(tried, current_bad_oid);\n \t\tif (res)\n-\t\t\texit(-res);\n+\t\t\treturn res;\n \t\tprintf(\"%s is the first %s commit\\n\", oid_to_hex(bisect_rev),\n \t\t\tterm_bad);\n+\n \t\tshow_diff_tree(r, prefix, revs.commits->item);\n-\t\t/* This means the bisection process succeeded. */\n-\t\texit(10);\n+\t\t/*\n+\t\t * This means the bisection process succeeded.\n+\t\t * Using BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND (-10)\n+\t\t * so that the call chain can simply check\n+\t\t * for negative return values for early returns up\n+\t\t * until the cmd_bisect__helper() caller.\n+\t\t */\n+\t\treturn BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;\n \t}\n \n \tnr = all - reaches - 1;\ndiff --git a/bisect.h b/bisect.h\nindex 0d9758179f..8bad8d8391 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -39,16 +39,22 @@ struct rev_list_info {\n * BISECT_FAILED error code: default error code.\n * BISECT_ONLY_SKIPPED_LEFT error code: only skipped\n * commits left to be tested.\n+ * BISECT_MERGE_BASE_CHECK error code: merge base check failed.\n+ * BISECT_NO_TESTABLE_COMMIT error code: no testable commit found.\n+ * BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND early success code:\n+ * first term_bad commit found.\n * BISECT_INTERNAL_SUCCESS_MERGE_BASE early success\n * code: found merge base that should be tested.\n- * Early success code BISECT_INTERNAL_SUCCESS_MERGE_BASE\n- * should be only an internal code.\n+ * Early success codes BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND and\n+ * BISECT_INTERNAL_SUCCESS_MERGE_BASE should be only internal codes.\n */\n enum bisect_error {\n \tBISECT_OK = 0,\n \tBISECT_FAILED = -1,\n \tBISECT_ONLY_SKIPPED_LEFT = -2,\n \tBISECT_MERGE_BASE_CHECK = -3,\n+\tBISECT_NO_TESTABLE_COMMIT = -4,\n+\tBISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND = -10,\n \tBISECT_INTERNAL_SUCCESS_MERGE_BASE = -11\n };\n \n-- \n2.25.0\n\n"},{"id":"392085","messageId":"xmqqd0aafpbg.fsf@gitster-ct.c.googlers.com","threadId":"52821","inReplyTo":"20200217084039.78215-1-mirucam@gmail.com","subject":"Re: [Outreachy] [PATCH v4 00/12] Finish converting git bisect to C part 1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-19T17:40:35Z","receivedAt":"2020-02-19T17:40:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miriam Rubio writes:\n\n> These patches correspond to a first part of patch series \n> of Outreachy project \"Finish converting `git bisect` from shell to C\" \n> started by Pranit Bauva and Tanushree Tumane\n> (https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and\n> continued by me.\n>\n> This first part is formed of preparatory/clean-up patches and all \n> `bisect.c` libification work. \n>\n> I would like to thank René Scharfe and Taylor Blau for their\n> suggestions.\n\nThanks all.\n\nWill queue. I guess this \"part 1\" is pretty much done and ready for\n'next'.\n"}]}