{"thread":{"id":"53787","subject":"[PATCH v4 00/13] Finish converting git bisect to C part 2","startedAt":"2020-07-01T13:35:33Z","lastAt":"2020-07-08T21:39:01Z","messageCount":17,"participants":["Miriam Rubio","Chris Torek","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":13},"messages":[{"id":"400905","messageId":"20200701133504.18360-1-mirucam@gmail.com","threadId":"53787","inReplyTo":null,"subject":"[PATCH v4 00/13] Finish converting git bisect to C part 2","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:34:51Z","receivedAt":"2020-07-01T13:35:33Z","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-v4.\n\nI would like to thank Johannes Schindelin for reviewing this patch series\nand Christian Couder for his help.\n\nGeneral changes\n---------------\n\n* Rebase on master branch: f402ea6816 (The fifth batch, 2020-06-25)\n* Improve commit messages.\n\nSpecific changes\n----------------\n\n[3/12] bisect--helper: introduce new `write_in_file()` function\n\n* Add a wrapper for \"w\" mode.\n* Adapt calls to new wrapper.\n\n--\n\n[4/12] bisect--helper: reimplement `bisect_autostart` shell function in C\n\n* Add file_is_not_empty() function and call.\n* Change fprintf to fprintf_ln.\n* !isatty(STDIN_FILENO) returns 0 instead of 1.\n* Change bisect_autostart() return implementation.\n* Move bisect_autostart() previous header declaration to patch [6/12].\n* Change some error and user messages.\n\n--\n[5/12] bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'\n\n* New patch splitted from [6/12] patch.\n\n--\n\n[6/12] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell\nfunctions in C\n\n* Add struct add_bisect_ref_data.\n* Add wrapper append_to_file() and adapt calls.\n* Remove register_good_ref() and prepare_rev_argv() functions.\n* Add add_bisect_ref() function.\n* Reimplementation of prepare_revs() function.\n* Include process_skipped_commits() code in bisect_skipped_commits().\n* Use lookup_commit_reference_by_name() instead of lookup_commit_reference().\n\n--\n\n[7/12] bisect--helper: finish porting `bisect_start()` to C\n\n* Amend code comments.\n* Add is_bisect_success() function and call.\n* git-bisect.sh: merge with master changes.\n\n--\n\n[10/12] bisect--helper: reimplement `bisect_state` & `bisect_head` shell\nfunctions in C\n\n* Remove bisect_head() function.\n* Add mark_for_removal() function.\n* Reimplement bisect_state().\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 | 452 ++++++++++++++++++++++++++++++++-------\n git-bisect.sh            | 145 +------------\n 3 files changed, 387 insertions(+), 218 deletions(-)\n\n-- \n2.25.0\n\n"},{"id":"400906","messageId":"20200701133504.18360-2-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 01/13] bisect--helper: BUG() in cmd_*() on invalid subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:34:52Z","receivedAt":"2020-07-01T13:35:34Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"In cmd_bisect__helper() function, there would be a bug\nif an invalid or no subcommand was passed.return error()\nshould be replaced by BUG() in that case.\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 ec4996282e..c452d3f6b7 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -716,7 +716,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tres = bisect_start(&terms, no_checkout, argv, argc);\n \t\tbreak;\n \tdefault:\n-\t\treturn error(\"BUG: unknown subcommand '%d'\", cmdmode);\n+\t\tBUG(\"unknown subcommand %d\", (int)cmdmode);\n \t}\n \tfree_terms(&terms);\n \n-- \n2.25.0\n\n"},{"id":"400907","messageId":"20200701133504.18360-4-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 03/13] bisect--helper: introduce new `write_in_file()` function","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:34:54Z","receivedAt":"2020-07-01T13:35:36Z","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 writting 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 | 39 +++++++++++++++++++++++++++++++++------\n 1 file changed, 33 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 0466b07a43..b421056546 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -74,6 +74,38 @@ 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, \"a\") && !strcmp(mode, \"w\"))\n+\t\treturn error(_(\"wrong writing mode '%s'\"), 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) {\n+\t\tfclose(fp);\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 +136,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,12 +144,8 @@ 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 }\n \n-- \n2.25.0\n\n"},{"id":"400908","messageId":"20200701133504.18360-3-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 02/13] bisect--helper: use '-res' in 'cmd_bisect__helper' return","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:34:53Z","receivedAt":"2020-07-01T13:35:37Z","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>\nSigned-off-by: Miriam Rubio <mirucam@gmail.com>\nHelped-by: Junio C Hamano <gitster@pobox.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 c452d3f6b7..0466b07a43 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -727,5 +727,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":"400909","messageId":"20200701133504.18360-6-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 05/13] bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:34:56Z","receivedAt":"2020-07-01T13:35:40Z","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 d5e830410f..db00850065 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1064,6 +1064,8 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int\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":"400910","messageId":"20200701133504.18360-5-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 04/13] bisect--helper: reimplement `bisect_autostart` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:34:55Z","receivedAt":"2020-07-01T13:35:40Z","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 b421056546..36166840d1 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] [<bad> [<good>...]] [--] [<paths>...]\"),\n+\tN_(\"git bisect--helper --bisect-autostart\"),\n \tNULL\n };\n \n@@ -645,6 +646,38 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\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, 0, NULL, 0);\n+\n+\t\treturn res;\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -657,7 +690,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 no_checkout = 0, res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -681,6 +715,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-checkout\", &no_checkout,\n \t\t\t N_(\"update BISECT_HEAD instead of checking out the current commit\")),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n@@ -742,6 +778,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, no_checkout, 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\", (int)cmdmode);\n \t}\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 08a6ed57dd..d7a6990c29 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":"400911","messageId":"20200701133504.18360-7-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 06/13] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:34:57Z","receivedAt":"2020-07-01T13:35:42Z","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 | 181 ++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh            |  47 +---------\n 3 files changed, 189 insertions(+), 45 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex db00850065..0864e2a21b 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -980,6 +980,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 36166840d1..9a9705c9e3 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] [<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@@ -107,6 +117,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@@ -449,6 +471,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 = file_exists(git_path_bisect_head());\n+\n+\t/* Perform all bisection computation, display and checkout */\n+\tres = bisect_next_all(the_repository, prefix, no_checkout);\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, int no_checkout,\n \t\t\tconst char **argv, int argc)\n {\n@@ -691,7 +850,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 no_checkout = 0, res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -715,6 +876,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-checkout\", &no_checkout,\n@@ -778,6 +943,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, no_checkout, 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@@ -793,7 +970,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 d7a6990c29..809be25424 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 $(test -f \"$GIT_DIR/BISECT_HEAD\" && echo --no-checkout)\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":"400912","messageId":"20200701133504.18360-9-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 08/13] bisect--helper: retire `--bisect-clean-state` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:34:59Z","receivedAt":"2020-07-01T13:35:45Z","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 b52a925dc6..59385e2b9d 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_names, \"BISECT_NAMES\")\n static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --next-all [--no-checkout]\"),\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@@ -864,7 +863,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@@ -882,8 +880,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@@ -927,10 +923,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":"400913","messageId":"20200701133504.18360-10-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 09/13] bisect--helper: retire `--next-all` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:35:00Z","receivedAt":"2020-07-01T13:35:46Z","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 59385e2b9d..3ded6a0ea2 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -20,7 +20,6 @@ static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n static GIT_PATH_FUNC(git_path_bisect_names, \"BISECT_NAMES\")\n \n static const char * const git_bisect_helper_usage[] = {\n-\tN_(\"git bisect--helper --next-all [--no-checkout]\"),\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@@ -861,8 +860,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@@ -876,8 +874,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t} cmdmode = 0;\n \tint no_checkout = 0, 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@@ -916,9 +912,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, no_checkout);\n-\t\tbreak;\n \tcase WRITE_TERMS:\n \t\tif (argc != 2)\n \t\t\treturn error(_(\"--write-terms requires two arguments\"));\n-- \n2.25.0\n\n"},{"id":"400914","messageId":"20200701133504.18360-8-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 07/13] bisect--helper: finish porting `bisect_start()` to C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:34:58Z","receivedAt":"2020-07-01T13:35:48Z","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 9a9705c9e3..b52a925dc6 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@@ -608,11 +621,12 @@ 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, int no_checkout,\n+static enum bisect_error bisect_start(struct bisect_terms *terms, int no_checkout,\n \t\t\tconst char **argv, int argc)\n {\n \tint i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;\n-\tint flags, pathspec_pos, 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@@ -670,9 +684,12 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\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@@ -750,14 +767,7 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\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@@ -771,7 +781,7 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\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@@ -783,25 +793,37 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,\n \tfor (i = 0; i < states.nr; i++)\n \t\tif (bisect_write(states.items[i].string,\n \t\t\t\t revs.items[i].string, terms, 1)) {\n-\t\t\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 809be25424..cfb05c0511 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":"400915","messageId":"20200701133504.18360-11-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 10/13] bisect--helper: reimplement `bisect_state` & `bisect_head` shell functions in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:35:01Z","receivedAt":"2020-07-01T13:35:48Z","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 | 87 +++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh            | 55 ++-----------------------\n 2 files changed, 90 insertions(+), 52 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 3ded6a0ea2..ef8121afc7 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@@ -856,6 +858,81 @@ static int bisect_autostart(struct bisect_terms *terms)\n \n \t\treturn res;\n }\n+ int mark_for_removal(const char *refname, const struct object_id *oid,\n+\t\t\t    int flag, void *cb_data)\n+{\n+\tstruct string_list *refs = cb_data;\n+\tchar *ref = xstrfmt(\"refs/bisect%s\", refname);\n+\tstring_list_append(refs, ref);\n+\treturn 0;\n+}\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@@ -870,7 +947,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 no_checkout = 0, res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -896,6 +974,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-checkout\", &no_checkout,\n \t\t\t N_(\"update BISECT_HEAD instead of checking out the current commit\")),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n@@ -968,6 +1048,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\", (int)cmdmode);\n \t}\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex cfb05c0511..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 test -f \"$GIT_DIR/BISECT_HEAD\"\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":"400916","messageId":"20200701133504.18360-12-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 11/13] bisect--helper: retire `--check-expected-revs` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:35:02Z","receivedAt":"2020-07-01T13:35:50Z","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 ef8121afc7..874f8c38c4 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -185,30 +185,6 @@ static int write_terms(const char *bad, const char *good)\n \treturn (res < 0) ? -1 : 0;\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@@ -938,7 +914,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@@ -954,8 +929,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@@ -996,9 +969,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":"400917","messageId":"20200701133504.18360-13-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 12/13] bisect--helper: retire `--write-terms` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:35:03Z","receivedAt":"2020-07-01T13:35:52Z","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 874f8c38c4..ca9d27b005 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -20,7 +20,6 @@ static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n static GIT_PATH_FUNC(git_path_bisect_names, \"BISECT_NAMES\")\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@@ -913,8 +912,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@@ -927,8 +925,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t} cmdmode = 0;\n \tint no_checkout = 0, 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@@ -965,10 +961,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":"400918","messageId":"20200701133504.18360-14-mirucam@gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-1-mirucam@gmail.com","subject":"[PATCH v4 13/13] bisect--helper: retire `--bisect-autostart` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-07-01T13:35:04Z","receivedAt":"2020-07-01T13:35: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-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 ca9d27b005..7a9fa4de78 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] [<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@@ -920,7 +919,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 no_checkout = 0, res = 0, nolog = 0;\n@@ -941,8 +939,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-checkout\", &no_checkout,\n@@ -1004,12 +1000,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":"400921","messageId":"CAPx1GvfFCKqTN2zodrO767CQQ2t2m-s9SRwR+s3cs0oAWyJ8Fg@mail.gmail.com","threadId":"53787","inReplyTo":"20200701133504.18360-4-mirucam@gmail.com","subject":"Re: [PATCH v4 03/13] bisect--helper: introduce new `write_in_file()` function","fromName":"Chris Torek","fromEmail":"chris.torek@gmail.com","sentAt":"2020-07-01T14:56:32Z","receivedAt":"2020-07-01T14:56:47Z","isPatch":true,"sender":{"key":"chris.torek@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16826774?v=4"},"body":"On Wed, Jul 1, 2020 at 6:37 AM Miriam Rubio <mirucam@gmail.com> wrote:\n> Let's refactor code adding a new `write_in_file()` function\n> that opens a file for writing a message and closes it and a\n> wrapper for writting mode.\n\nNit: typo, s/writting/writing/\n\n[snippage]\n\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index 0466b07a43..b421056546 100644\n> --- a/builtin/bisect--helper.c\n> +++ b/builtin/bisect--helper.c\n> @@ -74,6 +74,38 @@ static int one_of(const char *term, ...)\n>         return res;\n>  }\n>\n> +static int write_in_file(const char *path, const char *mode, const char *format, va_list args)\n> +{\n> +       FILE *fp = NULL;\n> +       int res = 0;\n> +\n> +       if (!strcmp(mode, \"a\") && !strcmp(mode, \"w\"))\n> +               return error(_(\"wrong writing mode '%s'\"), mode);\n\nShould this maybe just be BUG()?\n\n> +       fp = fopen(path, mode);\n> +       if (!fp)\n> +               return error_errno(_(\"cannot open file '%s' in mode '%s'\"), path, mode);\n> +       res = vfprintf(fp, format, args);\n> +\n> +       if (!res) {\n\nThis isn't quite right - vfprintf() returns a negative value on\nerror, or the number of characters printed on success.  Zero is\ntechnically OK (if the format and arguments ended up empty).\n\n[rest snipped]\n\nChris\n"},{"id":"401194","messageId":"xmqqwo3d23a4.fsf@gitster.c.googlers.com","threadId":"53787","inReplyTo":"20200701133504.18360-4-mirucam@gmail.com","subject":"Re: [PATCH v4 03/13] bisect--helper: introduce new `write_in_file()` function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-08T19:52:03Z","receivedAt":"2020-07-08T19:52:13Z","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> +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, \"a\") && !strcmp(mode, \"w\"))\n> +\t\treturn error(_(\"wrong writing mode '%s'\"), mode);\n\nHow can a string \"mode\" be equal to \"a\" and \"w\" at the same time?\n\nThe only caller you have at this point passes \"w\" and nothing else.\nIt may be easier to follow the evolution of the code in the series \nif you left this as\n\n\tif (strcmp(mode, \"w\"))\n\t\tBUG(\"write-in-file does not support '%s' mode\", mode);\n\nat this step, and turned it into\n\n\tif (strcmp(mode, \"w\") && strcmp(mode, \"a\"))\n\t\tBUG(\"write-in-file does not support '%s' mode\", mode);\n\nin the step where a caller starts to call it for appending.\n\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) {\n\nShouldn't this check for negative return?  It is not an error to\npass format that ends up writing nothing, but any error should be\nreported by returning a negative value from vfprintf(), I would\nthink.\n\n    (a tip for a student/mentee)\n\n    Any time you call a system library you are not familiar with,\n    make it a habit to always consult its manual page for return\n    values and its error reporting convention.\n\n>  \tif (!strcmp(bad, good))\n> @@ -113,12 +144,8 @@ 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\nShouldn't this now be just\n\n\treturn res;\n\nas all the error codepaths that can reach here are returning error()\nor error_errno() after this patch?\n\n>  }\n"},{"id":"401201","messageId":"xmqq7dvd1yc2.fsf@gitster.c.googlers.com","threadId":"53787","inReplyTo":"20200701133504.18360-11-mirucam@gmail.com","subject":"Re: [PATCH v4 10/13] bisect--helper: reimplement `bisect_state` & `bisect_head` shell functions in C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-08T21:38:53Z","receivedAt":"2020-07-08T21:39:01Z","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> @@ -856,6 +858,81 @@ static int bisect_autostart(struct bisect_terms *terms)\n>  \n>  \t\treturn res;\n>  }\n> + int mark_for_removal(const char *refname, const struct object_id *oid,\n> +\t\t\t    int flag, void *cb_data)\n> +{\n> +\tstruct string_list *refs = cb_data;\n> +\tchar *ref = xstrfmt(\"refs/bisect%s\", refname);\n> +\tstring_list_append(refs, ref);\n> +\treturn 0;\n> +}\n\nFunny indentation at the beginning, and worse yet, triggers an error\nwith -Werror=missing-prototypes.\n\nWhat is the relationship between this one and the similarly-named on\nin bisect.c?\n\n\n\n"}]}