{"thread":{"id":"54279","subject":"[PATCH v8 0/6] Finish converting git bisect to C part 2 subset 1","startedAt":"2020-09-23T07:28:03Z","lastAt":"2020-09-23T18:49:11Z","messageCount":10,"participants":["Miriam Rubio","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":8,"patchTotal":6},"messages":[{"id":"406170","messageId":"20200923072740.20772-1-mirucam@gmail.com","threadId":"54279","inReplyTo":null,"subject":"[PATCH v8 0/6] Finish converting git bisect to C part 2 subset 1","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-23T07:27:34Z","receivedAt":"2020-09-23T07:28:03Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"These patches correspond to a second part of patch series \nof Outreachy project \"Finish converting `git bisect` from shell to C\" \nstarted by Pranit Bauva and Tanushree Tumane\n(https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and\ncontinued by me.\n\nThis patch series version is composed by the first six patches of the\nprevious version due to a bug discovered during the test stage, that \naffects the rest of the patches and that needs to be fixed before \nsending them.\n\nThese patch series emails were generated from:\nhttps://gitlab.com/mirucam/git/commits/git-bisect-work-part2-v8-subset1.\n\nI would like to thank Johannes Schindelin for reviewing this patch series\nand to Christian Couder for his advice and help.\n\nGeneral changes\n---------------\n\n* Rebased on 385c171a01, (Fifteenth batch, 2020-09-18) to update command\nhelp messages.\n\n\nSpecific changes\n----------------\n\n\n[4/6] bisect--helper: reimplement `bisect_autostart` shell function in C\n\n* Amend commit message.\n* Change to `return -1` when we cannot autostart.\n* Add `|| exit` to bisect-autostart call in shell script.\n\n---\n\n\n[6/6] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell\n functions in C\n \n* Amend commit message.\n* Amend new modes order in cmdmode enum.\n* Add `|| exit` to bisect-auto-next call in shell script.\n\n \n---\n\nMiriam Rubio (4):\n bisect--helper: BUG() in cmd_*() on invalid subcommand\n bisect--helper: use '-res' in 'cmd_bisect__helper' return\n bisect--helper: introduce new `write_in_file()` function\n bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'\n\nPranit Bauva (2):\n bisect--helper: reimplement `bisect_autostart` shell function in C\n bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell\n functions in C\n\n bisect.c | 13 +-\n builtin/bisect--helper.c | 269 +++++++++++++++++++++++++++++++++++++--\n git-bisect.sh | 70 +---------\n 3 files changed, 274 insertions(+), 78 deletions(-)\n\n-- \n2.25.0\n\n"},{"id":"406171","messageId":"20200923072740.20772-2-mirucam@gmail.com","threadId":"54279","inReplyTo":"20200923072740.20772-1-mirucam@gmail.com","subject":"[PATCH v8 1/6] bisect--helper: BUG() in cmd_*() on invalid subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-23T07:27:35Z","receivedAt":"2020-09-23T07:28:06Z","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 \nMentored-by: Johannes Schindelin \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 7dcc1b5188..23a9f00035 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -720,7 +720,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tres = bisect_start(&terms, argv, argc);\n \t\tbreak;\n \tdefault:\n-\t\treturn error(\"BUG: unknown subcommand '%d'\", cmdmode);\n+\t\tBUG(\"unknown subcommand %d\", cmdmode);\n \t}\n \tfree_terms(&terms);\n \n-- \n2.25.0\n\n"},{"id":"406172","messageId":"20200923072740.20772-3-mirucam@gmail.com","threadId":"54279","inReplyTo":"20200923072740.20772-1-mirucam@gmail.com","subject":"[PATCH v8 2/6] bisect--helper: use '-res' in 'cmd_bisect__helper' return","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-23T07:27:36Z","receivedAt":"2020-09-23T07:28:07Z","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 \nHelped-by: Junio C Hamano \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 23a9f00035..8037888ae2 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -731,5 +731,5 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \tif (res == BISECT_INTERNAL_SUCCESS_MERGE_BASE)\n \t\tres = BISECT_OK;\n \n-\treturn abs(res);\n+\treturn -res;\n }\n-- \n2.25.0\n\n"},{"id":"406173","messageId":"20200923072740.20772-5-mirucam@gmail.com","threadId":"54279","inReplyTo":"20200923072740.20772-1-mirucam@gmail.com","subject":"[PATCH v8 4/6] bisect--helper: reimplement `bisect_autostart` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-23T07:27:38Z","receivedAt":"2020-09-23T07:28:10Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nReimplement the `bisect_autostart()` shell function in C and add the\nC implementation from `bisect_next()` which was previously left\nuncovered.\n\nAdd `--bisect-autostart` subcommand to be called from git-bisect.sh.\nUsing `--bisect-autostart` subcommand is a temporary measure to port\nthe shell function to C so as to use the existing test suite. As more\nfunctions are ported, this subcommand will be retired and\nbisect_autostart() will be called directly by `bisect_state()`.\n\nChange behavior of shell script that returned success when user aborted\nthe bisection.\n\nMentored-by: Lars Schneider \nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 44 +++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh | 25 ++---------------------\n 2 files changed, 45 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 46d2abb36f..ab27a8dff6 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -29,6 +29,7 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n \tN_(\"git bisect--helper --bisect-start [--term-{new,bad}= --term-{old,good}=]\"\n \t\t\t\t\t \" [--no-checkout] [--first-parent] [ [...]] [--] [...]\"),\n+\tN_(\"git bisect--helper --bisect-autostart\"),\n \tNULL\n };\n \n@@ -653,6 +654,38 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n \treturn res;\n }\n \n+static inline int file_is_not_empty(const char *path)\n+{\n+\treturn !is_empty_or_missing_file(path);\n+}\n+\n+static int bisect_autostart(struct bisect_terms *terms)\n+{\n+\tint res;\n+\tconst char *yesno;\n+\n+\tif (file_is_not_empty(git_path_bisect_start()))\n+\t\treturn 0;\n+\n+\tfprintf_ln(stderr, _(\"You need to start by \\\"git bisect \"\n+\t\t\t \"start\\\"\\n\"));\n+\n+\tif (!isatty(STDIN_FILENO))\n+\t\treturn -1;\n+\n+\t/*\n+\t * TRANSLATORS: Make sure to include [Y] and [n] in your\n+\t * translation. The program will only accept English input\n+\t * at this point.\n+\t */\n+\tyesno = git_prompt(_(\"Do you want me to do it for you \"\n+\t\t\t \"[Y/n]? \"), PROMPT_ECHO);\n+\tres = tolower(*yesno) == 'n' ?\n+\t\t-1 : bisect_start(terms, empty_strvec, 0);\n+\n+\treturn res;\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -665,7 +698,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tCHECK_AND_SET_TERMS,\n \t\tBISECT_NEXT_CHECK,\n \t\tBISECT_TERMS,\n-\t\tBISECT_START\n+\t\tBISECT_START,\n+\t\tBISECT_AUTOSTART,\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -689,6 +723,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"print out the bisect terms\"), BISECT_TERMS),\n \t\tOPT_CMDMODE(0, \"bisect-start\", &cmdmode,\n \t\t\t N_(\"start the bisect session\"), BISECT_START),\n+\t\tOPT_CMDMODE(0, \"bisect-autostart\", &cmdmode,\n+\t\t\t N_(\"start the bisection if it has not yet been started\"), BISECT_AUTOSTART),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -748,6 +784,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tset_terms(&terms, \"bad\", \"good\");\n \t\tres = bisect_start(&terms, argv, argc);\n \t\tbreak;\n+\tcase BISECT_AUTOSTART:\n+\t\tif (argc)\n+\t\t\treturn error(_(\"--bisect-autostart does not accept arguments\"));\n+\t\tset_terms(&terms, \"bad\", \"good\");\n+\t\tres = bisect_autostart(&terms);\n+\t\tbreak;\n \tdefault:\n \t\tBUG(\"unknown subcommand %d\", cmdmode);\n \t}\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 2f60fefcfa..4e9b2d36cb 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -49,27 +49,6 @@ bisect_head()\n \tfi\n }\n \n-bisect_autostart() {\n-\ttest -s \"$GIT_DIR/BISECT_START\" || {\n-\t\tgettextln \"You need to start by \\\"git bisect start\\\"\" >&2\n-\t\tif test -t 0\n-\t\tthen\n-\t\t\t# TRANSLATORS: Make sure to include [Y] and [n] in your\n-\t\t\t# translation. The program will only accept English input\n-\t\t\t# at this point.\n-\t\t\tgettext \"Do you want me to do it for you [Y/n]? \" >&2\n-\t\t\tread yesno\n-\t\t\tcase \"$yesno\" in\n-\t\t\t[Nn]*)\n-\t\t\t\texit ;;\n-\t\t\tesac\n-\t\t\tbisect_start\n-\t\telse\n-\t\t\texit 1\n-\t\tfi\n-\t}\n-}\n-\n bisect_start() {\n \tgit bisect--helper --bisect-start $@ || exit\n \n@@ -108,7 +87,7 @@ bisect_skip() {\n }\n \n bisect_state() {\n-\tbisect_autostart\n+\tgit bisect--helper --bisect-autostart || exit\n \tstate=$1\n \tgit bisect--helper --check-and-set-terms $state $TERM_GOOD $TERM_BAD || exit\n \tget_terms\n@@ -149,7 +128,7 @@ bisect_auto_next() {\n \n bisect_next() {\n \tcase \"$#\" in 0) ;; *) usage ;; esac\n-\tbisect_autostart\n+\tgit bisect--helper --bisect-autostart\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":"406174","messageId":"20200923072740.20772-4-mirucam@gmail.com","threadId":"54279","inReplyTo":"20200923072740.20772-1-mirucam@gmail.com","subject":"[PATCH v8 3/6] bisect--helper: introduce new `write_in_file()` function","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-23T07:27:37Z","receivedAt":"2020-09-23T07:28:11Z","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 \nMentored-by: Johannes Schindelin \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 43 +++++++++++++++++++++++++++++++++-------\n 1 file changed, 36 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 8037888ae2..46d2abb36f 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -74,6 +74,40 @@ static int one_of(const char *term, ...)\n \treturn res;\n }\n \n+static int write_in_file(const char *path, const char *mode, const char *format, va_list args)\n+{\n+\tFILE *fp = NULL;\n+\tint res = 0;\n+\n+\tif (strcmp(mode, \"w\"))\n+\t\tBUG(\"write-in-file does not support '%s' mode\", mode);\n+\tfp = fopen(path, mode);\n+\tif (!fp)\n+\t\treturn error_errno(_(\"cannot open file '%s' in mode '%s'\"), path, mode);\n+\tres = vfprintf(fp, format, args);\n+\n+\tif (res < 0) {\n+\t\tint saved_errno = errno;\n+\t\tfclose(fp);\n+\t\terrno = saved_errno;\n+\t\treturn error_errno(_(\"could not write to file '%s'\"), path);\n+\t}\n+\n+\treturn fclose(fp);\n+}\n+\n+static int write_to_file(const char *path, const char *format, ...)\n+{\n+\tint res;\n+\tva_list args;\n+\n+\tva_start(args, format);\n+\tres = write_in_file(path, \"w\", format, args);\n+\tva_end(args);\n+\n+\treturn res;\n+}\n+\n static int check_term_format(const char *term, const char *orig_term)\n {\n \tint res;\n@@ -104,7 +138,6 @@ static int check_term_format(const char *term, const char *orig_term)\n \n static int write_terms(const char *bad, const char *good)\n {\n-\tFILE *fp = NULL;\n \tint res;\n \n \tif (!strcmp(bad, good))\n@@ -113,13 +146,9 @@ static int write_terms(const char *bad, const char *good)\n \tif (check_term_format(bad, \"bad\") || check_term_format(good, \"good\"))\n \t\treturn -1;\n \n-\tfp = fopen(git_path_bisect_terms(), \"w\");\n-\tif (!fp)\n-\t\treturn error_errno(_(\"could not open the file BISECT_TERMS\"));\n+\tres = write_to_file(git_path_bisect_terms(), \"%s\\n%s\\n\", bad, good);\n \n-\tres = fprintf(fp, \"%s\\n%s\\n\", bad, good);\n-\tres |= fclose(fp);\n-\treturn (res < 0) ? -1 : 0;\n+\treturn res;\n }\n \n static int is_expected_rev(const char *expected_hex)\n-- \n2.25.0\n\n"},{"id":"406175","messageId":"20200923072740.20772-6-mirucam@gmail.com","threadId":"54279","inReplyTo":"20200923072740.20772-1-mirucam@gmail.com","subject":"[PATCH v8 5/6] bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-23T07:27:39Z","receivedAt":"2020-09-23T07:28:12Z","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 \nSigned-off-by: Miriam Rubio \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":"406176","messageId":"20200923072740.20772-7-mirucam@gmail.com","threadId":"54279","inReplyTo":"20200923072740.20772-1-mirucam@gmail.com","subject":"[PATCH v8 6/6] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-09-23T07:27:40Z","receivedAt":"2020-09-23T07:28:15Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nReimplement the `bisect_next()` and the `bisect_auto_next()` shell functions\nin C and add the subcommands to `git bisect--helper` to call them from\ngit-bisect.sh .\n\nbisect_auto_next() function returns an enum bisect_error type as whole\n`git bisect` can exit with an error code when bisect_next() does.\n\nReturn an error when `bisect_next()` fails, that fix a bug on shell script\nversion.\n\nUsing `--bisect-next` and `--bisect-auto-next` subcommands is a\ntemporary measure to port shell function to C so as to use the existing\ntest suite. As more functions are ported, `--bisect-auto-next`\nsubcommand will be retired and will be called by some other methods.\n\nMentored-by: Lars Schneider \nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n bisect.c | 11 ++-\n builtin/bisect--helper.c | 180 ++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh | 47 +---------\n 3 files changed, 191 insertions(+), 47 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex c6aba2b9f2..f5b1368128 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -988,8 +988,11 @@ void read_bisect_terms(const char **read_bad, const char **read_good)\n * the bisection process finished successfully.\n * In this case the calling function or command should not turn a\n * BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND return code into an error or a non zero exit code.\n- * If no_checkout is non-zero, the bisection process does not\n- * checkout the trial commit but instead simply updates BISECT_HEAD.\n+ *\n+ * Checking BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND\n+ * in bisect_helper::bisect_next() and only transforming it to 0 at\n+ * the end of bisect_helper::cmd_bisect__helper() helps bypassing\n+ * all the code related to finding a commit to test.\n */\n enum bisect_error bisect_next_all(struct repository *r, const char *prefix)\n {\n@@ -999,6 +1002,10 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)\n \tenum bisect_error res = BISECT_OK;\n \tstruct object_id *bisect_rev;\n \tchar *steps_msg;\n+\t/*\n+\t * If no_checkout is non-zero, the bisection process does not\n+\t * checkout the trial commit but instead simply updates BISECT_HEAD.\n+\t */\n \tint no_checkout = ref_exists(\"BISECT_HEAD\");\n \tunsigned bisect_flags = 0;\n \ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex ab27a8dff6..3a5bf99257 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -8,6 +8,7 @@\n #include \"run-command.h\"\n #include \"prompt.h\"\n #include \"quote.h\"\n+#include \"revision.h\"\n \n static GIT_PATH_FUNC(git_path_bisect_terms, \"BISECT_TERMS\")\n static GIT_PATH_FUNC(git_path_bisect_expected_rev, \"BISECT_EXPECTED_REV\")\n@@ -29,10 +30,17 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n \tN_(\"git bisect--helper --bisect-start [--term-{new,bad}= --term-{old,good}=]\"\n \t\t\t\t\t \" [--no-checkout] [--first-parent] [ [...]] [--] [...]\"),\n+\tN_(\"git bisect--helper --bisect-next\"),\n+\tN_(\"git bisect--helper --bisect-auto-next\"),\n \tN_(\"git bisect--helper --bisect-autostart\"),\n \tNULL\n };\n \n+struct add_bisect_ref_data {\n+\tstruct rev_info *revs;\n+\tunsigned int object_flags;\n+};\n+\n struct bisect_terms {\n \tchar *term_good;\n \tchar *term_bad;\n@@ -56,6 +64,8 @@ static void set_terms(struct bisect_terms *terms, const char *bad,\n static const char vocab_bad[] = \"bad|new\";\n static const char vocab_good[] = \"good|old\";\n \n+static int bisect_autostart(struct bisect_terms *terms);\n+\n /*\n * Check whether the string `term` belongs to the set of strings\n * included in the variable arguments.\n@@ -80,7 +90,7 @@ static int write_in_file(const char *path, const char *mode, const char *format,\n \tFILE *fp = NULL;\n \tint res = 0;\n \n-\tif (strcmp(mode, \"w\"))\n+\tif (strcmp(mode, \"w\") && strcmp(mode, \"a\"))\n \t\tBUG(\"write-in-file does not support '%s' mode\", mode);\n \tfp = fopen(path, mode);\n \tif (!fp)\n@@ -109,6 +119,18 @@ static int write_to_file(const char *path, const char *format, ...)\n \treturn res;\n }\n \n+static int append_to_file(const char *path, const char *format, ...)\n+{\n+\tint res;\n+\tva_list args;\n+\n+\tva_start(args, format);\n+\tres = write_in_file(path, \"a\", format, args);\n+\tva_end(args);\n+\n+\treturn res;\n+}\n+\n static int check_term_format(const char *term, const char *orig_term)\n {\n \tint res;\n@@ -451,6 +473,142 @@ static int bisect_append_log_quoted(const char **argv)\n \treturn res;\n }\n \n+static int add_bisect_ref(const char *refname, const struct object_id *oid,\n+\t\t\t int flags, void *cb)\n+{\n+\tstruct add_bisect_ref_data *data = cb;\n+\n+\tadd_pending_oid(data->revs, refname, oid, data->object_flags);\n+\n+\treturn 0;\n+}\n+\n+static int prepare_revs(struct bisect_terms *terms, struct rev_info *revs)\n+{\n+\tint res = 0;\n+\tstruct add_bisect_ref_data cb = { revs };\n+\tchar *good = xstrfmt(\"%s-*\", terms->term_good);\n+\n+\t/*\n+\t * We cannot use terms->term_bad directly in\n+\t * for_each_glob_ref_in() and we have to append a '*' to it,\n+\t * otherwise for_each_glob_ref_in() will append '/' and '*'.\n+\t */\n+\tchar *bad = xstrfmt(\"%s*\", terms->term_bad);\n+\n+\t/*\n+\t * It is important to reset the flags used by revision walks\n+\t * as the previous call to bisect_next_all() in turn\n+\t * sets up a revision walk.\n+\t */\n+\treset_revision_walk();\n+\tinit_revisions(revs, NULL);\n+\tsetup_revisions(0, NULL, revs, NULL);\n+\tfor_each_glob_ref_in(add_bisect_ref, bad, \"refs/bisect/\", &cb);\n+\tcb.object_flags = UNINTERESTING;\n+\tfor_each_glob_ref_in(add_bisect_ref, good, \"refs/bisect/\", &cb);\n+\tif (prepare_revision_walk(revs))\n+\t\tres = error(_(\"revision walk setup failed\\n\"));\n+\n+\tfree(good);\n+\tfree(bad);\n+\treturn res;\n+}\n+\n+static int bisect_skipped_commits(struct bisect_terms *terms)\n+{\n+\tint res;\n+\tFILE *fp = NULL;\n+\tstruct rev_info revs;\n+\tstruct commit *commit;\n+\tstruct pretty_print_context pp = {0};\n+\tstruct strbuf commit_name = STRBUF_INIT;\n+\n+\tres = prepare_revs(terms, &revs);\n+\tif (res)\n+\t\treturn res;\n+\n+\tfp = fopen(git_path_bisect_log(), \"a\");\n+\tif (!fp)\n+\t\treturn error_errno(_(\"could not open '%s' for appending\"),\n+\t\t\t\t git_path_bisect_log());\n+\n+\tif (fprintf(fp, \"# only skipped commits left to test\\n\") < 0)\n+\t\treturn error_errno(_(\"failed to write to '%s'\"), git_path_bisect_log());\n+\n+\twhile ((commit = get_revision(&revs)) != NULL) {\n+\t\tstrbuf_reset(&commit_name);\n+\t\tformat_commit_message(commit, \"%s\",\n+\t\t\t\t &commit_name, &pp);\n+\t\tfprintf(fp, \"# possible first %s commit: [%s] %s\\n\",\n+\t\t\tterms->term_bad, oid_to_hex(&commit->object.oid),\n+\t\t\tcommit_name.buf);\n+\t}\n+\n+\t/*\n+\t * Reset the flags used by revision walks in case\n+\t * there is another revision walk after this one.\n+\t */\n+\treset_revision_walk();\n+\n+\tstrbuf_release(&commit_name);\n+\tfclose(fp);\n+\treturn 0;\n+}\n+\n+static int bisect_successful(struct bisect_terms *terms)\n+{\n+\tstruct object_id oid;\n+\tstruct commit *commit;\n+\tstruct pretty_print_context pp = {0};\n+\tstruct strbuf commit_name = STRBUF_INIT;\n+\tchar *bad_ref = xstrfmt(\"refs/bisect/%s\",terms->term_bad);\n+\tint res;\n+\n+\tread_ref(bad_ref, &oid);\n+\tcommit = lookup_commit_reference_by_name(bad_ref);\n+\tformat_commit_message(commit, \"%s\", &commit_name, &pp);\n+\n+\tres = append_to_file(git_path_bisect_log(), \"# first %s commit: [%s] %s\\n\",\n+\t\t\t terms->term_bad, oid_to_hex(&commit->object.oid),\n+\t\t\t commit_name.buf);\n+\n+\tstrbuf_release(&commit_name);\n+\tfree(bad_ref);\n+\treturn res;\n+}\n+\n+static enum bisect_error bisect_next(struct bisect_terms *terms, const char *prefix)\n+{\n+\tenum bisect_error res;\n+\n+\tif (bisect_autostart(terms))\n+\t\treturn BISECT_FAILED;\n+\n+\tif (bisect_next_check(terms, terms->term_good))\n+\t\treturn BISECT_FAILED;\n+\n+\t/* Perform all bisection computation */\n+\tres = bisect_next_all(the_repository, prefix);\n+\n+\tif (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {\n+\t\tres = bisect_successful(terms);\n+\t\treturn res ? res : BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;\n+\t} else if (res == BISECT_ONLY_SKIPPED_LEFT) {\n+\t\tres = bisect_skipped_commits(terms);\n+\t\treturn res ? res : BISECT_ONLY_SKIPPED_LEFT;\n+\t}\n+\treturn res;\n+}\n+\n+static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char *prefix)\n+{\n+\tif (bisect_next_check(terms, NULL))\n+\t\treturn BISECT_OK;\n+\n+\treturn bisect_next(terms, prefix);\n+}\n+\n static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n {\n \tint no_checkout = 0;\n@@ -700,6 +858,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_TERMS,\n \t\tBISECT_START,\n \t\tBISECT_AUTOSTART,\n+\t\tBISECT_NEXT,\n+\t\tBISECT_AUTO_NEXT\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -723,6 +883,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"print out the bisect terms\"), BISECT_TERMS),\n \t\tOPT_CMDMODE(0, \"bisect-start\", &cmdmode,\n \t\t\t N_(\"start the bisect session\"), BISECT_START),\n+\t\tOPT_CMDMODE(0, \"bisect-next\", &cmdmode,\n+\t\t\t N_(\"find the next bisection commit\"), BISECT_NEXT),\n+\t\tOPT_CMDMODE(0, \"bisect-auto-next\", &cmdmode,\n+\t\t\t N_(\"verify the next bisection state then checkout the next bisection commit\"), BISECT_AUTO_NEXT),\n \t\tOPT_CMDMODE(0, \"bisect-autostart\", &cmdmode,\n \t\t\t N_(\"start the bisection if it has not yet been started\"), BISECT_AUTOSTART),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n@@ -784,6 +948,18 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tset_terms(&terms, \"bad\", \"good\");\n \t\tres = bisect_start(&terms, argv, argc);\n \t\tbreak;\n+\tcase BISECT_NEXT:\n+\t\tif (argc)\n+\t\t\treturn error(_(\"--bisect-next requires 0 arguments\"));\n+\t\tget_terms(&terms);\n+\t\tres = bisect_next(&terms, prefix);\n+\t\tbreak;\n+\tcase BISECT_AUTO_NEXT:\n+\t\tif (argc)\n+\t\t\treturn error(_(\"--bisect-auto-next requires 0 arguments\"));\n+\t\tget_terms(&terms);\n+\t\tres = bisect_auto_next(&terms, prefix);\n+\t\tbreak;\n \tcase BISECT_AUTOSTART:\n \t\tif (argc)\n \t\t\treturn error(_(\"--bisect-autostart does not accept arguments\"));\n@@ -799,7 +975,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t * Handle early success\n \t * From check_merge_bases > check_good_are_ancestors_of_bad > bisect_next_all\n \t */\n-\tif (res == BISECT_INTERNAL_SUCCESS_MERGE_BASE)\n+\tif ((res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) || (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND))\n \t\tres = BISECT_OK;\n \n \treturn -res;\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 4e9b2d36cb..ea7e684ebb 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -65,8 +65,7 @@ bisect_start() {\n \t#\n \t# Check if we can proceed to the next bisect state.\n \t#\n-\tget_terms\n-\tbisect_auto_next\n+\tgit bisect--helper --bisect-auto-next || exit\n \n \ttrap '-' 0\n }\n@@ -119,45 +118,7 @@ bisect_state() {\n \t*)\n \t\tusage ;;\n \tesac\n-\tbisect_auto_next\n-}\n-\n-bisect_auto_next() {\n-\tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD && bisect_next || :\n-}\n-\n-bisect_next() {\n-\tcase \"$#\" in 0) ;; *) usage ;; esac\n-\tgit bisect--helper --bisect-autostart\n-\tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD $TERM_GOOD|| exit\n-\n-\t# Perform all bisection computation, display and checkout\n-\tgit bisect--helper --next-all\n-\tres=$?\n-\n-\t# Check if we should exit because bisection is finished\n-\tif test $res -eq 10\n-\tthen\n-\t\tbad_rev=$(git show-ref --hash --verify refs/bisect/$TERM_BAD)\n-\t\tbad_commit=$(git show-branch $bad_rev)\n-\t\techo \"# first $TERM_BAD commit: $bad_commit\" >>\"$GIT_DIR/BISECT_LOG\"\n-\t\texit 0\n-\telif test $res -eq 2\n-\tthen\n-\t\techo \"# only skipped commits left to test\" >>\"$GIT_DIR/BISECT_LOG\"\n-\t\tgood_revs=$(git for-each-ref --format=\"%(objectname)\" \"refs/bisect/$TERM_GOOD-*\")\n-\t\tfor skipped in $(git rev-list refs/bisect/$TERM_BAD --not $good_revs)\n-\t\tdo\n-\t\t\tskipped_commit=$(git show-branch $skipped)\n-\t\t\techo \"# possible first $TERM_BAD commit: $skipped_commit\" >>\"$GIT_DIR/BISECT_LOG\"\n-\t\tdone\n-\t\texit $res\n-\tfi\n-\n-\t# Check for an error in the bisection process\n-\ttest $res -ne 0 && exit $res\n-\n-\treturn 0\n+\tgit bisect--helper --bisect-auto-next\n }\n \n bisect_visualize() {\n@@ -213,7 +174,7 @@ bisect_replay () {\n \t\tesac\n \tdone <\"$file\"\n \tIFS=\"$oIFS\"\n-\tbisect_auto_next\n+\tgit bisect--helper --bisect-auto-next || exit\n }\n \n bisect_run () {\n@@ -310,7 +271,7 @@ case \"$#\" in\n \t\tbisect_skip \"$@\" ;;\n \tnext)\n \t\t# Not sure we want \"next\" at the UI level anymore.\n-\t\tbisect_next \"$@\" ;;\n+\t\tgit bisect--helper --bisect-next \"$@\" || exit ;;\n \tvisualize|view)\n \t\tbisect_visualize \"$@\" ;;\n \treset)\n-- \n2.25.0\n\n"},{"id":"406197","messageId":"xmqqmu1gjwqm.fsf@gitster.c.googlers.com","threadId":"54279","inReplyTo":"20200923072740.20772-1-mirucam@gmail.com","subject":"Re: [PATCH v8 0/6] Finish converting git bisect to C part 2 subset 1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-23T16:14:57Z","receivedAt":"2020-09-23T16:15:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miriam Rubio writes:\n\n> These patches correspond to a second part of patch series \n> of Outreachy project \"Finish converting `git bisect` from shell to C\" \n> started by Pranit Bauva and Tanushree Tumane\n> (https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and\n> continued by me.\n>\n> This patch series version is composed by the first six patches of the\n> previous version due to a bug discovered during the test stage, that \n> affects the rest of the patches and that needs to be fixed before \n> sending them.\n\nThanks for being careful.\n\n> * Rebased on 385c171a01, (Fifteenth batch, 2020-09-18) to update command\n> help messages.\n\nPersonally I find this rebase unwarranted. What ef4d9f8a (bisect:\nswap command-line options in documentation, 2020-08-28) on the\n'master' did does not overlap with this series in any meaningful\nway. Since it is also trivial to adjust these patches back on\ne9b77c84 (Tenth batch, 2020-08-24) on which the previous round was\nbased, I won't complain ;-)\n\n> [4/6] bisect--helper: reimplement `bisect_autostart` shell function in C\n>\n> * Amend commit message.\n> * Change to `return -1` when we cannot autostart.\n> * Add `|| exit` to bisect-autostart call in shell script.\n\nSounds good.\n\n> [6/6] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell\n> functions in C\n> \n> * Amend commit message.\n> * Amend new modes order in cmdmode enum.\n\nMeaning that the two new ones can just be at the end and there\nwasn't a reason for them to come before autostart? OK.\n\n> * Add `|| exit` to bisect-auto-next call in shell script.\n\nOK.\n\nThanks.\n"},{"id":"406211","messageId":"nycvar.QRO.7.76.6.2009231640370.5061@tvgsbejvaqbjf.bet","threadId":"54279","inReplyTo":"20200923072740.20772-5-mirucam@gmail.com","subject":"Re: [PATCH v8 4/6] bisect--helper: reimplement `bisect_autostart` shell function in C","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-09-23T14:41:15Z","receivedAt":"2020-09-23T18:43:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Miriam,\n\nOn Wed, 23 Sep 2020, Miriam Rubio wrote:\n\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index 46d2abb36f..ab27a8dff6 100644\n> --- a/builtin/bisect--helper.c\n> +++ b/builtin/bisect--helper.c\n> @@ -29,6 +29,7 @@ static const char * const git_bisect_helper_usage[] = {\n> \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n> \tN_(\"git bisect--helper --bisect-start [--term-{new,bad}= --term-{old,good}=]\"\n> \t\t\t\t\t \" [--no-checkout] [--first-parent] [ [...]] [--] [...]\"),\n> +\tN_(\"git bisect--helper --bisect-autostart\"),\n> \tNULL\n> };\n>\n> @@ -653,6 +654,38 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n> \treturn res;\n> }\n>\n> +static inline int file_is_not_empty(const char *path)\n> +{\n> +\treturn !is_empty_or_missing_file(path);\n> +}\n> +\n> +static int bisect_autostart(struct bisect_terms *terms)\n> +{\n> +\tint res;\n> +\tconst char *yesno;\n> +\n> +\tif (file_is_not_empty(git_path_bisect_start()))\n> +\t\treturn 0;\n> +\n> +\tfprintf_ln(stderr, _(\"You need to start by \\\"git bisect \"\n> +\t\t\t \"start\\\"\\n\"));\n> +\n> +\tif (!isatty(STDIN_FILENO))\n> +\t\treturn -1;\n> +\n> +\t/*\n> +\t * TRANSLATORS: Make sure to include [Y] and [n] in your\n> +\t * translation. The program will only accept English input\n> +\t * at this point.\n> +\t */\n> +\tyesno = git_prompt(_(\"Do you want me to do it for you \"\n> +\t\t\t \"[Y/n]? \"), PROMPT_ECHO);\n> +\tres = tolower(*yesno) == 'n' ?\n> +\t\t-1 : bisect_start(terms, empty_strvec, 0);\n> +\n> +\treturn res;\n> +}\n> +\n> int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> {\n> \tenum {\n> @@ -665,7 +698,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> \t\tCHECK_AND_SET_TERMS,\n> \t\tBISECT_NEXT_CHECK,\n> \t\tBISECT_TERMS,\n> -\t\tBISECT_START\n> +\t\tBISECT_START,\n> +\t\tBISECT_AUTOSTART,\n> \t} cmdmode = 0;\n> \tint res = 0, nolog = 0;\n> \tstruct option options[] = {\n> @@ -689,6 +723,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> \t\t\t N_(\"print out the bisect terms\"), BISECT_TERMS),\n> \t\tOPT_CMDMODE(0, \"bisect-start\", &cmdmode,\n> \t\t\t N_(\"start the bisect session\"), BISECT_START),\n> +\t\tOPT_CMDMODE(0, \"bisect-autostart\", &cmdmode,\n> +\t\t\t N_(\"start the bisection if it has not yet been started\"), BISECT_AUTOSTART),\n> \t\tOPT_BOOL(0, \"no-log\", &nolog,\n> \t\t\t N_(\"no log for BISECT_WRITE\")),\n> \t\tOPT_END()\n> @@ -748,6 +784,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> \t\tset_terms(&terms, \"bad\", \"good\");\n> \t\tres = bisect_start(&terms, argv, argc);\n> \t\tbreak;\n> +\tcase BISECT_AUTOSTART:\n> +\t\tif (argc)\n> +\t\t\treturn error(_(\"--bisect-autostart does not accept arguments\"));\n> +\t\tset_terms(&terms, \"bad\", \"good\");\n> +\t\tres = bisect_autostart(&terms);\n> +\t\tbreak;\n> \tdefault:\n> \t\tBUG(\"unknown subcommand %d\", cmdmode);\n> \t}\n> diff --git a/git-bisect.sh b/git-bisect.sh\n> index 2f60fefcfa..4e9b2d36cb 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -49,27 +49,6 @@ bisect_head()\n> \tfi\n> }\n>\n> -bisect_autostart() {\n> -\ttest -s \"$GIT_DIR/BISECT_START\" || {\n> -\t\tgettextln \"You need to start by \\\"git bisect start\\\"\" >&2\n> -\t\tif test -t 0\n> -\t\tthen\n> -\t\t\t# TRANSLATORS: Make sure to include [Y] and [n] in your\n> -\t\t\t# translation. The program will only accept English input\n> -\t\t\t# at this point.\n> -\t\t\tgettext \"Do you want me to do it for you [Y/n]? \" >&2\n> -\t\t\tread yesno\n> -\t\t\tcase \"$yesno\" in\n> -\t\t\t[Nn]*)\n> -\t\t\t\texit ;;\n> -\t\t\tesac\n> -\t\t\tbisect_start\n> -\t\telse\n> -\t\t\texit 1\n> -\t\tfi\n> -\t}\n> -}\n> -\n> bisect_start() {\n> \tgit bisect--helper --bisect-start $@ || exit\n>\n> @@ -108,7 +87,7 @@ bisect_skip() {\n> }\n>\n> bisect_state() {\n> -\tbisect_autostart\n> +\tgit bisect--helper --bisect-autostart || exit\n> \tstate=$1\n> \tgit bisect--helper --check-and-set-terms $state $TERM_GOOD $TERM_BAD || exit\n> \tget_terms\n> @@ -149,7 +128,7 @@ bisect_auto_next() {\n>\n> bisect_next() {\n> \tcase \"$#\" in 0) ;; *) usage ;; esac\n> -\tbisect_autostart\n> +\tgit bisect--helper --bisect-autostart\n\nJust like above, we need an `|| exit` here.\n\nOther than that, this patch looks good to me.\n\nThanks,\nDscho\n\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> --\n> 2.25.0\n>\n>\n"},{"id":"406212","messageId":"nycvar.QRO.7.76.6.2009231645380.5061@tvgsbejvaqbjf.bet","threadId":"54279","inReplyTo":"20200923072740.20772-1-mirucam@gmail.com","subject":"Re: [PATCH v8 0/6] Finish converting git bisect to C part 2 subset 1","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-09-23T14:46:49Z","receivedAt":"2020-09-23T18:49:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Miriam,\n\nOn Wed, 23 Sep 2020, Miriam Rubio wrote:\n\n> General changes\n> ---------------\n>\n> * Rebased on 385c171a01, (Fifteenth batch, 2020-09-18) to update command\n> help messages.\n>\n>\n> Specific changes\n> ----------------\n>\n>\n> [4/6] bisect--helper: reimplement `bisect_autostart` shell function in C\n>\n> * Amend commit message.\n> * Change to `return -1` when we cannot autostart.\n> * Add `|| exit` to bisect-autostart call in shell script.\n\nI think the `bisect_next` function needs the same treatment. Other than\nthat, this iteration is ready to go from my side.\n\nThank you,\nDscho\n"}]}