{"thread":{"id":"54286","subject":"[PATCH v9 0/6]Finish converting git bisect to C part 2 subset 1","startedAt":"2020-09-24T12:34:00Z","lastAt":"2020-09-29T20:56:05Z","messageCount":8,"participants":["Miriam Rubio","Junio C Hamano"],"isPatch":true,"patchVersion":9,"patchTotal":6},"messages":[{"id":"406255","messageId":"20200924123340.52632-1-mirucam@gmail.com","threadId":"54286","inReplyTo":null,"subject":"[PATCH v9 0/6]Finish converting git bisect to C part 2 subset 1","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-24T12:33:34Z","receivedAt":"2020-09-24T12:34:00Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"These patches correspond to a second 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 patch series version is composed by the first six patches of the\nprevious version due to a bug discovered during the test stage, that \naffects the rest of the patches and that needs to be fixed before \nsending them.\n\nThese patch series emails were generated from:\nhttps://gitlab.com/mirucam/git/commits/git-bisect-work-part2-v9-subset1.\n\nI would like to thank Johannes Schindelin for reviewing this patch series\nand to Christian Couder for his advice and help.\n\nGeneral information\n-------------------\n\n* Rebased on 385c171a01, (Fifteenth batch, 2020-09-18).\n\nSpecific changes\n----------------\n\n[4/6] bisect--helper: reimplement `bisect_autostart` shell function in C\n\n* Add `|| exit` to bisect-autostart call in shell script.\n\n---\n\nMiriam Rubio (4):\n  bisect--helper: BUG() in cmd_*() on invalid subcommand\n  bisect--helper: use '-res' in 'cmd_bisect__helper' return\n  bisect--helper: introduce new `write_in_file()` function\n  bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'\n\nPranit Bauva (2):\n  bisect--helper: reimplement `bisect_autostart` shell function in C\n  bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell\n    functions in C\n\n bisect.c                 |  13 +-\n builtin/bisect--helper.c | 269 +++++++++++++++++++++++++++++++++++++--\n git-bisect.sh            |  70 +---------\n 3 files changed, 274 insertions(+), 78 deletions(-)\n\n-- \n2.25.0\n\n"},{"id":"406256","messageId":"20200924123340.52632-2-mirucam@gmail.com","threadId":"54286","inReplyTo":"20200924123340.52632-1-mirucam@gmail.com","subject":"[PATCH v9 1/6] bisect--helper: BUG() in cmd_*() on invalid subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-24T12:33:35Z","receivedAt":"2020-09-24T12:34:01Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"In cmd_bisect__helper() function, if an invalid or no\nsubcommand is passed there is a BUG.\n\nBUG() out instead of returning an error.\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nMentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Miriam Rubio <mirucam@gmail.com>\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 7dcc1b5188..23a9f00035 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -720,7 +720,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tres = bisect_start(&terms, argv, argc);\n \t\tbreak;\n \tdefault:\n-\t\treturn error(\"BUG: unknown subcommand '%d'\", cmdmode);\n+\t\tBUG(\"unknown subcommand %d\", cmdmode);\n \t}\n \tfree_terms(&terms);\n \n-- \n2.25.0\n\n"},{"id":"406257","messageId":"20200924123340.52632-3-mirucam@gmail.com","threadId":"54286","inReplyTo":"20200924123340.52632-1-mirucam@gmail.com","subject":"[PATCH v9 2/6] bisect--helper: use '-res' in 'cmd_bisect__helper' return","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-24T12:33:36Z","receivedAt":"2020-09-24T12:34:02Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Following 'enum bisect_error' vocabulary, return variable 'res' is\nalways non-positive.\nLet's use '-res' instead of 'abs(res)' to make the code clearer.\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Miriam Rubio <mirucam@gmail.com>\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 23a9f00035..8037888ae2 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -731,5 +731,5 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \tif (res == BISECT_INTERNAL_SUCCESS_MERGE_BASE)\n \t\tres = BISECT_OK;\n \n-\treturn abs(res);\n+\treturn -res;\n }\n-- \n2.25.0\n\n"},{"id":"406258","messageId":"20200924123340.52632-4-mirucam@gmail.com","threadId":"54286","inReplyTo":"20200924123340.52632-1-mirucam@gmail.com","subject":"[PATCH v9 3/6] bisect--helper: introduce new `write_in_file()` function","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-24T12:33:37Z","receivedAt":"2020-09-24T12:34:05Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Let's refactor code adding a new `write_in_file()` function\nthat opens a file for writing a message and closes it and a\nwrapper for writing mode.\n\nThis helper will be used in later steps and makes the code\nsimpler and easier to understand.\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nMentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Miriam Rubio <mirucam@gmail.com>\n---\n builtin/bisect--helper.c | 43 +++++++++++++++++++++++++++++++++-------\n 1 file changed, 36 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 8037888ae2..46d2abb36f 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -74,6 +74,40 @@ static int one_of(const char *term, ...)\n \treturn res;\n }\n \n+static int write_in_file(const char *path, const char *mode, const char *format, va_list args)\n+{\n+\tFILE *fp = NULL;\n+\tint res = 0;\n+\n+\tif (strcmp(mode, \"w\"))\n+\t\tBUG(\"write-in-file does not support '%s' mode\", mode);\n+\tfp = fopen(path, mode);\n+\tif (!fp)\n+\t\treturn error_errno(_(\"cannot open file '%s' in mode '%s'\"), path, mode);\n+\tres = vfprintf(fp, format, args);\n+\n+\tif (res < 0) {\n+\t\tint saved_errno = errno;\n+\t\tfclose(fp);\n+\t\terrno = saved_errno;\n+\t\treturn error_errno(_(\"could not write to file '%s'\"), path);\n+\t}\n+\n+\treturn fclose(fp);\n+}\n+\n+static int write_to_file(const char *path, const char *format, ...)\n+{\n+\tint res;\n+\tva_list args;\n+\n+\tva_start(args, format);\n+\tres = write_in_file(path, \"w\", format, args);\n+\tva_end(args);\n+\n+\treturn res;\n+}\n+\n static int check_term_format(const char *term, const char *orig_term)\n {\n \tint res;\n@@ -104,7 +138,6 @@ static int check_term_format(const char *term, const char *orig_term)\n \n static int write_terms(const char *bad, const char *good)\n {\n-\tFILE *fp = NULL;\n \tint res;\n \n \tif (!strcmp(bad, good))\n@@ -113,13 +146,9 @@ static int write_terms(const char *bad, const char *good)\n \tif (check_term_format(bad, \"bad\") || check_term_format(good, \"good\"))\n \t\treturn -1;\n \n-\tfp = fopen(git_path_bisect_terms(), \"w\");\n-\tif (!fp)\n-\t\treturn error_errno(_(\"could not open the file BISECT_TERMS\"));\n+\tres = write_to_file(git_path_bisect_terms(), \"%s\\n%s\\n\", bad, good);\n \n-\tres = fprintf(fp, \"%s\\n%s\\n\", bad, good);\n-\tres |= fclose(fp);\n-\treturn (res < 0) ? -1 : 0;\n+\treturn res;\n }\n \n static int is_expected_rev(const char *expected_hex)\n-- \n2.25.0\n\n"},{"id":"406259","messageId":"20200924123340.52632-6-mirucam@gmail.com","threadId":"54286","inReplyTo":"20200924123340.52632-1-mirucam@gmail.com","subject":"[PATCH v9 5/6] bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-24T12:33:39Z","receivedAt":"2020-09-24T12:34:06Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"As there can be other revision walks after bisect_next_all(),\nlet's add a call to a function to clear all the marks at the\nend of bisect_next_all().\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Miriam Rubio <mirucam@gmail.com>\n---\n bisect.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/bisect.c b/bisect.c\nindex d42a3a3767..c6aba2b9f2 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1082,6 +1082,8 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)\n \t\t  \"Bisecting: %d revisions left to test after this %s\\n\",\n \t\t  nr), nr, steps_msg);\n \tfree(steps_msg);\n+\t/* Clean up objects used, as they will be reused. */\n+\tclear_commit_marks_all(ALL_REV_FLAGS);\n \n \treturn bisect_checkout(bisect_rev, no_checkout);\n }\n-- \n2.25.0\n\n"},{"id":"406260","messageId":"20200924123340.52632-5-mirucam@gmail.com","threadId":"54286","inReplyTo":"20200924123340.52632-1-mirucam@gmail.com","subject":"[PATCH v9 4/6] bisect--helper: reimplement `bisect_autostart` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-24T12:33:38Z","receivedAt":"2020-09-24T12:34:07Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva <pranit.bauva@gmail.com>\n\nReimplement the `bisect_autostart()` shell function in C and add the\nC implementation from `bisect_next()` which was previously left\nuncovered.\n\nAdd `--bisect-autostart` subcommand to be called from git-bisect.sh.\nUsing `--bisect-autostart` subcommand is a temporary measure to port\nthe shell function to C so as to use the existing test suite. As more\nfunctions are ported, this subcommand will be retired and\nbisect_autostart() will be called directly by `bisect_state()`.\n\nChange behavior of shell script that returned success when user aborted\nthe bisection.\n\nMentored-by: Lars Schneider <larsxschneider@gmail.com>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nMentored-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Pranit Bauva <pranit.bauva@gmail.com>\nSigned-off-by: Tanushree Tumane <tanushreetumane@gmail.com>\nSigned-off-by: Miriam Rubio <mirucam@gmail.com>\n---\n builtin/bisect--helper.c | 44 +++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh            | 25 ++---------------------\n 2 files changed, 45 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 46d2abb36f..ab27a8dff6 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -29,6 +29,7 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n \tN_(\"git bisect--helper --bisect-start [--term-{new,bad}=<term> --term-{old,good}=<term>]\"\n \t\t\t\t\t    \" [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]\"),\n+\tN_(\"git bisect--helper --bisect-autostart\"),\n \tNULL\n };\n \n@@ -653,6 +654,38 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n \treturn res;\n }\n \n+static inline int file_is_not_empty(const char *path)\n+{\n+\treturn !is_empty_or_missing_file(path);\n+}\n+\n+static int bisect_autostart(struct bisect_terms *terms)\n+{\n+\tint res;\n+\tconst char *yesno;\n+\n+\tif (file_is_not_empty(git_path_bisect_start()))\n+\t\treturn 0;\n+\n+\tfprintf_ln(stderr, _(\"You need to start by \\\"git bisect \"\n+\t\t\t  \"start\\\"\\n\"));\n+\n+\tif (!isatty(STDIN_FILENO))\n+\t\treturn -1;\n+\n+\t/*\n+\t * TRANSLATORS: Make sure to include [Y] and [n] in your\n+\t * translation. The program will only accept English input\n+\t * at this point.\n+\t */\n+\tyesno = git_prompt(_(\"Do you want me to do it for you \"\n+\t\t\t     \"[Y/n]? \"), PROMPT_ECHO);\n+\tres = tolower(*yesno) == 'n' ?\n+\t\t-1 : bisect_start(terms, empty_strvec, 0);\n+\n+\treturn res;\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -665,7 +698,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tCHECK_AND_SET_TERMS,\n \t\tBISECT_NEXT_CHECK,\n \t\tBISECT_TERMS,\n-\t\tBISECT_START\n+\t\tBISECT_START,\n+\t\tBISECT_AUTOSTART,\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -689,6 +723,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"print out the bisect terms\"), BISECT_TERMS),\n \t\tOPT_CMDMODE(0, \"bisect-start\", &cmdmode,\n \t\t\t N_(\"start the bisect session\"), BISECT_START),\n+\t\tOPT_CMDMODE(0, \"bisect-autostart\", &cmdmode,\n+\t\t\t N_(\"start the bisection if it has not yet been started\"), BISECT_AUTOSTART),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -748,6 +784,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tset_terms(&terms, \"bad\", \"good\");\n \t\tres = bisect_start(&terms, argv, argc);\n \t\tbreak;\n+\tcase BISECT_AUTOSTART:\n+\t\tif (argc)\n+\t\t\treturn error(_(\"--bisect-autostart does not accept arguments\"));\n+\t\tset_terms(&terms, \"bad\", \"good\");\n+\t\tres = bisect_autostart(&terms);\n+\t\tbreak;\n \tdefault:\n \t\tBUG(\"unknown subcommand %d\", cmdmode);\n \t}\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 2f60fefcfa..6ed3e42fc2 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -49,27 +49,6 @@ bisect_head()\n \tfi\n }\n \n-bisect_autostart() {\n-\ttest -s \"$GIT_DIR/BISECT_START\" || {\n-\t\tgettextln \"You need to start by \\\"git bisect start\\\"\" >&2\n-\t\tif test -t 0\n-\t\tthen\n-\t\t\t# TRANSLATORS: Make sure to include [Y] and [n] in your\n-\t\t\t# translation. The program will only accept English input\n-\t\t\t# at this point.\n-\t\t\tgettext \"Do you want me to do it for you [Y/n]? \" >&2\n-\t\t\tread yesno\n-\t\t\tcase \"$yesno\" in\n-\t\t\t[Nn]*)\n-\t\t\t\texit ;;\n-\t\t\tesac\n-\t\t\tbisect_start\n-\t\telse\n-\t\t\texit 1\n-\t\tfi\n-\t}\n-}\n-\n bisect_start() {\n \tgit bisect--helper --bisect-start $@ || exit\n \n@@ -108,7 +87,7 @@ bisect_skip() {\n }\n \n bisect_state() {\n-\tbisect_autostart\n+\tgit bisect--helper --bisect-autostart || exit\n \tstate=$1\n \tgit bisect--helper --check-and-set-terms $state $TERM_GOOD $TERM_BAD || exit\n \tget_terms\n@@ -149,7 +128,7 @@ bisect_auto_next() {\n \n bisect_next() {\n \tcase \"$#\" in 0) ;; *) usage ;; esac\n-\tbisect_autostart\n+\tgit bisect--helper --bisect-autostart || exit\n \tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD $TERM_GOOD|| exit\n \n \t# Perform all bisection computation, display and checkout\n-- \n2.25.0\n\n"},{"id":"406261","messageId":"20200924123340.52632-7-mirucam@gmail.com","threadId":"54286","inReplyTo":"20200924123340.52632-1-mirucam@gmail.com","subject":"[PATCH v9 6/6] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-24T12:33:40Z","receivedAt":"2020-09-24T12:34:12Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva <pranit.bauva@gmail.com>\n\nReimplement the `bisect_next()` and the `bisect_auto_next()` shell functions\nin C and add the subcommands to `git bisect--helper` to call them from\ngit-bisect.sh .\n\nbisect_auto_next() function returns an enum bisect_error type as whole\n`git bisect` can exit with an error code when bisect_next() does.\n\nReturn an error when `bisect_next()` fails, that fix a bug on shell script\nversion.\n\nUsing `--bisect-next` and `--bisect-auto-next` subcommands is a\ntemporary measure to port shell function to C so as to use the existing\ntest suite. As more functions are ported, `--bisect-auto-next`\nsubcommand will be retired and will be called by some other methods.\n\nMentored-by: Lars Schneider <larsxschneider@gmail.com>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nMentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Pranit Bauva <pranit.bauva@gmail.com>\nSigned-off-by: Tanushree Tumane <tanushreetumane@gmail.com>\nSigned-off-by: Miriam Rubio <mirucam@gmail.com>\n---\n bisect.c                 |  11 ++-\n builtin/bisect--helper.c | 180 ++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh            |  47 +---------\n 3 files changed, 191 insertions(+), 47 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex c6aba2b9f2..f5b1368128 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -988,8 +988,11 @@ void read_bisect_terms(const char **read_bad, const char **read_good)\n  * the bisection process finished successfully.\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+ * Checking BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND\n+ * in bisect_helper::bisect_next() and only transforming it to 0 at\n+ * the end of bisect_helper::cmd_bisect__helper() helps bypassing\n+ * all the code related to finding a commit to test.\n  */\n enum bisect_error bisect_next_all(struct repository *r, const char *prefix)\n {\n@@ -999,6 +1002,10 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n+\t/*\n+\t * If no_checkout is non-zero, the bisection process does not\n+\t * checkout the trial commit but instead simply updates BISECT_HEAD.\n+\t */\n \tint no_checkout = ref_exists(\"BISECT_HEAD\");\n \tunsigned bisect_flags = 0;\n \ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex ab27a8dff6..3a5bf99257 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -8,6 +8,7 @@\n #include \"run-command.h\"\n #include \"prompt.h\"\n #include \"quote.h\"\n+#include \"revision.h\"\n \n static GIT_PATH_FUNC(git_path_bisect_terms, \"BISECT_TERMS\")\n static GIT_PATH_FUNC(git_path_bisect_expected_rev, \"BISECT_EXPECTED_REV\")\n@@ -29,10 +30,17 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n \tN_(\"git bisect--helper --bisect-start [--term-{new,bad}=<term> --term-{old,good}=<term>]\"\n \t\t\t\t\t    \" [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]\"),\n+\tN_(\"git bisect--helper --bisect-next\"),\n+\tN_(\"git bisect--helper --bisect-auto-next\"),\n \tN_(\"git bisect--helper --bisect-autostart\"),\n \tNULL\n };\n \n+struct add_bisect_ref_data {\n+\tstruct rev_info *revs;\n+\tunsigned int object_flags;\n+};\n+\n struct bisect_terms {\n \tchar *term_good;\n \tchar *term_bad;\n@@ -56,6 +64,8 @@ static void set_terms(struct bisect_terms *terms, const char *bad,\n static const char vocab_bad[] = \"bad|new\";\n static const char vocab_good[] = \"good|old\";\n \n+static int bisect_autostart(struct bisect_terms *terms);\n+\n /*\n  * Check whether the string `term` belongs to the set of strings\n  * included in the variable arguments.\n@@ -80,7 +90,7 @@ static int write_in_file(const char *path, const char *mode, const char *format,\n \tFILE *fp = NULL;\n \tint res = 0;\n \n-\tif (strcmp(mode, \"w\"))\n+\tif (strcmp(mode, \"w\") && strcmp(mode, \"a\"))\n \t\tBUG(\"write-in-file does not support '%s' mode\", mode);\n \tfp = fopen(path, mode);\n \tif (!fp)\n@@ -109,6 +119,18 @@ static int write_to_file(const char *path, const char *format, ...)\n \treturn res;\n }\n \n+static int append_to_file(const char *path, const char *format, ...)\n+{\n+\tint res;\n+\tva_list args;\n+\n+\tva_start(args, format);\n+\tres = write_in_file(path, \"a\", format, args);\n+\tva_end(args);\n+\n+\treturn res;\n+}\n+\n static int check_term_format(const char *term, const char *orig_term)\n {\n \tint res;\n@@ -451,6 +473,142 @@ static int bisect_append_log_quoted(const char **argv)\n \treturn res;\n }\n \n+static int add_bisect_ref(const char *refname, const struct object_id *oid,\n+\t\t\t  int flags, void *cb)\n+{\n+\tstruct add_bisect_ref_data *data = cb;\n+\n+\tadd_pending_oid(data->revs, refname, oid, data->object_flags);\n+\n+\treturn 0;\n+}\n+\n+static int prepare_revs(struct bisect_terms *terms, struct rev_info *revs)\n+{\n+\tint res = 0;\n+\tstruct add_bisect_ref_data cb = { revs };\n+\tchar *good = xstrfmt(\"%s-*\", terms->term_good);\n+\n+\t/*\n+\t * We cannot use terms->term_bad directly in\n+\t * for_each_glob_ref_in() and we have to append a '*' to it,\n+\t * otherwise for_each_glob_ref_in() will append '/' and '*'.\n+\t */\n+\tchar *bad = xstrfmt(\"%s*\", terms->term_bad);\n+\n+\t/*\n+\t * It is important to reset the flags used by revision walks\n+\t * as the previous call to bisect_next_all() in turn\n+\t * sets up a revision walk.\n+\t */\n+\treset_revision_walk();\n+\tinit_revisions(revs, NULL);\n+\tsetup_revisions(0, NULL, revs, NULL);\n+\tfor_each_glob_ref_in(add_bisect_ref, bad, \"refs/bisect/\", &cb);\n+\tcb.object_flags = UNINTERESTING;\n+\tfor_each_glob_ref_in(add_bisect_ref, good, \"refs/bisect/\", &cb);\n+\tif (prepare_revision_walk(revs))\n+\t\tres = error(_(\"revision walk setup failed\\n\"));\n+\n+\tfree(good);\n+\tfree(bad);\n+\treturn res;\n+}\n+\n+static int bisect_skipped_commits(struct bisect_terms *terms)\n+{\n+\tint res;\n+\tFILE *fp = NULL;\n+\tstruct rev_info revs;\n+\tstruct commit *commit;\n+\tstruct pretty_print_context pp = {0};\n+\tstruct strbuf commit_name = STRBUF_INIT;\n+\n+\tres = prepare_revs(terms, &revs);\n+\tif (res)\n+\t\treturn res;\n+\n+\tfp = fopen(git_path_bisect_log(), \"a\");\n+\tif (!fp)\n+\t\treturn error_errno(_(\"could not open '%s' for appending\"),\n+\t\t\t\t  git_path_bisect_log());\n+\n+\tif (fprintf(fp, \"# only skipped commits left to test\\n\") < 0)\n+\t\treturn error_errno(_(\"failed to write to '%s'\"), git_path_bisect_log());\n+\n+\twhile ((commit = get_revision(&revs)) != NULL) {\n+\t\tstrbuf_reset(&commit_name);\n+\t\tformat_commit_message(commit, \"%s\",\n+\t\t\t\t      &commit_name, &pp);\n+\t\tfprintf(fp, \"# possible first %s commit: [%s] %s\\n\",\n+\t\t\tterms->term_bad, oid_to_hex(&commit->object.oid),\n+\t\t\tcommit_name.buf);\n+\t}\n+\n+\t/*\n+\t * Reset the flags used by revision walks in case\n+\t * there is another revision walk after this one.\n+\t */\n+\treset_revision_walk();\n+\n+\tstrbuf_release(&commit_name);\n+\tfclose(fp);\n+\treturn 0;\n+}\n+\n+static int bisect_successful(struct bisect_terms *terms)\n+{\n+\tstruct object_id oid;\n+\tstruct commit *commit;\n+\tstruct pretty_print_context pp = {0};\n+\tstruct strbuf commit_name = STRBUF_INIT;\n+\tchar *bad_ref = xstrfmt(\"refs/bisect/%s\",terms->term_bad);\n+\tint res;\n+\n+\tread_ref(bad_ref, &oid);\n+\tcommit = lookup_commit_reference_by_name(bad_ref);\n+\tformat_commit_message(commit, \"%s\", &commit_name, &pp);\n+\n+\tres = append_to_file(git_path_bisect_log(), \"# first %s commit: [%s] %s\\n\",\n+\t\t\t    terms->term_bad, oid_to_hex(&commit->object.oid),\n+\t\t\t    commit_name.buf);\n+\n+\tstrbuf_release(&commit_name);\n+\tfree(bad_ref);\n+\treturn res;\n+}\n+\n+static enum bisect_error bisect_next(struct bisect_terms *terms, const char *prefix)\n+{\n+\tenum bisect_error res;\n+\n+\tif (bisect_autostart(terms))\n+\t\treturn BISECT_FAILED;\n+\n+\tif (bisect_next_check(terms, terms->term_good))\n+\t\treturn BISECT_FAILED;\n+\n+\t/* Perform all bisection computation */\n+\tres = bisect_next_all(the_repository, prefix);\n+\n+\tif (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {\n+\t\tres = bisect_successful(terms);\n+\t\treturn res ? res : BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;\n+\t} else if (res == BISECT_ONLY_SKIPPED_LEFT) {\n+\t\tres = bisect_skipped_commits(terms);\n+\t\treturn res ? res : BISECT_ONLY_SKIPPED_LEFT;\n+\t}\n+\treturn res;\n+}\n+\n+static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char *prefix)\n+{\n+\tif (bisect_next_check(terms, NULL))\n+\t\treturn BISECT_OK;\n+\n+\treturn bisect_next(terms, prefix);\n+}\n+\n static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n {\n \tint no_checkout = 0;\n@@ -700,6 +858,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_TERMS,\n \t\tBISECT_START,\n \t\tBISECT_AUTOSTART,\n+\t\tBISECT_NEXT,\n+\t\tBISECT_AUTO_NEXT\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -723,6 +883,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"print out the bisect terms\"), BISECT_TERMS),\n \t\tOPT_CMDMODE(0, \"bisect-start\", &cmdmode,\n \t\t\t N_(\"start the bisect session\"), BISECT_START),\n+\t\tOPT_CMDMODE(0, \"bisect-next\", &cmdmode,\n+\t\t\t N_(\"find the next bisection commit\"), BISECT_NEXT),\n+\t\tOPT_CMDMODE(0, \"bisect-auto-next\", &cmdmode,\n+\t\t\t N_(\"verify the next bisection state then checkout the next bisection commit\"), BISECT_AUTO_NEXT),\n \t\tOPT_CMDMODE(0, \"bisect-autostart\", &cmdmode,\n \t\t\t N_(\"start the bisection if it has not yet been started\"), BISECT_AUTOSTART),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n@@ -784,6 +948,18 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tset_terms(&terms, \"bad\", \"good\");\n \t\tres = bisect_start(&terms, argv, argc);\n \t\tbreak;\n+\tcase BISECT_NEXT:\n+\t\tif (argc)\n+\t\t\treturn error(_(\"--bisect-next requires 0 arguments\"));\n+\t\tget_terms(&terms);\n+\t\tres = bisect_next(&terms, prefix);\n+\t\tbreak;\n+\tcase BISECT_AUTO_NEXT:\n+\t\tif (argc)\n+\t\t\treturn error(_(\"--bisect-auto-next requires 0 arguments\"));\n+\t\tget_terms(&terms);\n+\t\tres = bisect_auto_next(&terms, prefix);\n+\t\tbreak;\n \tcase BISECT_AUTOSTART:\n \t\tif (argc)\n \t\t\treturn error(_(\"--bisect-autostart does not accept arguments\"));\n@@ -799,7 +975,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\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+\tif ((res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) || (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND))\n \t\tres = BISECT_OK;\n \n \treturn -res;\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 6ed3e42fc2..ea7e684ebb 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -65,8 +65,7 @@ bisect_start() {\n \t#\n \t# Check if we can proceed to the next bisect state.\n \t#\n-\tget_terms\n-\tbisect_auto_next\n+\tgit bisect--helper --bisect-auto-next || exit\n \n \ttrap '-' 0\n }\n@@ -119,45 +118,7 @@ bisect_state() {\n \t*)\n \t\tusage ;;\n \tesac\n-\tbisect_auto_next\n-}\n-\n-bisect_auto_next() {\n-\tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD && bisect_next || :\n-}\n-\n-bisect_next() {\n-\tcase \"$#\" in 0) ;; *) usage ;; esac\n-\tgit bisect--helper --bisect-autostart || exit\n-\tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD $TERM_GOOD|| exit\n-\n-\t# Perform all bisection computation, display and checkout\n-\tgit bisect--helper --next-all\n-\tres=$?\n-\n-\t# Check if we should exit because bisection is finished\n-\tif test $res -eq 10\n-\tthen\n-\t\tbad_rev=$(git show-ref --hash --verify refs/bisect/$TERM_BAD)\n-\t\tbad_commit=$(git show-branch $bad_rev)\n-\t\techo \"# first $TERM_BAD commit: $bad_commit\" >>\"$GIT_DIR/BISECT_LOG\"\n-\t\texit 0\n-\telif test $res -eq 2\n-\tthen\n-\t\techo \"# only skipped commits left to test\" >>\"$GIT_DIR/BISECT_LOG\"\n-\t\tgood_revs=$(git for-each-ref --format=\"%(objectname)\" \"refs/bisect/$TERM_GOOD-*\")\n-\t\tfor skipped in $(git rev-list refs/bisect/$TERM_BAD --not $good_revs)\n-\t\tdo\n-\t\t\tskipped_commit=$(git show-branch $skipped)\n-\t\t\techo \"# possible first $TERM_BAD commit: $skipped_commit\" >>\"$GIT_DIR/BISECT_LOG\"\n-\t\tdone\n-\t\texit $res\n-\tfi\n-\n-\t# Check for an error in the bisection process\n-\ttest $res -ne 0 && exit $res\n-\n-\treturn 0\n+\tgit bisect--helper --bisect-auto-next\n }\n \n bisect_visualize() {\n@@ -213,7 +174,7 @@ bisect_replay () {\n \t\tesac\n \tdone <\"$file\"\n \tIFS=\"$oIFS\"\n-\tbisect_auto_next\n+\tgit bisect--helper --bisect-auto-next || exit\n }\n \n bisect_run () {\n@@ -310,7 +271,7 @@ case \"$#\" in\n \t\tbisect_skip \"$@\" ;;\n \tnext)\n \t\t# Not sure we want \"next\" at the UI level anymore.\n-\t\tbisect_next \"$@\" ;;\n+\t\tgit bisect--helper --bisect-next \"$@\" || exit ;;\n \tvisualize|view)\n \t\tbisect_visualize \"$@\" ;;\n \treset)\n-- \n2.25.0\n\n"},{"id":"406643","messageId":"xmqq5z7wi9pc.fsf@gitster.c.googlers.com","threadId":"54286","inReplyTo":"20200924123340.52632-1-mirucam@gmail.com","subject":"Re: [PATCH v9 0/6]Finish converting git bisect to C part 2 subset 1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-29T20:55:59Z","receivedAt":"2020-09-29T20:56:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miriam Rubio <mirucam@gmail.com> writes:\n\n> These patches correspond to a second 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 patch series version is composed by the first six patches of the\n> previous version due to a bug discovered during the test stage, that \n> affects the rest of the patches and that needs to be fixed before \n> sending them.\n>\n> These patch series emails were generated from:\n> https://gitlab.com/mirucam/git/commits/git-bisect-work-part2-v9-subset1.\n>\n> I would like to thank Johannes Schindelin for reviewing this patch series\n> and to Christian Couder for his advice and help.\n>\n> General information\n> -------------------\n>\n> * Rebased on 385c171a01, (Fifteenth batch, 2020-09-18).\n>\n> Specific changes\n> ----------------\n>\n> [4/6] bisect--helper: reimplement `bisect_autostart` shell function in C\n>\n> * Add `|| exit` to bisect-autostart call in shell script.\n>\n> ---\n\nThis hasn't seen any responses, but seeing the difference between v8\nand v9 is so small and obviously good, and there weren't anything\nglaringly wrong spotted in v8, I'd say this is ready to advance to\n'next'.\n\nAnything I missed in the previous or this review round?\n\nThanks.\n\n>\n> Miriam Rubio (4):\n>   bisect--helper: BUG() in cmd_*() on invalid subcommand\n>   bisect--helper: use '-res' in 'cmd_bisect__helper' return\n>   bisect--helper: introduce new `write_in_file()` function\n>   bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'\n>\n> Pranit Bauva (2):\n>   bisect--helper: reimplement `bisect_autostart` shell function in C\n>   bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell\n>     functions in C\n>\n>  bisect.c                 |  13 +-\n>  builtin/bisect--helper.c | 269 +++++++++++++++++++++++++++++++++++++--\n>  git-bisect.sh            |  70 +---------\n>  3 files changed, 274 insertions(+), 78 deletions(-)\n"}]}