{"thread":{"id":"54150","subject":"[PATCH v6 00/13] Finish converting git bisect to C part 2","startedAt":"2020-08-28T12:47:26Z","lastAt":"2020-08-31T17:09:18Z","messageCount":18,"participants":["Miriam Rubio","Junio C Hamano","Miriam R."],"isPatch":true,"patchVersion":6,"patchTotal":13},"messages":[{"id":"404683","messageId":"20200828124617.60618-1-mirucam@gmail.com","threadId":"54150","inReplyTo":null,"subject":"[PATCH v6 00/13] Finish converting git bisect to C part 2","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:04Z","receivedAt":"2020-08-28T12:47:26Z","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\nThese patch series emails were generated from:\nhttps://gitlab.com/mirucam/git/commits/git-bisect-work-part2-v6ps.\n\nI would like to thank the reviewers for their help.\n\nGeneral changes\n---------------\n\n* Rebase on master branch: e9b77c84a0 (Tenth batch, 2020-08-24)\n\nSpecific changes\n----------------\n\n[1/13] bisect--helper: BUG() in cmd_*() on invalid subcommand\n\n* Amend commit message.\n* Remove casting to int.\n\n---\n\n[2/13] bisect--helper: use '-res' in 'cmd_bisect__helper' return\n\n* Amend commit message.\n\n---\n\n[3/13] bisect--helper: introduce new `write_in_file()` function\n\n* Use saved_errno variable.\n\n---\n\n[5/13] bisect--helper: reimplement `bisect_autostart` shell function in C\n\n* Fix bug using empty_strvec instead of NULL in a `bisect_start()` call.\n\n---\n\n\n[6/13] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell\n functions in C\n \n* Change `file_exists(git_path_bisect_head())` to a refs API call.\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 (9):\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  bisect--helper: finish porting `bisect_start()` to C\n  bisect--helper: retire `--bisect-clean-state` subcommand\n  bisect--helper: retire `--next-all` subcommand\n  bisect--helper: reimplement `bisect_state` & `bisect_head` shell\n    functions in C\n  bisect--helper: retire `--check-expected-revs` subcommand\n  bisect--helper: retire `--write-terms` subcommand\n  bisect--helper: retire `--bisect-autostart` subcommand\n\n bisect.c                 |   8 +\n builtin/bisect--helper.c | 445 ++++++++++++++++++++++++++++++++-------\n git-bisect.sh            | 145 +------------\n 3 files changed, 380 insertions(+), 218 deletions(-)\n\n-- \n2.25.0\n\n"},{"id":"404684","messageId":"20200828124617.60618-2-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 01/13] bisect--helper: BUG() in cmd_*() on invalid subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:05Z","receivedAt":"2020-08-28T12:47:30Z","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 cdda279b23..f464e95792 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":"404685","messageId":"20200828124617.60618-3-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 02/13] bisect--helper: use '-res' in 'cmd_bisect__helper' return","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:06Z","receivedAt":"2020-08-28T12:47:34Z","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 f464e95792..b7345be3a5 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":"404686","messageId":"20200828124617.60618-4-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 03/13] bisect--helper: introduce new `write_in_file()` function","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:07Z","receivedAt":"2020-08-28T12:47:38Z","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 b7345be3a5..bae09ce65d 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":"404687","messageId":"20200828124617.60618-6-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 05/13] bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:09Z","receivedAt":"2020-08-28T12:47:42Z","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":"404688","messageId":"20200828124617.60618-5-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 04/13] bisect--helper: reimplement `bisect_autostart` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:08Z","receivedAt":"2020-08-28T12:48:14Z","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\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 bae09ce65d..f71e8e54d2 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-{old,good}=<term> --term-{new,bad}=<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 0;\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 c7580e51a0..9ca583d964 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\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\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":"404689","messageId":"20200828124617.60618-7-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 06/13] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:10Z","receivedAt":"2020-08-28T12:48:34Z","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\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                 |   6 ++\n builtin/bisect--helper.c | 183 ++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh            |  47 +---------\n 3 files changed, 190 insertions(+), 46 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex c6aba2b9f2..f0fca5c6f3 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -988,6 +988,12 @@ 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+ *\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  * If no_checkout is non-zero, the bisection process does not\n  * checkout the trial commit but instead simply updates BISECT_HEAD.\n  */\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex f71e8e54d2..5d443829e0 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-{old,good}=<term> --term-{new,bad}=<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,143 @@ 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+\tint no_checkout;\n+\tenum bisect_error res;\n+\n+\tbisect_autostart(terms);\n+\tif (bisect_next_check(terms, terms->term_good))\n+\t\treturn BISECT_FAILED;\n+\n+\tno_checkout = ref_exists(\"BISECT_HEAD\");\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@@ -699,7 +858,9 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_NEXT_CHECK,\n \t\tBISECT_TERMS,\n \t\tBISECT_START,\n-\t\tBISECT_AUTOSTART,\n+\t\tBISECT_NEXT,\n+\t\tBISECT_AUTO_NEXT,\n+\t\tBISECT_AUTOSTART\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -723,6 +884,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 +949,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 +976,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 9ca583d964..59424f5b37 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\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\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":"404690","messageId":"20200828124617.60618-8-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 07/13] bisect--helper: finish porting `bisect_start()` to C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:11Z","receivedAt":"2020-08-28T12:48:47Z","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\nAdd the subcommand to `git bisect--helper` and call it from\ngit-bisect.sh.\n\nWith the conversion of `bisect_auto_next()` from shell to C in a\nprevious commit, `bisect_start()` can now be fully ported to C.\n\nSo let's complete the `--bisect-start` subcommand of\n`git bisect--helper` so that it fully implements `bisect_start()`,\nand let's use this subcommand in `git-bisect.sh` instead of\n`bisect_start()`.\n\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 | 56 ++++++++++++++++++++++++++++------------\n git-bisect.sh            | 26 ++-----------------\n 2 files changed, 41 insertions(+), 41 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 5d443829e0..56da407871 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -85,6 +85,19 @@ static int one_of(const char *term, ...)\n \treturn res;\n }\n \n+/*\n+ * return code BISECT_INTERNAL_SUCCESS_MERGE_BASE\n+ * and BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND are codes\n+ * that indicate special success.\n+ */\n+\n+static int is_bisect_success(enum bisect_error res)\n+{\n+\treturn !res ||\n+\t\tres == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND ||\n+\t\tres == BISECT_INTERNAL_SUCCESS_MERGE_BASE;\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@@ -610,12 +623,13 @@ static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char\n \treturn bisect_next(terms, prefix);\n }\n \n-static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n+static enum bisect_error bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n {\n \tint no_checkout = 0;\n \tint first_parent_only = 0;\n \tint i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;\n-\tint flags, pathspec_pos, res = 0;\n+\tint flags, pathspec_pos;\n+\tenum bisect_error res = BISECT_OK;\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@@ -675,9 +689,12 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n \t\t\treturn error(_(\"unrecognized option: '%s'\"), arg);\n \t\t} else {\n \t\t\tchar *commit_id = xstrfmt(\"%s^{commit}\", arg);\n-\t\t\tif (get_oid(commit_id, &oid) && has_double_dash)\n-\t\t\t\tdie(_(\"'%s' does not appear to be a valid \"\n-\t\t\t\t      \"revision\"), arg);\n+\t\t\tif (get_oid(commit_id, &oid) && has_double_dash) {\n+\t\t\t\terror(_(\"'%s' does not appear to be a valid \"\n+\t\t\t\t\t\"revision\"), arg);\n+\t\t\t\tfree(commit_id);\n+\t\t\t\treturn BISECT_FAILED;\n+\t\t\t}\n \n \t\t\tstring_list_append(&revs, oid_to_hex(&oid));\n \t\t\tfree(commit_id);\n@@ -755,14 +772,7 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n \t * Get rid of any old bisect state.\n \t */\n \tif (bisect_clean_state())\n-\t\treturn -1;\n-\n-\t/*\n-\t * In case of mistaken revs or checkout error, or signals received,\n-\t * \"bisect_auto_next\" below may exit or misbehave.\n-\t * We have to trap this to be able to clean up using\n-\t * \"bisect_clean_state\".\n-\t */\n+\t\treturn BISECT_FAILED;\n \n \t/*\n \t * Write new start state\n@@ -779,7 +789,7 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\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\tres = -1;\n+\t\t\tres = BISECT_FAILED;\n \t\t\tgoto finish;\n \t\t}\n \t}\n@@ -791,25 +801,37 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\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\tres = -1;\n+\t\t\tres = BISECT_FAILED;\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\tres = -1;\n+\t\tres = BISECT_FAILED;\n \t\tgoto finish;\n \t}\n \n \tres = bisect_append_log_quoted(argv);\n \tif (res)\n-\t\tres = -1;\n+\t\tres = BISECT_FAILED;\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+\tif (res)\n+\t\treturn res;\n+\n+\tres = bisect_auto_next(terms, NULL);\n+\t/*\n+\t * In case of mistaken revs or checkout error,\n+\t * \"bisect_auto_next\" above may exit or misbehave.\n+\t * We have to handle this to be able to clean up using\n+\t * \"bisect_clean_state\".\n+\t */\n+\tif (!is_bisect_success(res))\n+\t\tbisect_clean_state();\n \treturn res;\n }\n \ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 59424f5b37..356264caf0 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -49,27 +49,6 @@ bisect_head()\n \tfi\n }\n \n-bisect_start() {\n-\tgit bisect--helper --bisect-start $@ || exit\n-\n-\t#\n-\t# Change state.\n-\t# In case of mistaken revs or checkout error, or signals received,\n-\t# \"bisect_auto_next\" below may exit or misbehave.\n-\t# We have to trap this to be able to clean up using\n-\t# \"bisect_clean_state\".\n-\t#\n-\ttrap 'git bisect--helper --bisect-clean-state' 0\n-\ttrap 'exit 255' 1 2 3 15\n-\n-\t#\n-\t# Check if we can proceed to the next bisect state.\n-\t#\n-\tgit bisect--helper --bisect-auto-next || exit\n-\n-\ttrap '-' 0\n-}\n-\n bisect_skip() {\n \tall=''\n \tfor arg in \"$@\"\n@@ -163,8 +142,7 @@ bisect_replay () {\n \t\tget_terms\n \t\tcase \"$command\" in\n \t\tstart)\n-\t\t\tcmd=\"bisect_start $rev $tail\"\n-\t\t\teval \"$cmd\" ;;\n+\t\t\teval \"git bisect--helper --bisect-start $rev $tail\" ;;\n \t\t\"$TERM_GOOD\"|\"$TERM_BAD\"|skip)\n \t\t\tgit bisect--helper --bisect-write \"$command\" \"$rev\" \"$TERM_GOOD\" \"$TERM_BAD\" || exit;;\n \t\tterms)\n@@ -264,7 +242,7 @@ case \"$#\" in\n \thelp)\n \t\tgit bisect -h ;;\n \tstart)\n-\t\tbisect_start \"$@\" ;;\n+\t\tgit bisect--helper --bisect-start \"$@\" ;;\n \tbad|good|new|old|\"$TERM_BAD\"|\"$TERM_GOOD\")\n \t\tbisect_state \"$cmd\" \"$@\" ;;\n \tskip)\n-- \n2.25.0\n\n"},{"id":"404691","messageId":"20200828124617.60618-9-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 08/13] bisect--helper: retire `--bisect-clean-state` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:12Z","receivedAt":"2020-08-28T12:48:53Z","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\nThe `--bisect-clean-state` subcommand is no longer used from the\ngit-bisect.sh shell script. Instead the function\n`bisect_clean_state()` is directly called from the C\nimplementation.\n\nMentored-by: Lars Schneider <larsxschneider@gmail.com>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\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 | 8 --------\n 1 file changed, 8 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 56da407871..106e5b788e 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -22,7 +22,6 @@ static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --next-all\"),\n \tN_(\"git bisect--helper --write-terms <bad_term> <good_term>\"),\n-\tN_(\"git bisect--helper --bisect-clean-state\"),\n \tN_(\"git bisect--helper --bisect-reset [<commit>]\"),\n \tN_(\"git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>\"),\n \tN_(\"git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>\"),\n@@ -872,7 +871,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \tenum {\n \t\tNEXT_ALL = 1,\n \t\tWRITE_TERMS,\n-\t\tBISECT_CLEAN_STATE,\n \t\tCHECK_EXPECTED_REVS,\n \t\tBISECT_RESET,\n \t\tBISECT_WRITE,\n@@ -890,8 +888,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"perform 'git bisect next'\"), NEXT_ALL),\n \t\tOPT_CMDMODE(0, \"write-terms\", &cmdmode,\n \t\t\t N_(\"write the terms to .git/BISECT_TERMS\"), WRITE_TERMS),\n-\t\tOPT_CMDMODE(0, \"bisect-clean-state\", &cmdmode,\n-\t\t\t N_(\"cleanup the bisection state\"), BISECT_CLEAN_STATE),\n \t\tOPT_CMDMODE(0, \"check-expected-revs\", &cmdmode,\n \t\t\t N_(\"check for expected revs\"), CHECK_EXPECTED_REVS),\n \t\tOPT_CMDMODE(0, \"bisect-reset\", &cmdmode,\n@@ -933,10 +929,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tif (argc != 2)\n \t\t\treturn error(_(\"--write-terms requires two arguments\"));\n \t\treturn write_terms(argv[0], argv[1]);\n-\tcase BISECT_CLEAN_STATE:\n-\t\tif (argc != 0)\n-\t\t\treturn error(_(\"--bisect-clean-state requires no arguments\"));\n-\t\treturn bisect_clean_state();\n \tcase CHECK_EXPECTED_REVS:\n \t\tcheck_expected_revs(argv, argc);\n \t\treturn 0;\n-- \n2.25.0\n\n"},{"id":"404692","messageId":"20200828124617.60618-10-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 09/13] bisect--helper: retire `--next-all` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:13Z","receivedAt":"2020-08-28T12:48:55Z","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\nThe `--next-all` subcommand is no longer used from the git-bisect.sh\nshell script. Instead the function `bisect_next_all()` is called from\nthe C implementation of `bisect_next()`.\n\nMentored-by: Lars Schneider <larsxschneider@gmail.com>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\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 | 9 +--------\n 1 file changed, 1 insertion(+), 8 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 106e5b788e..0a32dbb68f 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -20,7 +20,6 @@ static GIT_PATH_FUNC(git_path_bisect_names, \"BISECT_NAMES\")\n static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n \n static const char * const git_bisect_helper_usage[] = {\n-\tN_(\"git bisect--helper --next-all\"),\n \tN_(\"git bisect--helper --write-terms <bad_term> <good_term>\"),\n \tN_(\"git bisect--helper --bisect-reset [<commit>]\"),\n \tN_(\"git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>\"),\n@@ -869,8 +868,7 @@ static int bisect_autostart(struct bisect_terms *terms)\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n-\t\tNEXT_ALL = 1,\n-\t\tWRITE_TERMS,\n+\t\tWRITE_TERMS = 1,\n \t\tCHECK_EXPECTED_REVS,\n \t\tBISECT_RESET,\n \t\tBISECT_WRITE,\n@@ -884,8 +882,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n-\t\tOPT_CMDMODE(0, \"next-all\", &cmdmode,\n-\t\t\t N_(\"perform 'git bisect next'\"), NEXT_ALL),\n \t\tOPT_CMDMODE(0, \"write-terms\", &cmdmode,\n \t\t\t N_(\"write the terms to .git/BISECT_TERMS\"), WRITE_TERMS),\n \t\tOPT_CMDMODE(0, \"check-expected-revs\", &cmdmode,\n@@ -922,9 +918,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(git_bisect_helper_usage, options);\n \n \tswitch (cmdmode) {\n-\tcase NEXT_ALL:\n-\t\tres = bisect_next_all(the_repository, prefix);\n-\t\tbreak;\n \tcase WRITE_TERMS:\n \t\tif (argc != 2)\n \t\t\treturn error(_(\"--write-terms requires two arguments\"));\n-- \n2.25.0\n\n"},{"id":"404693","messageId":"20200828124617.60618-11-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 10/13] bisect--helper: reimplement `bisect_state` & `bisect_head` shell functions in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:14Z","receivedAt":"2020-08-28T12:48:57Z","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_state()` shell functions in C and also add a\nsubcommand `--bisect-state` to `git-bisect--helper` to call them from\ngit-bisect.sh .\n\nUsing `--bisect-state` subcommand is a temporary measure to port shell\nfunction to C so as to use the existing test suite. As more functions\nare ported, this subcommand will be retired and will be called by some\nother methods.\n\n`bisect_head()` is only called from `bisect_state()`, thus it is not\nrequired to introduce another subcommand.\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 | 78 +++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh            | 55 +++-------------------------\n 2 files changed, 81 insertions(+), 52 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 0a32dbb68f..6d2847dc9c 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -31,6 +31,8 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-next\"),\n \tN_(\"git bisect--helper --bisect-auto-next\"),\n \tN_(\"git bisect--helper --bisect-autostart\"),\n+\tN_(\"git bisect--helper --bisect-state (bad|new) [<rev>]\"),\n+\tN_(\"git bisect--helper --bisect-state (good|old) [<rev>...]\"),\n \tNULL\n };\n \n@@ -865,6 +867,72 @@ static int bisect_autostart(struct bisect_terms *terms)\n \treturn res;\n }\n \n+static enum bisect_error bisect_state(struct bisect_terms *terms, const char **argv,\n+\t\t\t\t      int argc)\n+{\n+\tconst char *state;\n+\tint i, verify_expected = 1;\n+\tstruct object_id oid, expected;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct oid_array revs = OID_ARRAY_INIT;\n+\n+\tif (!argc)\n+\t\treturn error(_(\"Please call `--bisect-state` with at least one argument\"));\n+\n+\tstate = argv[0];\n+\tif (check_and_set_terms(terms, state) ||\n+\t    !one_of(state, terms->term_good, terms->term_bad, \"skip\", NULL))\n+\t\treturn BISECT_FAILED;\n+\n+\targv++;\n+\targc--;\n+\tif (argc > 1 && !strcmp(state, terms->term_bad))\n+\t\treturn error(_(\"'git bisect %s' can take only one argument.\"), terms->term_bad);\n+\n+\tif (argc == 0) {\n+\t\tenum get_oid_result res_head = get_oid(\"BISECT_HEAD\", &oid);\n+\t\tif (res_head == MISSING_OBJECT)\n+\t\t\tres_head = get_oid(\"HEAD\", &oid);\n+\t\tif (res_head) {\n+\t\t\terror(_(\"Bad bisect_head rev input\"));\n+\t\t\treturn BISECT_FAILED;\n+\t\t}\n+\t\toid_array_append(&revs, &oid);\n+\t}\n+\n+\t/*\n+\t * All input revs must be checked before executing bisect_write()\n+\t * to discard junk revs.\n+\t */\n+\n+\tfor (; argc; argc--, argv++) {\n+\t\tif (get_oid(*argv, &oid)){\n+\t\t\terror(_(\"Bad rev input: %s\"), *argv);\n+\t\t\treturn BISECT_FAILED;\n+\t\t}\n+\t\toid_array_append(&revs, &oid);\n+\t}\n+\n+\tif (strbuf_read_file(&buf, git_path_bisect_expected_rev(), 0) < the_hash_algo->hexsz ||\n+\t    get_oid_hex(buf.buf, &expected) < 0)\n+\t\tverify_expected = 0; /* Ignore invalid file contents */\n+\tstrbuf_release(&buf);\n+\n+\tfor (i = 0; i < revs.nr; i++) {\n+\t\tif (bisect_write(state, oid_to_hex(&revs.oid[i]), terms, 0))\n+\t\t\treturn BISECT_FAILED;\n+\n+\t\tif (verify_expected && !oideq(&revs.oid[i], &expected)) {\n+\t\t\tunlink_or_warn(git_path_bisect_ancestors_ok());\n+\t\t\tunlink_or_warn(git_path_bisect_expected_rev());\n+\t\t\tverify_expected = 0;\n+\t\t}\n+\t}\n+\n+\toid_array_clear(&revs);\n+\treturn bisect_auto_next(terms, NULL);\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -878,7 +946,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_START,\n \t\tBISECT_NEXT,\n \t\tBISECT_AUTO_NEXT,\n-\t\tBISECT_AUTOSTART\n+\t\tBISECT_AUTOSTART,\n+\t\tBISECT_STATE\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -904,6 +973,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\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_CMDMODE(0, \"bisect-state\", &cmdmode,\n+\t\t\t N_(\"mark the state of ref (or refs)\"), BISECT_STATE),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -974,6 +1045,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tset_terms(&terms, \"bad\", \"good\");\n \t\tres = bisect_autostart(&terms);\n \t\tbreak;\n+\tcase BISECT_STATE:\n+\t\tset_terms(&terms, \"bad\", \"good\");\n+\t\tget_terms(&terms);\n+\t\tres = bisect_state(&terms, argv, argc);\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 356264caf0..7a8f796251 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -39,16 +39,6 @@ _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n TERM_BAD=bad\n TERM_GOOD=good\n \n-bisect_head()\n-{\n-\tif git rev-parse --verify -q BISECT_HEAD > /dev/null\n-\tthen\n-\t\techo BISECT_HEAD\n-\telse\n-\t\techo HEAD\n-\tfi\n-}\n-\n bisect_skip() {\n \tall=''\n \tfor arg in \"$@\"\n@@ -61,43 +51,7 @@ bisect_skip() {\n \t\tesac\n \t\tall=\"$all $revs\"\n \tdone\n-\teval bisect_state 'skip' $all\n-}\n-\n-bisect_state() {\n-\tgit bisect--helper --bisect-autostart\n-\tstate=$1\n-\tgit bisect--helper --check-and-set-terms $state $TERM_GOOD $TERM_BAD || exit\n-\tget_terms\n-\tcase \"$#,$state\" in\n-\t0,*)\n-\t\tdie \"Please call 'bisect_state' with at least one argument.\" ;;\n-\t1,\"$TERM_BAD\"|1,\"$TERM_GOOD\"|1,skip)\n-\t\tbisected_head=$(bisect_head)\n-\t\trev=$(git rev-parse --verify \"$bisected_head\") ||\n-\t\t\tdie \"$(eval_gettext \"Bad rev input: \\$bisected_head\")\"\n-\t\tgit bisect--helper --bisect-write \"$state\" \"$rev\" \"$TERM_GOOD\" \"$TERM_BAD\" || exit\n-\t\tgit bisect--helper --check-expected-revs \"$rev\" ;;\n-\t2,\"$TERM_BAD\"|*,\"$TERM_GOOD\"|*,skip)\n-\t\tshift\n-\t\thash_list=''\n-\t\tfor rev in \"$@\"\n-\t\tdo\n-\t\t\tsha=$(git rev-parse --verify \"$rev^{commit}\") ||\n-\t\t\t\tdie \"$(eval_gettext \"Bad rev input: \\$rev\")\"\n-\t\t\thash_list=\"$hash_list $sha\"\n-\t\tdone\n-\t\tfor rev in $hash_list\n-\t\tdo\n-\t\t\tgit bisect--helper --bisect-write \"$state\" \"$rev\" \"$TERM_GOOD\" \"$TERM_BAD\" || exit\n-\t\tdone\n-\t\tgit bisect--helper --check-expected-revs $hash_list ;;\n-\t*,\"$TERM_BAD\")\n-\t\tdie \"$(eval_gettext \"'git bisect \\$TERM_BAD' can take only one argument.\")\" ;;\n-\t*)\n-\t\tusage ;;\n-\tesac\n-\tgit bisect--helper --bisect-auto-next\n+\teval git bisect--helper --bisect-state 'skip' $all\n }\n \n bisect_visualize() {\n@@ -187,8 +141,7 @@ exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n \t\t\tstate=\"$TERM_GOOD\"\n \t\tfi\n \n-\t\t# We have to use a subshell because \"bisect_state\" can exit.\n-\t\t( bisect_state $state >\"$GIT_DIR/BISECT_RUN\" )\n+\t\tgit bisect--helper --bisect-state $state >\"$GIT_DIR/BISECT_RUN\"\n \t\tres=$?\n \n \t\tcat \"$GIT_DIR/BISECT_RUN\"\n@@ -203,7 +156,7 @@ exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n \t\tif [ $res -ne 0 ]\n \t\tthen\n \t\t\teval_gettextln \"bisect run failed:\n-'bisect_state \\$state' exited with error code \\$res\" >&2\n+'git bisect--helper --bisect-state \\$state' exited with error code \\$res\" >&2\n \t\t\texit $res\n \t\tfi\n \n@@ -244,7 +197,7 @@ case \"$#\" in\n \tstart)\n \t\tgit bisect--helper --bisect-start \"$@\" ;;\n \tbad|good|new|old|\"$TERM_BAD\"|\"$TERM_GOOD\")\n-\t\tbisect_state \"$cmd\" \"$@\" ;;\n+\t\tgit bisect--helper --bisect-state \"$cmd\" \"$@\" ;;\n \tskip)\n \t\tbisect_skip \"$@\" ;;\n \tnext)\n-- \n2.25.0\n\n"},{"id":"404694","messageId":"20200828124617.60618-12-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 11/13] bisect--helper: retire `--check-expected-revs` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:15Z","receivedAt":"2020-08-28T12:49:11Z","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\nThe `--check-expected-revs` subcommand is no longer used from the\ngit-bisect.sh shell script. Functions `check_expected_revs` and\n`is_expected_revs` are also deleted.\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 | 30 ------------------------------\n 1 file changed, 30 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 6d2847dc9c..d795f2e5c7 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -187,30 +187,6 @@ static int write_terms(const char *bad, const char *good)\n \treturn res;\n }\n \n-static int is_expected_rev(const char *expected_hex)\n-{\n-\tstruct strbuf actual_hex = STRBUF_INIT;\n-\tint res = 0;\n-\tif (strbuf_read_file(&actual_hex, git_path_bisect_expected_rev(), 0) >= 40) {\n-\t\tstrbuf_trim(&actual_hex);\n-\t\tres = !strcmp(actual_hex.buf, expected_hex);\n-\t}\n-\tstrbuf_release(&actual_hex);\n-\treturn res;\n-}\n-\n-static void check_expected_revs(const char **revs, int rev_nr)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < rev_nr; i++) {\n-\t\tif (!is_expected_rev(revs[i])) {\n-\t\t\tunlink_or_warn(git_path_bisect_ancestors_ok());\n-\t\t\tunlink_or_warn(git_path_bisect_expected_rev());\n-\t\t}\n-\t}\n-}\n-\n static int bisect_reset(const char *commit)\n {\n \tstruct strbuf branch = STRBUF_INIT;\n@@ -937,7 +913,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n \t\tWRITE_TERMS = 1,\n-\t\tCHECK_EXPECTED_REVS,\n \t\tBISECT_RESET,\n \t\tBISECT_WRITE,\n \t\tCHECK_AND_SET_TERMS,\n@@ -953,8 +928,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_CMDMODE(0, \"write-terms\", &cmdmode,\n \t\t\t N_(\"write the terms to .git/BISECT_TERMS\"), WRITE_TERMS),\n-\t\tOPT_CMDMODE(0, \"check-expected-revs\", &cmdmode,\n-\t\t\t N_(\"check for expected revs\"), CHECK_EXPECTED_REVS),\n \t\tOPT_CMDMODE(0, \"bisect-reset\", &cmdmode,\n \t\t\t N_(\"reset the bisection state\"), BISECT_RESET),\n \t\tOPT_CMDMODE(0, \"bisect-write\", &cmdmode,\n@@ -993,9 +966,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tif (argc != 2)\n \t\t\treturn error(_(\"--write-terms requires two arguments\"));\n \t\treturn write_terms(argv[0], argv[1]);\n-\tcase CHECK_EXPECTED_REVS:\n-\t\tcheck_expected_revs(argv, argc);\n-\t\treturn 0;\n \tcase BISECT_RESET:\n \t\tif (argc > 1)\n \t\t\treturn error(_(\"--bisect-reset requires either no argument or a commit\"));\n-- \n2.25.0\n\n"},{"id":"404695","messageId":"20200828124617.60618-13-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 12/13] bisect--helper: retire `--write-terms` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:16Z","receivedAt":"2020-08-28T12:49:35Z","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\nThe `--write-terms` subcommand is no longer used from the\ngit-bisect.sh shell script. Instead the function `write_terms()`\nis called from the C implementation of `set_terms()` and\n`bisect_start()`.\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 | 10 +---------\n 1 file changed, 1 insertion(+), 9 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex d795f2e5c7..a6282d54a2 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -20,7 +20,6 @@ static GIT_PATH_FUNC(git_path_bisect_names, \"BISECT_NAMES\")\n static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n \n static const char * const git_bisect_helper_usage[] = {\n-\tN_(\"git bisect--helper --write-terms <bad_term> <good_term>\"),\n \tN_(\"git bisect--helper --bisect-reset [<commit>]\"),\n \tN_(\"git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>\"),\n \tN_(\"git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>\"),\n@@ -912,8 +911,7 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n-\t\tWRITE_TERMS = 1,\n-\t\tBISECT_RESET,\n+\t\tBISECT_RESET = 1,\n \t\tBISECT_WRITE,\n \t\tCHECK_AND_SET_TERMS,\n \t\tBISECT_NEXT_CHECK,\n@@ -926,8 +924,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n-\t\tOPT_CMDMODE(0, \"write-terms\", &cmdmode,\n-\t\t\t N_(\"write the terms to .git/BISECT_TERMS\"), WRITE_TERMS),\n \t\tOPT_CMDMODE(0, \"bisect-reset\", &cmdmode,\n \t\t\t N_(\"reset the bisection state\"), BISECT_RESET),\n \t\tOPT_CMDMODE(0, \"bisect-write\", &cmdmode,\n@@ -962,10 +958,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(git_bisect_helper_usage, options);\n \n \tswitch (cmdmode) {\n-\tcase WRITE_TERMS:\n-\t\tif (argc != 2)\n-\t\t\treturn error(_(\"--write-terms requires two arguments\"));\n-\t\treturn write_terms(argv[0], argv[1]);\n \tcase BISECT_RESET:\n \t\tif (argc > 1)\n \t\t\treturn error(_(\"--bisect-reset requires either no argument or a commit\"));\n-- \n2.25.0\n\n"},{"id":"404696","messageId":"20200828124617.60618-14-mirucam@gmail.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"[PATCH v6 13/13] bisect--helper: retire `--bisect-autostart` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-28T12:46:17Z","receivedAt":"2020-08-28T12:49:49Z","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\nThe `--bisect-autostart` subcommand is no longer used from the\ngit-bisect.sh shell script. Instead the function\n`bisect_autostart()` is directly called from the C implementation.\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 | 10 ----------\n 1 file changed, 10 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex a6282d54a2..be1291f036 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -29,7 +29,6 @@ static const char * const git_bisect_helper_usage[] = {\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 \tN_(\"git bisect--helper --bisect-state (bad|new) [<rev>]\"),\n \tN_(\"git bisect--helper --bisect-state (good|old) [<rev>...]\"),\n \tNULL\n@@ -919,7 +918,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_START,\n \t\tBISECT_NEXT,\n \t\tBISECT_AUTO_NEXT,\n-\t\tBISECT_AUTOSTART,\n \t\tBISECT_STATE\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n@@ -940,8 +938,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\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_CMDMODE(0, \"bisect-state\", &cmdmode,\n \t\t\t N_(\"mark the state of ref (or refs)\"), BISECT_STATE),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n@@ -1001,12 +997,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\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-\t\tset_terms(&terms, \"bad\", \"good\");\n-\t\tres = bisect_autostart(&terms);\n-\t\tbreak;\n \tcase BISECT_STATE:\n \t\tset_terms(&terms, \"bad\", \"good\");\n \t\tget_terms(&terms);\n-- \n2.25.0\n\n"},{"id":"404758","messageId":"xmqq8sdxi70a.fsf@gitster.c.googlers.com","threadId":"54150","inReplyTo":"20200828124617.60618-7-mirucam@gmail.com","subject":"Re: [PATCH v6 06/13] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-29T19:31:17Z","receivedAt":"2020-08-29T19:31:24Z","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> diff --git a/bisect.c b/bisect.c\n> index c6aba2b9f2..f0fca5c6f3 100644\n> --- a/bisect.c\n> +++ b/bisect.c\n> @@ -988,6 +988,12 @@ 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> + *\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>   * If no_checkout is non-zero, the bisection process does not\n>   * checkout the trial commit but instead simply updates BISECT_HEAD.\n>   */\n\nNot a problem introduced by this step, but the above description on\nno_checkout describes a parameter that no longer exists.  \n\nThe comments before a function is to guide the developers how to\ncall the function correctly, so it should have been removed, moved\nto where no_checkout is used in the function, or moved to where\nBISECT_HEAD ref gets created, as necessary, but by mistake be5fe200\n(cmd_bisect__helper: defer parsing no-checkout flag, 2020-08-07),\nforgot to do any of the three.\n\n\n> +static enum bisect_error bisect_next(struct bisect_terms *terms, const char *prefix)\n> +{\n> +\tint no_checkout;\n> +\tenum bisect_error res;\n> +\n> +\tbisect_autostart(terms);\n> +\tif (bisect_next_check(terms, terms->term_good))\n> +\t\treturn BISECT_FAILED;\n> +\n> +\tno_checkout = ref_exists(\"BISECT_HEAD\");\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\nThe no_checkout local variable is assigned but never used.  It is\nunderstandable if a variable that used to be used becomes unused\nwhen some part (i.e. the part that used to use the variable) of a\nfunction is factored out, but it is rather unusual how a brand new\nfunction has such an unused code and stay to be that way throughout\na topic.  Makes a reviewer suspect that there may be a code missing,\nthat has to use the variable to decide to do things differently, in\nthis function.  It seems to break -Werror builds.\n\nThanks.\n\n\n"},{"id":"404759","messageId":"xmqq1rjpi704.fsf@gitster.c.googlers.com","threadId":"54150","inReplyTo":"20200828124617.60618-1-mirucam@gmail.com","subject":"Re: [PATCH v6 00/13] Finish converting git bisect to C part 2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-29T19:31:23Z","receivedAt":"2020-08-29T19:31:34Z","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> These patch series emails were generated from:\n> https://gitlab.com/mirucam/git/commits/git-bisect-work-part2-v6ps.\n>\n> I would like to thank the reviewers for their help.\n>\n> General changes\n> ---------------\n>\n> * Rebase on master branch: e9b77c84a0 (Tenth batch, 2020-08-24)\n\nWas this done because the updated series newly depends on something\nthat was not available in f402ea68 (The fifth batch, 2020-06-25)[*]\nbut is available in e9b77c84a0 (Tenth batch, 2020-08-24), and/or is\nthere a change between the fifth and tenth batch that conflicts with\nthe old iteration?\n\n\tSide note: the previous iteration, which is queued in\n\t'seen', was applied on top of f402ea68.\n\nUpdating to a new base is not by itself wrong, but please also\ndescribe the reason why the topic was rebased, perhaps like\n\n * Rebased on e9b77c84a0 (Tenth batch, 2020-08-24), to build on the\n   strvec API (instead of argv_array) and adjust to the codebase\n   after the \"--first-parent\" feature was added.\n\nor something like that.\n\n> [6/13] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell\n>  functions in C\n>  \n> * Change `file_exists(git_path_bisect_head())` to a refs API call.\n\nVery much appreciated.\n\nThanks.\n"},{"id":"404796","messageId":"CAN7CjDBd=41PQ5qfqazdtx4uoRcfcc6cUWf5u0cNiooSo24ENg@mail.gmail.com","threadId":"54150","inReplyTo":"xmqq8sdxi70a.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v6 06/13] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2020-08-31T10:50:33Z","receivedAt":"2020-08-31T10:50:53Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"El sáb., 29 ago. 2020 a las 21:31, Junio C Hamano\n(<gitster@pobox.com>) escribió:\n>\n> Miriam Rubio <mirucam@gmail.com> writes:\n>\n> > diff --git a/bisect.c b/bisect.c\n> > index c6aba2b9f2..f0fca5c6f3 100644\n> > --- a/bisect.c\n> > +++ b/bisect.c\n> > @@ -988,6 +988,12 @@ 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> > + *\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> >   * If no_checkout is non-zero, the bisection process does not\n> >   * checkout the trial commit but instead simply updates BISECT_HEAD.\n> >   */\n>\n> Not a problem introduced by this step, but the above description on\n> no_checkout describes a parameter that no longer exists.\n>\n> The comments before a function is to guide the developers how to\n> call the function correctly, so it should have been removed, moved\n> to where no_checkout is used in the function, or moved to where\n> BISECT_HEAD ref gets created, as necessary, but by mistake be5fe200\n> (cmd_bisect__helper: defer parsing no-checkout flag, 2020-08-07),\n> forgot to do any of the three.\n>\n>\n> > +static enum bisect_error bisect_next(struct bisect_terms *terms, const char *prefix)\n> > +{\n> > +     int no_checkout;\n> > +     enum bisect_error res;\n> > +\n> > +     bisect_autostart(terms);\n> > +     if (bisect_next_check(terms, terms->term_good))\n> > +             return BISECT_FAILED;\n> > +\n> > +     no_checkout = ref_exists(\"BISECT_HEAD\");\n> > +\n> > +     /* Perform all bisection computation */\n> > +     res = bisect_next_all(the_repository, prefix);\n> > +\n> > +     if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {\n> > +             res = bisect_successful(terms);\n> > +             return res ? res : BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;\n> > +     } else if (res == BISECT_ONLY_SKIPPED_LEFT) {\n> > +             res = bisect_skipped_commits(terms);\n> > +             return res ? res : BISECT_ONLY_SKIPPED_LEFT;\n> > +     }\n> > +     return res;\n> > +}\n> > +\n>\n> The no_checkout local variable is assigned but never used.  It is\n> understandable if a variable that used to be used becomes unused\n> when some part (i.e. the part that used to use the variable) of a\n> function is factored out, but it is rather unusual how a brand new\n> function has such an unused code and stay to be that way throughout\n> a topic.  Makes a reviewer suspect that there may be a code missing,\n> that has to use the variable to decide to do things differently, in\n> this function.  It seems to break -Werror builds.\n\nOk, I will send a new version with this local variable removed.\n\nI don't know if it is something related to my compiler but my -Werror\nbuild does not break. Something similar happened with a previous patch\nseries version and a warning related with missing-prototypes.\nI always compile with : make -j 16 DEVELOPER=1 CFLAGS=\"-O0 -g3\" install;\nand I have also tested adding the -Werror flag and commits always\ncompile without problems.\nMaybe someone in the community knows what my problem is, because it\ncould avoid extra patch series versions and reviewers work.\n\nThank you.\n\n>\n> Thanks.\n>\n>\n"},{"id":"404817","messageId":"xmqq8sduhhdz.fsf@gitster.c.googlers.com","threadId":"54150","inReplyTo":"CAN7CjDBd=41PQ5qfqazdtx4uoRcfcc6cUWf5u0cNiooSo24ENg@mail.gmail.com","subject":"Re: [PATCH v6 06/13] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-31T17:09:12Z","receivedAt":"2020-08-31T17:09:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Miriam R.\" <mirucam@gmail.com> writes:\n\n> I don't know if it is something related to my compiler but my -Werror\n> build does not break.\n\nFWIW, here is a full compilation command line from \"make V=1\" for\nthe file:\n\ncc -o builtin/bisect--helper.o -c -MF builtin/.depend/bisect--helper.o.d -MQ builtin/bisect--helper.o -MMD -MP -Werror -Wall -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter  -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU=\"\\\"x86_64\\\"\" -DHAVE_ALLOCA_H  -DUSE_CURL_FOR_IMAP_SEND -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C=\"\\\"cache.h\\\"\" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C=\"\\\"git-compat-util.h\\\"\" -DSHA256_BLK  -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH=\"/proc/self/exe\"'  -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -DSHELL_PATH='\"/bin/sh\"' -DPAGER_ENV='\"LESS=FRX LV=-c\"'  builtin/bisect--helper.c\nbuiltin/bisect--helper.c: In function 'bisect_next':\nbuiltin/bisect--helper.c:570:6: error: variable 'no_checkout' set but not used [-Werror=unused-but-set-variable]\n  570 |  int no_checkout;\n      |      ^~~~~~~~~~~\ncc1: all warnings being treated as errors\n"}]}