{"thread":{"id":"55036","subject":"[PATCH v3 0/7] Finish converting git bisect to C part 3","startedAt":"2021-01-23T15:42:47Z","lastAt":"2021-01-27T14:14:48Z","messageCount":19,"participants":["Miriam Rubio","Junio C Hamano","Rafael Silva","Miriam R."],"isPatch":true,"patchVersion":3,"patchTotal":7},"messages":[{"id":"415061","messageId":"20210123154056.48234-1-mirucam@gmail.com","threadId":"55036","inReplyTo":null,"subject":"[PATCH v3 0/7] Finish converting git bisect to C part 3","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-23T15:40:49Z","receivedAt":"2021-01-23T15:42:47Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"These patches correspond to a third 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 third part is formed by reimplementations of some `git bisect` \nsubcommands and removal of some temporary subcommands.\n\nThese patch series emails were generated from:\nhttps://gitlab.com/mirucam/git/commits/git-bisect-work-part3_v2.\n\nI would like to thank Rafael Silva and Johannes Schindelin for their\nreviews and suggestions.\n\nSpecific changes\n----------------\n\n[1/7] bisect--helper: reimplement `bisect_log` shell function in C\n* Change command description message.\n---\n\n[2/7] bisect--helper: reimplement `bisect_replay` shell function in C\n* Remove get_next_word().\n* Change process_replay_line()'s while loop implementation.\n* Change error message.\n---\n\n[6/7] bisect--helper: reimplement `bisect_skip` shell function in C\n* Use strstr() instead of wildmatch.\n* Remove strvec use.\n---\n\nPranit Bauva (7):\n bisect--helper: reimplement `bisect_log` shell function in C\n bisect--helper: reimplement `bisect_replay` shell function in C\n bisect--helper: retire `--bisect-write` subcommand\n bisect--helper: use `res` instead of return in BISECT_RESET case\n option\n bisect--helper: retire `--bisect-auto-next` subcommand\n bisect--helper: reimplement `bisect_skip` shell function in C\n bisect--helper: retire `--check-and-set-terms` subcommand\n\n builtin/bisect--helper.c | 178 ++++++++++++++++++++++++++++++++-------\n git-bisect.sh | 58 +------------\n 2 files changed, 150 insertions(+), 86 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"415062","messageId":"20210123154056.48234-2-mirucam@gmail.com","threadId":"55036","inReplyTo":"20210123154056.48234-1-mirucam@gmail.com","subject":"[PATCH v3 1/7] bisect--helper: reimplement `bisect_log` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-23T15:40:50Z","receivedAt":"2021-01-23T15:42:47Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nReimplement the `bisect_log()` shell function in C and also add\n`--bisect-log` subcommand to `git bisect--helper` to call it from\ngit-bisect.sh .\n\nUsing `--bisect-log` subcommand is a temporary measure to port shell\nfunction to C so as to use the existing test suite.\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 | 22 +++++++++++++++++++++-\n git-bisect.sh | 7 +------\n 2 files changed, 22 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 709eb713a3..a65244a0f5 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -904,6 +904,18 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a\n \treturn bisect_auto_next(terms, NULL);\n }\n \n+static enum bisect_error bisect_log(void)\n+{\n+\tint fd, status;\n+\tfd = open(git_path_bisect_log(), O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn BISECT_FAILED;\n+\n+\tstatus = copy_fd(fd, STDOUT_FILENO);\n+\tclose(fd);\n+\treturn status ? BISECT_FAILED : BISECT_OK;\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -916,7 +928,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_AUTOSTART,\n \t\tBISECT_NEXT,\n \t\tBISECT_AUTO_NEXT,\n-\t\tBISECT_STATE\n+\t\tBISECT_STATE,\n+\t\tBISECT_LOG\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -938,6 +951,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-state\", &cmdmode,\n \t\t\t N_(\"mark the state of ref (or refs)\"), BISECT_STATE),\n+\t\tOPT_CMDMODE(0, \"bisect-log\", &cmdmode,\n+\t\t\t N_(\"list the bisection steps so far\"), BISECT_LOG),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -1000,6 +1015,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tget_terms(&terms);\n \t\tres = bisect_state(&terms, argv, argc);\n \t\tbreak;\n+\tcase BISECT_LOG:\n+\t\tif (argc)\n+\t\t\treturn error(_(\"--bisect-log requires 0 arguments\"));\n+\t\tres = bisect_log();\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 1f3f6e9fc5..c6149846ff 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -169,11 +169,6 @@ exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n \tdone\n }\n \n-bisect_log () {\n-\ttest -s \"$GIT_DIR/BISECT_LOG\" || die \"$(gettext \"We are not bisecting.\")\"\n-\tcat \"$GIT_DIR/BISECT_LOG\"\n-}\n-\n get_terms () {\n \tif test -s \"$GIT_DIR/BISECT_TERMS\"\n \tthen\n@@ -210,7 +205,7 @@ case \"$#\" in\n \treplay)\n \t\tbisect_replay \"$@\" ;;\n \tlog)\n-\t\tbisect_log ;;\n+\t\tgit bisect--helper --bisect-log || exit;;\n \trun)\n \t\tbisect_run \"$@\" ;;\n \tterms)\n-- \n2.29.2\n\n"},{"id":"415063","messageId":"20210123154056.48234-4-mirucam@gmail.com","threadId":"55036","inReplyTo":"20210123154056.48234-1-mirucam@gmail.com","subject":"[PATCH v3 3/7] bisect--helper: retire `--bisect-write` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-23T15:40:52Z","receivedAt":"2021-01-23T15:42:48Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nThe `--bisect-write` subcommand is no longer used from the\ngit-bisect.sh shell script. Instead the function `bisect_write()`\nis directly called from the C implementation.\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 | 9 ---------\n 1 file changed, 9 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 4d8dca3717..1f731167f6 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -21,7 +21,6 @@ static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n \n static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-reset []\"),\n-\tN_(\"git bisect--helper --bisect-write [--no-log] \"),\n \tN_(\"git bisect--helper --bisect-check-and-set-terms \"),\n \tN_(\"git bisect--helper --bisect-next-check []\"),\n \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n@@ -993,7 +992,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n \t\tBISECT_RESET = 1,\n-\t\tBISECT_WRITE,\n \t\tCHECK_AND_SET_TERMS,\n \t\tBISECT_NEXT_CHECK,\n \t\tBISECT_TERMS,\n@@ -1009,8 +1007,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\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-\t\t\t N_(\"write out the bisection state in BISECT_LOG\"), BISECT_WRITE),\n \t\tOPT_CMDMODE(0, \"check-and-set-terms\", &cmdmode,\n \t\t\t N_(\"check and set terms in a bisection state\"), CHECK_AND_SET_TERMS),\n \t\tOPT_CMDMODE(0, \"bisect-next-check\", &cmdmode,\n@@ -1047,11 +1043,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tif (argc > 1)\n \t\t\treturn error(_(\"--bisect-reset requires either no argument or a commit\"));\n \t\treturn !!bisect_reset(argc ? argv[0] : NULL);\n-\tcase BISECT_WRITE:\n-\t\tif (argc != 4 && argc != 5)\n-\t\t\treturn error(_(\"--bisect-write requires either 4 or 5 arguments\"));\n-\t\tset_terms(&terms, argv[3], argv[2]);\n-\t\tres = bisect_write(argv[0], argv[1], &terms, nolog);\n \t\tbreak;\n \tcase CHECK_AND_SET_TERMS:\n \t\tif (argc != 3)\n-- \n2.29.2\n\n"},{"id":"415064","messageId":"20210123154056.48234-3-mirucam@gmail.com","threadId":"55036","inReplyTo":"20210123154056.48234-1-mirucam@gmail.com","subject":"[PATCH v3 2/7] bisect--helper: reimplement `bisect_replay` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-23T15:40:51Z","receivedAt":"2021-01-23T15:42:48Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nReimplement the `bisect_replay` shell function in C and also add\n`--bisect-replay` subcommand to `git bisect--helper` to call it from\ngit-bisect.sh\n\nUsing `--bisect-replay` subcommand is a temporary measure to port shell\nfunction to C so as to use the existing test suite.\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 | 84 +++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh | 34 +---------------\n 2 files changed, 84 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex a65244a0f5..4d8dca3717 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -31,6 +31,7 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-auto-next\"),\n \tN_(\"git bisect--helper --bisect-state (bad|new) []\"),\n \tN_(\"git bisect--helper --bisect-state (good|old) [...]\"),\n+\tN_(\"git bisect--helper --bisect-replay \"),\n \tNULL\n };\n \n@@ -916,6 +917,78 @@ static enum bisect_error bisect_log(void)\n \treturn status ? BISECT_FAILED : BISECT_OK;\n }\n \n+static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)\n+{\n+\tconst char *p = line->buf + strspn(line->buf, \" \\t\");\n+\n+\tif ((!skip_prefix(p, \"git bisect\", &p) &&\n+\t!skip_prefix(p, \"git-bisect\", &p)) || !isspace(*p))\n+\t\treturn 0;\n+\tp += strspn(p, \" \\t\");\n+\n+\tchar *word_end = (char*)p + strcspn(p, \" \\t\");\n+\tchar *rev = word_end + strspn(word_end, \" \\t\");\n+\t*word_end = '\\0'; /* NUL-terminate the word */\n+\n+\tget_terms(terms);\n+\tif (check_and_set_terms(terms, p))\n+\t\treturn -1;\n+\n+\tif (!strcmp(p, \"start\")) {\n+\t\tstruct strvec argv = STRVEC_INIT;\n+\t\tint res;\n+\t\tsq_dequote_to_strvec(rev, &argv);\n+\t\tres = bisect_start(terms, argv.v, argv.nr);\n+\t\tstrvec_clear(&argv);\n+\t\treturn res;\n+\t}\n+\n+\tif (one_of(p, terms->term_good,\n+\t terms->term_bad, \"skip\", NULL))\n+\t\treturn bisect_write(p, rev, terms, 0);\n+\n+\tif (!strcmp(p, \"terms\")) {\n+\t\tstruct strvec argv = STRVEC_INIT;\n+\t\tint res;\n+\t\tsq_dequote_to_strvec(rev, &argv);\n+\t\tres = bisect_terms(terms, argv.nr == 1 ? argv.v[0] : NULL);\n+\t\tstrvec_clear(&argv);\n+\t\treturn res;\n+\t}\n+\terror(_(\"'%s'?? what are you talking about?\"), p);\n+\n+\treturn -1;\n+}\n+\n+static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *filename)\n+{\n+\tFILE *fp = NULL;\n+\tenum bisect_error res = BISECT_OK;\n+\tstruct strbuf line = STRBUF_INIT;\n+\n+\tif (is_empty_or_missing_file(filename))\n+\t\treturn error(_(\"cannot read file '%s' for replaying\"), filename);\n+\n+\tif (bisect_reset(NULL))\n+\t\treturn BISECT_FAILED;\n+\n+\tfp = fopen(filename, \"r\");\n+\tif (!fp)\n+\t\treturn BISECT_FAILED;\n+\n+\twhile ((strbuf_getline(&line, fp) != EOF) && !res){\n+\t\tres = process_replay_line(terms, &line);\n+\t}\n+\n+\tstrbuf_release(&line);\n+\tfclose(fp);\n+\n+\tif (res)\n+\t\treturn BISECT_FAILED;\n+\n+\treturn bisect_auto_next(terms, NULL);\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -929,7 +1002,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_NEXT,\n \t\tBISECT_AUTO_NEXT,\n \t\tBISECT_STATE,\n-\t\tBISECT_LOG\n+\t\tBISECT_LOG,\n+\t\tBISECT_REPLAY\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -953,6 +1027,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"mark the state of ref (or refs)\"), BISECT_STATE),\n \t\tOPT_CMDMODE(0, \"bisect-log\", &cmdmode,\n \t\t\t N_(\"list the bisection steps so far\"), BISECT_LOG),\n+\t\tOPT_CMDMODE(0, \"bisect-replay\", &cmdmode,\n+\t\t\t N_(\"replay the bisection process from the given file\"), BISECT_REPLAY),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -1020,6 +1096,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\treturn error(_(\"--bisect-log requires 0 arguments\"));\n \t\tres = bisect_log();\n \t\tbreak;\n+\tcase BISECT_REPLAY:\n+\t\tif (argc != 1)\n+\t\t\treturn error(_(\"no logfile given\"));\n+\t\tset_terms(&terms, \"bad\", \"good\");\n+\t\tres = bisect_replay(&terms, argv[0]);\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 c6149846ff..79bcd31bd7 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -77,38 +77,6 @@ bisect_visualize() {\n \teval '\"$@\"' --bisect -- $(cat \"$GIT_DIR/BISECT_NAMES\")\n }\n \n-bisect_replay () {\n-\tfile=\"$1\"\n-\ttest \"$#\" -eq 1 || die \"$(gettext \"No logfile given\")\"\n-\ttest -r \"$file\" || die \"$(eval_gettext \"cannot read \\$file for replaying\")\"\n-\tgit bisect--helper --bisect-reset || exit\n-\toIFS=\"$IFS\" IFS=\"$IFS$(printf '\\015')\"\n-\twhile read git bisect command rev tail\n-\tdo\n-\t\ttest \"$git $bisect\" = \"git bisect\" || test \"$git\" = \"git-bisect\" || continue\n-\t\tif test \"$git\" = \"git-bisect\"\n-\t\tthen\n-\t\t\trev=\"$command\"\n-\t\t\tcommand=\"$bisect\"\n-\t\tfi\n-\t\tget_terms\n-\t\tgit bisect--helper --check-and-set-terms \"$command\" \"$TERM_GOOD\" \"$TERM_BAD\" || exit\n-\t\tget_terms\n-\t\tcase \"$command\" in\n-\t\tstart)\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-\t\t\tgit bisect--helper --bisect-terms $rev || exit;;\n-\t\t*)\n-\t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n-\t\tesac\n-\tdone <\"$file\"\n-\tIFS=\"$oIFS\"\n-\tgit bisect--helper --bisect-auto-next || exit\n-}\n-\n bisect_run () {\n \tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n \n@@ -203,7 +171,7 @@ case \"$#\" in\n \treset)\n \t\tgit bisect--helper --bisect-reset \"$@\" ;;\n \treplay)\n-\t\tbisect_replay \"$@\" ;;\n+\t\tgit bisect--helper --bisect-replay \"$@\" || exit;;\n \tlog)\n \t\tgit bisect--helper --bisect-log || exit;;\n \trun)\n-- \n2.29.2\n\n"},{"id":"415065","messageId":"20210123154056.48234-6-mirucam@gmail.com","threadId":"55036","inReplyTo":"20210123154056.48234-1-mirucam@gmail.com","subject":"[PATCH v3 5/7] bisect--helper: retire `--bisect-auto-next` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-23T15:40:54Z","receivedAt":"2021-01-23T15:42:48Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nThe --bisect-auto-next subcommand is no longer used from the\ngit-bisect.sh shell script. Instead the function bisect_auto_next()\nis directly called from the C implementation.\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 | 10 ----------\n 1 file changed, 10 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 1ce0399ad4..1e53845e69 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -27,7 +27,6 @@ static const char * const git_bisect_helper_usage[] = {\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-state (bad|new) []\"),\n \tN_(\"git bisect--helper --bisect-state (good|old) [...]\"),\n \tN_(\"git bisect--helper --bisect-replay \"),\n@@ -998,7 +997,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_START,\n \t\tBISECT_AUTOSTART,\n \t\tBISECT_NEXT,\n-\t\tBISECT_AUTO_NEXT,\n \t\tBISECT_STATE,\n \t\tBISECT_LOG,\n \t\tBISECT_REPLAY\n@@ -1017,8 +1015,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\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-state\", &cmdmode,\n \t\t\t N_(\"mark the state of ref (or refs)\"), BISECT_STATE),\n \t\tOPT_CMDMODE(0, \"bisect-log\", &cmdmode,\n@@ -1071,12 +1067,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\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_STATE:\n \t\tset_terms(&terms, \"bad\", \"good\");\n \t\tget_terms(&terms);\n-- \n2.29.2\n\n"},{"id":"415066","messageId":"20210123154056.48234-5-mirucam@gmail.com","threadId":"55036","inReplyTo":"20210123154056.48234-1-mirucam@gmail.com","subject":"[PATCH v3 4/7] bisect--helper: use `res` instead of return in BISECT_RESET case option","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-23T15:40:53Z","receivedAt":"2021-01-23T15:42:48Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nUse `res` variable to store `bisect_reset()` output in BISECT_RESET\ncase option to make bisect--helper.c more consistent.\n\nMentored-by: Christian Couder \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 1f731167f6..1ce0399ad4 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -1042,7 +1042,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \tcase BISECT_RESET:\n \t\tif (argc > 1)\n \t\t\treturn error(_(\"--bisect-reset requires either no argument or a commit\"));\n-\t\treturn !!bisect_reset(argc ? argv[0] : NULL);\n+\t\tres = bisect_reset(argc ? argv[0] : NULL);\n \t\tbreak;\n \tcase CHECK_AND_SET_TERMS:\n \t\tif (argc != 3)\n-- \n2.29.2\n\n"},{"id":"415067","messageId":"20210123154056.48234-7-mirucam@gmail.com","threadId":"55036","inReplyTo":"20210123154056.48234-1-mirucam@gmail.com","subject":"[PATCH v3 6/7] bisect--helper: reimplement `bisect_skip` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-23T15:40:55Z","receivedAt":"2021-01-23T15:43:14Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nReimplement the `bisect_skip()` shell function in C and also add\n`bisect-skip` subcommand to `git bisect--helper` to call it from\ngit-bisect.sh\n\nUsing `--bisect-skip` subcommand is a temporary measure to port shell\nfunction to C so as to use the existing test suite.\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 | 45 +++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh | 17 +--------------\n 2 files changed, 45 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 1e53845e69..61ddaa6b9c 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -30,6 +30,7 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-state (bad|new) []\"),\n \tN_(\"git bisect--helper --bisect-state (good|old) [...]\"),\n \tN_(\"git bisect--helper --bisect-replay \"),\n+\tN_(\"git bisect--helper --bisect-skip [(|)...]\"),\n \tNULL\n };\n \n@@ -987,6 +988,41 @@ static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f\n \treturn bisect_auto_next(terms, NULL);\n }\n \n+static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **argv, int argc)\n+{\n+\tint i;\n+\tenum bisect_error res;\n+\tstruct strvec argv_state = STRVEC_INIT;\n+\n+\tstrvec_push(&argv_state, \"skip\");\n+\n+\tfor (i = 0; i < argc; i++) {\n+\t\tconst char *dotdot = strstr(argv[i], \"..\");\n+\n+\t\tif (dotdot) {\n+\t\t\tstruct rev_info revs;\n+\t\t\tstruct commit *commit;\n+\n+\t\t\tinit_revisions(&revs, NULL);\n+\t\t\tsetup_revisions(2, argv + i - 1, &revs, NULL);\n+\n+\t\t\tif (prepare_revision_walk(&revs))\n+\t\t\t\tdie(_(\"revision walk setup failed\\n\"));\n+\t\t\twhile ((commit = get_revision(&revs)) != NULL)\n+\t\t\t\tstrvec_push(&argv_state,\n+\t\t\t\t\t\toid_to_hex(&commit->object.oid));\n+\n+\t\t\treset_revision_walk();\n+\t\t} else {\n+\t\t\tstrvec_push(&argv_state, argv[i]);\n+\t\t}\n+\t}\n+\tres = bisect_state(terms, argv_state.v, argv_state.nr);\n+\n+\tstrvec_clear(&argv_state);\n+\treturn res;\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -999,7 +1035,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_NEXT,\n \t\tBISECT_STATE,\n \t\tBISECT_LOG,\n-\t\tBISECT_REPLAY\n+\t\tBISECT_REPLAY,\n+\t\tBISECT_SKIP\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -1021,6 +1058,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"list the bisection steps so far\"), BISECT_LOG),\n \t\tOPT_CMDMODE(0, \"bisect-replay\", &cmdmode,\n \t\t\t N_(\"replay the bisection process from the given file\"), BISECT_REPLAY),\n+\t\tOPT_CMDMODE(0, \"bisect-skip\", &cmdmode,\n+\t\t\t N_(\"skip some commits for checkout\"), BISECT_SKIP),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -1083,6 +1122,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tset_terms(&terms, \"bad\", \"good\");\n \t\tres = bisect_replay(&terms, argv[0]);\n \t\tbreak;\n+\tcase BISECT_SKIP:\n+\t\tset_terms(&terms, \"bad\", \"good\");\n+\t\tres = bisect_skip(&terms, argv, argc);\n+\t\tbreak;\n \tdefault:\n \t\tBUG(\"unknown subcommand %d\", cmdmode);\n \t}\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 79bcd31bd7..016cc34e03 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -39,21 +39,6 @@ _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n TERM_BAD=bad\n TERM_GOOD=good\n \n-bisect_skip() {\n-\tall=''\n-\tfor arg in \"$@\"\n-\tdo\n-\t\tcase \"$arg\" in\n-\t\t*..*)\n-\t\t\trevs=$(git rev-list \"$arg\") || die \"$(eval_gettext \"Bad rev input: \\$arg\")\" ;;\n-\t\t*)\n-\t\t\trevs=$(git rev-parse --sq-quote \"$arg\") ;;\n-\t\tesac\n-\t\tall=\"$all $revs\"\n-\tdone\n-\teval git bisect--helper --bisect-state 'skip' $all\n-}\n-\n bisect_visualize() {\n \tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n \n@@ -162,7 +147,7 @@ case \"$#\" in\n \tbad|good|new|old|\"$TERM_BAD\"|\"$TERM_GOOD\")\n \t\tgit bisect--helper --bisect-state \"$cmd\" \"$@\" ;;\n \tskip)\n-\t\tbisect_skip \"$@\" ;;\n+\t\tgit bisect--helper --bisect-skip \"$@\" || exit;;\n \tnext)\n \t\t# Not sure we want \"next\" at the UI level anymore.\n \t\tgit bisect--helper --bisect-next \"$@\" || exit ;;\n-- \n2.29.2\n\n"},{"id":"415068","messageId":"20210123154056.48234-8-mirucam@gmail.com","threadId":"55036","inReplyTo":"20210123154056.48234-1-mirucam@gmail.com","subject":"[PATCH v3 7/7] bisect--helper: retire `--check-and-set-terms` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-23T15:40:56Z","receivedAt":"2021-01-23T15:43:15Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nThe `--check-and-set-terms` subcommand is no longer from the\ngit-bisect.sh shell script. Instead the function\n`check_and_set_terms()` is called from the C implementation.\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 | 10 ----------\n 1 file changed, 10 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 61ddaa6b9c..3b0a6a7d0c 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -21,7 +21,6 @@ static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n \n static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-reset []\"),\n-\tN_(\"git bisect--helper --bisect-check-and-set-terms \"),\n \tN_(\"git bisect--helper --bisect-next-check []\"),\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@@ -1027,7 +1026,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n \t\tBISECT_RESET = 1,\n-\t\tCHECK_AND_SET_TERMS,\n \t\tBISECT_NEXT_CHECK,\n \t\tBISECT_TERMS,\n \t\tBISECT_START,\n@@ -1042,8 +1040,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_CMDMODE(0, \"bisect-reset\", &cmdmode,\n \t\t\t N_(\"reset the bisection state\"), BISECT_RESET),\n-\t\tOPT_CMDMODE(0, \"check-and-set-terms\", &cmdmode,\n-\t\t\t N_(\"check and set terms in a bisection state\"), CHECK_AND_SET_TERMS),\n \t\tOPT_CMDMODE(0, \"bisect-next-check\", &cmdmode,\n \t\t\t N_(\"check whether bad or good terms exist\"), BISECT_NEXT_CHECK),\n \t\tOPT_CMDMODE(0, \"bisect-terms\", &cmdmode,\n@@ -1079,12 +1075,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\treturn error(_(\"--bisect-reset requires either no argument or a commit\"));\n \t\tres = bisect_reset(argc ? argv[0] : NULL);\n \t\tbreak;\n-\tcase CHECK_AND_SET_TERMS:\n-\t\tif (argc != 3)\n-\t\t\treturn error(_(\"--check-and-set-terms requires 3 arguments\"));\n-\t\tset_terms(&terms, argv[2], argv[1]);\n-\t\tres = check_and_set_terms(&terms, argv[0]);\n-\t\tbreak;\n \tcase BISECT_NEXT_CHECK:\n \t\tif (argc != 2 && argc != 3)\n \t\t\treturn error(_(\"--bisect-next-check requires 2 or 3 arguments\"));\n-- \n2.29.2\n\n"},{"id":"415109","messageId":"xmqq35yr2nh0.fsf@gitster.c.googlers.com","threadId":"55036","inReplyTo":"20210123154056.48234-3-mirucam@gmail.com","subject":"Re: [PATCH v3 2/7] bisect--helper: reimplement `bisect_replay` shell function in C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-24T00:22:03Z","receivedAt":"2021-01-24T00:23:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miriam Rubio writes:\n\n> From: Pranit Bauva \n>\n> Reimplement the `bisect_replay` shell function in C and also add\n> `--bisect-replay` subcommand to `git bisect--helper` to call it from\n> git-bisect.sh\n> ...\n> +static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)\n> +{\n> +\tconst char *p = line->buf + strspn(line->buf, \" \\t\");\n> +\n> +\tif ((!skip_prefix(p, \"git bisect\", &p) &&\n> +\t!skip_prefix(p, \"git-bisect\", &p)) || !isspace(*p))\n> +\t\treturn 0;\n> +\tp += strspn(p, \" \\t\");\n> +\n> +\tchar *word_end = (char*)p + strcspn(p, \" \\t\");\n> +\tchar *rev = word_end + strspn(word_end, \" \\t\");\n\nAre these lines new in this round?\n\nThese break builds with -Werror=declaration-after-statement.\n\n"},{"id":"415124","messageId":"xmqqr1ma29z7.fsf@gitster.c.googlers.com","threadId":"55036","inReplyTo":"xmqq35yr2nh0.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 2/7] bisect--helper: reimplement `bisect_replay` shell function in C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-24T05:13:32Z","receivedAt":"2021-01-24T05:14:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Miriam Rubio writes:\n>\n>> From: Pranit Bauva \n>>\n>> Reimplement the `bisect_replay` shell function in C and also add\n>> `--bisect-replay` subcommand to `git bisect--helper` to call it from\n>> git-bisect.sh\n>> ...\n>> +static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)\n>> +{\n>> +\tconst char *p = line->buf + strspn(line->buf, \" \\t\");\n>> +\n>> +\tif ((!skip_prefix(p, \"git bisect\", &p) &&\n>> +\t!skip_prefix(p, \"git-bisect\", &p)) || !isspace(*p))\n>> +\t\treturn 0;\n>> +\tp += strspn(p, \" \\t\");\n>> +\n>> +\tchar *word_end = (char*)p + strcspn(p, \" \\t\");\n>> +\tchar *rev = word_end + strspn(word_end, \" \\t\");\n>\n> Are these lines new in this round?\n>\n> These break builds with -Werror=declaration-after-statement.\n\n\nThe following fix-up is tentatively queued at the tip of the topic\nin tonight's pushout. There might be better way to do what this\nfunction is doing (especially I am skeptical about the way the code\ncasts const away), so please don't blindly trust it, but at least it\nshould please the compiler.\n\n(char*) should be spelled (char *), by the way.\n\nThanks.\n\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 3b0a6a7d0c..c673f87795 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -918,14 +918,15 @@ static enum bisect_error bisect_log(void)\n static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)\n {\n \tconst char *p = line->buf + strspn(line->buf, \" \\t\");\n+\tchar *word_end, *rev;\n \n \tif ((!skip_prefix(p, \"git bisect\", &p) &&\n \t!skip_prefix(p, \"git-bisect\", &p)) || !isspace(*p))\n \t\treturn 0;\n \tp += strspn(p, \" \\t\");\n \n-\tchar *word_end = (char*)p + strcspn(p, \" \\t\");\n-\tchar *rev = word_end + strspn(word_end, \" \\t\");\n+\tword_end = (char*)p + strcspn(p, \" \\t\");\n+\trev = word_end + strspn(word_end, \" \\t\");\n \t*word_end = '\\0'; /* NUL-terminate the word */\n \n \tget_terms(terms);\n-- \n2.30.0-492-gde1138eff7\n\n"},{"id":"415157","messageId":"gohp6kv9bml9qc.fsf@gmail.com","threadId":"55036","inReplyTo":"20210123154056.48234-2-mirucam@gmail.com","subject":"Re: [PATCH v3 1/7] bisect--helper: reimplement `bisect_log` shell function in C","fromName":"Rafael Silva","fromEmail":"rafaeloliveira.cs@gmail.com","sentAt":"2021-01-24T13:56:13Z","receivedAt":"2021-01-24T13:57:44Z","isPatch":true,"sender":{"key":"rafaeloliveira.cs@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5935135?v=4"},"body":"\nNice work on this series.\n\nI have one comment on this series regarding a behavior diff between the\nC and shell version, and small comment about style, see below.\n\nMiriam Rubio writes:\n\n> From: Pranit Bauva \n>\n> Reimplement the `bisect_log()` shell function in C and also add\n> `--bisect-log` subcommand to `git bisect--helper` to call it from\n> git-bisect.sh .\n>\n> Using `--bisect-log` subcommand is a temporary measure to port shell\n> function to C so as to use the existing test suite.\n>\n> Mentored-by: Lars Schneider \n> Mentored-by: Christian Couder \n> Mentored-by: Johannes Schindelin \n> Signed-off-by: Pranit Bauva \n> Signed-off-by: Tanushree Tumane \n> Signed-off-by: Miriam Rubio \n> ---\n> builtin/bisect--helper.c | 22 +++++++++++++++++++++-\n> git-bisect.sh | 7 +------\n> 2 files changed, 22 insertions(+), 7 deletions(-)\n>\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index 709eb713a3..a65244a0f5 100644\n> --- a/builtin/bisect--helper.c\n> +++ b/builtin/bisect--helper.c\n> @@ -904,6 +904,18 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a\n> \treturn bisect_auto_next(terms, NULL);\n> }\n> \n> +static enum bisect_error bisect_log(void)\n> +{\n> +\tint fd, status;\n> +\tfd = open(git_path_bisect_log(), O_RDONLY);\n> +\tif (fd < 0)\n> +\t\treturn BISECT_FAILED;\n> +\n> +\tstatus = copy_fd(fd, STDOUT_FILENO);\n> +\tclose(fd);\n> +\treturn status ? BISECT_FAILED : BISECT_OK;\n> +}\n> +\n\nIn the shell version, when we are not bisecting it the `git bisect log`\ncommand will `die` with the text \"We are not bisecting.\" which state to\nthe user that a bisect is not yet started. The shell version does that\nby checking if the `$GIT_DIR/BISECT_LOG` file doesn't exists or it's\nan empty file as the following code snippet copied from the shell\nversion that is remove by this patch:\n\n test -s \"$GIT_DIR/BISECT_LOG\" || die \"$(gettext \"We are not bisecting.\")\"\n\nThis seems to be \"missing\" from the new C version implemented by this\npatch and perhaps we should add it. I'm not sure whether this change was\nintentional and I'm missing some context here of why we are dropping\nthe message, if that's the case I apologize in advance. But, IMHO\noutputting the error message provides a better user experience as it\nwould be obvious that the user forgot to `git bisect start` instead of\nsilently failing.\n\nWith that said, perhaps an obvious way of implementing is to use\n`is_empty_or_missing_file()`, much like `bisect_replay()` does it in the\n[2/7] patch on this series, and return the same error message from\nthe shell version:\n\n-- >8 --\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex a65244a0f5..ce11383125 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -907,7 +907,12 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a\n static enum bisect_error bisect_log(void)\n {\n int fd, status;\n- fd = open(git_path_bisect_log(), O_RDONLY);\n+ const char* filename = git_path_bisect_log();\n+\n+ if (is_empty_or_missing_file(filename))\n+ return error(_(\"We are not bisecting.\"));\n+\n+ fd = open(filename, O_RDONLY);\n if (fd < 0)\n return BISECT_FAILED;\n-- >8 --\n\nAlthough I compiled and did small test on the above code snippet, don't\ntrust it blindly and perform your own test and judge whether this is the\nbest way to implement this shortcoming.\n\n>\n> @@ -210,7 +205,7 @@ case \"$#\" in\n> \treplay)\n> \t\tbisect_replay \"$@\" ;;\n> \tlog)\n> -\t\tbisect_log ;;\n> +\t\tgit bisect--helper --bisect-log || exit;;\n\nStyle: just a minor change to add a space between `exit` and `;;`.\n\n-- \nThanks\nRafael\n"},{"id":"415230","messageId":"CAN7CjDC-GdA00jWJn=cKiu-Y1AXKSOJ-Uav7ZMyeGHVSBV5HqA@mail.gmail.com","threadId":"55036","inReplyTo":"xmqq35yr2nh0.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 2/7] bisect--helper: reimplement `bisect_replay` shell function in C","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-25T19:18:49Z","receivedAt":"2021-01-25T19:20:57Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Hi Junio,\n\nEl dom, 24 ene 2021 a las 1:22, Junio C Hamano () escribió:\n>\n> Miriam Rubio writes:\n>\n> > From: Pranit Bauva \n> >\n> > Reimplement the `bisect_replay` shell function in C and also add\n> > `--bisect-replay` subcommand to `git bisect--helper` to call it from\n> > git-bisect.sh\n> > ...\n> > +static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)\n> > +{\n> > + const char *p = line->buf + strspn(line->buf, \" \\t\");\n> > +\n> > + if ((!skip_prefix(p, \"git bisect\", &p) &&\n> > + !skip_prefix(p, \"git-bisect\", &p)) || !isspace(*p))\n> > + return 0;\n> > + p += strspn(p, \" \\t\");\n> > +\n> > + char *word_end = (char*)p + strcspn(p, \" \\t\");\n> > + char *rev = word_end + strspn(word_end, \" \\t\");\n>\n> Are these lines new in this round?\n>\nYes, they are a reviewer's suggestion.\n\n> These break builds with -Werror=declaration-after-statement.\n>\nSorry about this, my compiler sometimes does not alert with some -Werror.\nI have just sent a new version.\nBest,\nMiriam\n"},{"id":"415275","messageId":"CAN7CjDANLB85GHVVn32w_Y70bzvadtqdq2uRyc81j7nz+W05jA@mail.gmail.com","threadId":"55036","inReplyTo":"gohp6kv9bml9qc.fsf@gmail.com","subject":"Re: [PATCH v3 1/7] bisect--helper: reimplement `bisect_log` shell function in C","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-25T19:23:32Z","receivedAt":"2021-01-26T05:06:10Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Hi Rafael,\n\nEl dom, 24 ene 2021 a las 14:56, Rafael Silva\n() escribió:\n>\n>\n> Nice work on this series.\n>\n> I have one comment on this series regarding a behavior diff between the\n> C and shell version, and small comment about style, see below.\n>\n> Miriam Rubio writes:\n>\n> > From: Pranit Bauva \n> >\n> > Reimplement the `bisect_log()` shell function in C and also add\n> > `--bisect-log` subcommand to `git bisect--helper` to call it from\n> > git-bisect.sh .\n> >\n> > Using `--bisect-log` subcommand is a temporary measure to port shell\n> > function to C so as to use the existing test suite.\n> >\n> > Mentored-by: Lars Schneider \n> > Mentored-by: Christian Couder \n> > Mentored-by: Johannes Schindelin \n> > Signed-off-by: Pranit Bauva \n> > Signed-off-by: Tanushree Tumane \n> > Signed-off-by: Miriam Rubio \n> > ---\n> > builtin/bisect--helper.c | 22 +++++++++++++++++++++-\n> > git-bisect.sh | 7 +------\n> > 2 files changed, 22 insertions(+), 7 deletions(-)\n> >\n> > diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> > index 709eb713a3..a65244a0f5 100644\n> > --- a/builtin/bisect--helper.c\n> > +++ b/builtin/bisect--helper.c\n> > @@ -904,6 +904,18 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a\n> > return bisect_auto_next(terms, NULL);\n> > }\n> >\n> > +static enum bisect_error bisect_log(void)\n> > +{\n> > + int fd, status;\n> > + fd = open(git_path_bisect_log(), O_RDONLY);\n> > + if (fd < 0)\n> > + return BISECT_FAILED;\n> > +\n> > + status = copy_fd(fd, STDOUT_FILENO);\n> > + close(fd);\n> > + return status ? BISECT_FAILED : BISECT_OK;\n> > +}\n> > +\n>\n> In the shell version, when we are not bisecting it the `git bisect log`\n> command will `die` with the text \"We are not bisecting.\" which state to\n> the user that a bisect is not yet started. The shell version does that\n> by checking if the `$GIT_DIR/BISECT_LOG` file doesn't exists or it's\n> an empty file as the following code snippet copied from the shell\n> version that is remove by this patch:\n>\n> test -s \"$GIT_DIR/BISECT_LOG\" || die \"$(gettext \"We are not bisecting.\")\"\n>\n> This seems to be \"missing\" from the new C version implemented by this\n> patch and perhaps we should add it. I'm not sure whether this change was\n> intentional and I'm missing some context here of why we are dropping\n> the message, if that's the case I apologize in advance. But, IMHO\n> outputting the error message provides a better user experience as it\n> would be obvious that the user forgot to `git bisect start` instead of\n> silently failing.\n>\n> With that said, perhaps an obvious way of implementing is to use\n> `is_empty_or_missing_file()`, much like `bisect_replay()` does it in the\n> [2/7] patch on this series, and return the same error message from\n> the shell version:\n>\n> -- >8 --\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index a65244a0f5..ce11383125 100644\n> --- a/builtin/bisect--helper.c\n> +++ b/builtin/bisect--helper.c\n> @@ -907,7 +907,12 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a\n> static enum bisect_error bisect_log(void)\n> {\n> int fd, status;\n> - fd = open(git_path_bisect_log(), O_RDONLY);\n> + const char* filename = git_path_bisect_log();\n> +\n> + if (is_empty_or_missing_file(filename))\n> + return error(_(\"We are not bisecting.\"));\n> +\n> + fd = open(filename, O_RDONLY);\n> if (fd < 0)\n> return BISECT_FAILED;\n> -- >8 --\n>\n> Although I compiled and did small test on the above code snippet, don't\n> trust it blindly and perform your own test and judge whether this is the\n> best way to implement this shortcoming.\nOk, thank you.\nI am not the original author of this subcommand reimplementation and I\ndon't know if there is a reason\n for the difference with the error message. Maybe we can wait for some\nother reviewers opinion.\n>\n> >\n> > @@ -210,7 +205,7 @@ case \"$#\" in\n> > replay)\n> > bisect_replay \"$@\" ;;\n> > log)\n> > - bisect_log ;;\n> > + git bisect--helper --bisect-log || exit;;\n>\n> Style: just a minor change to add a space between `exit` and `;;`.\nNoted.\n>\n> --\n> Thanks\n> Rafael\nThank you for your suggestions.\nBest,\nMiriam\n"},{"id":"415283","messageId":"CAN7CjDAsRcLSZPjtJxcjyunSErer8DWKqnMcNNo2rxad7xA+Cw@mail.gmail.com","threadId":"55036","inReplyTo":"xmqq8s8gwjaq.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 0/7] Finish converting git bisect to C part 3","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-26T14:18:48Z","receivedAt":"2021-01-26T14:22:08Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Hi Junio,\n\nEl mar, 26 ene 2021 a las 2:58, Junio C Hamano () escribió:\n>\n> Miriam Rubio writes:\n>\n> > These patches correspond to a third 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 third part is formed by reimplementations of some `git bisect`\n> > subcommands and removal of some temporary subcommands.\n> >\n> > These patch series emails were generated from:\n> > https://gitlab.com/mirucam/git/commits/git-bisect-work-part3_v2.\n>\n> Unfortunately, with this series in the 'seen' branch, the CI jobs\n> seem to fail.\n\nWith the last patch series sent:\n\nhttps://public-inbox.org/git/20210125191710.45161-1-mirucam@gmail.com/\n(branch: https://gitlab.com/mirucam/git/commits/git-bisect-work-part3_v3)\n\nall tests pass (executed with: prove -j5 --shuffle t[0-9]*.sh):\n--------\nAll tests successful.\nFiles=916, Tests=22661, 1628 wallclock secs ( 7.93 usr 3.16 sys +\n1106.15 cusr 1387.58 csys = 2504.82 CPU)\nResult: PASS\n----------\n>\n> cf.\n> https://github.com/git/git/runs/1766239699?check_suite_focus=true#step:5:814\n\nI have seen that is \"t3415-rebase-autosquash.sh\" the test that fails in CI jobs.\n\nI have also executed it independently with no errors:\n./t3415-rebase-autosquash.sh\nok 1 - setup\nok 2 - auto fixup (option)\nok 3 - auto fixup (config)\nok 4 - auto squash (option)\nok 5 - auto squash (config)\nok 6 - misspelled auto squash\nok 7 - auto squash that matches 2 commits\nok 8 - auto squash that matches a commit after the squash\nok 9 - auto squash that matches a sha1\nok 10 - auto squash that matches longer sha1\nok 11 - use commit --fixup\nok 12 - use commit --squash\nok 13 - fixup! fixup!\nok 14 - fixup! squash!\nok 15 - squash! squash!\nok 16 - squash! fixup!\nok 17 - autosquash with custom inst format\nok 18 - autosquash with empty custom instructionFormat\nok 19 - autosquash with multiple empty patches\nok 20 - extra spaces after fixup!\nok 21 - wrapped original subject\nok 22 - abort last squash\nok 23 - fixup a fixup\n# passed all 23 test(s)\n1..23\n\nIf I am missing something or I should do any other test please tell me,\nthank you,\nMiriam.\n"},{"id":"415316","messageId":"xmqq8s8gwjaq.fsf@gitster.c.googlers.com","threadId":"55036","inReplyTo":"20210123154056.48234-1-mirucam@gmail.com","subject":"Re: [PATCH v3 0/7] Finish converting git bisect to C part 3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-26T01:58:37Z","receivedAt":"2021-01-26T20:46:27Z","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 third 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 third part is formed by reimplementations of some `git bisect` \n> subcommands and removal of some temporary subcommands.\n>\n> These patch series emails were generated from:\n> https://gitlab.com/mirucam/git/commits/git-bisect-work-part3_v2.\n\nUnfortunately, with this series in the 'seen' branch, the CI jobs\nseem to fail.\n\ncf.\nhttps://github.com/git/git/runs/1766239699?check_suite_focus=true#step:5:814\n"},{"id":"415341","messageId":"xmqqczxr35xs.fsf@gitster.c.googlers.com","threadId":"55036","inReplyTo":"CAN7CjDANLB85GHVVn32w_Y70bzvadtqdq2uRyc81j7nz+W05jA@mail.gmail.com","subject":"Re: [PATCH v3 1/7] bisect--helper: reimplement `bisect_log` shell function in C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-26T18:32:15Z","receivedAt":"2021-01-26T22:19:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Miriam R.\" writes:\n\n>> Although I compiled and did small test on the above code snippet, don't\n>> trust it blindly and perform your own test and judge whether this is the\n>> best way to implement this shortcoming.\n> Ok, thank you.\n> I am not the original author of this subcommand reimplementation\n> and I don't know if there is a reason for the difference with the\n> error message. Maybe we can wait for some other reviewers opinion.\n\nSorry I missed this thread.\n\nMy understanding is that this topic is an attempt to \"reimplement\"\nwhat is there in the scripted version, so any deviation of behaviour\nobserbable from outside, which is *not* justified, should by\ndefinition be treated as a bug.\n\nIf the original author did not explain why the behaviour difference\nexists and defend why the new behaviour in the reimplementation is\nbetter, and if you do not think of a good reason why the behaviour\nshould be different and the new behaviour is better, then let's\ntreat it in a bug and fix it.\n\nThanks.\n"},{"id":"415355","messageId":"xmqq4kj3342v.fsf@gitster.c.googlers.com","threadId":"55036","inReplyTo":"CAN7CjDAsRcLSZPjtJxcjyunSErer8DWKqnMcNNo2rxad7xA+Cw@mail.gmail.com","subject":"Re: [PATCH v3 0/7] Finish converting git bisect to C part 3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-26T19:12:24Z","receivedAt":"2021-01-26T22:19:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Miriam R.\" writes:\n\n>> cf.\n>> https://github.com/git/git/runs/1766239699?check_suite_focus=true#step:5:814\n>\n> I have seen that is \"t3415-rebase-autosquash.sh\" the test that fails in CI jobs.\n\nSorry, a false alarm. This I think was caused by another topic,\ncm/rebase-i, and not the bisect topic.\n\n"},{"id":"415423","messageId":"CAN7CjDCbbKLKVjCP5kWzAJXFBJs5G_MRST0rdTgvH0yTusRqeg@mail.gmail.com","threadId":"55036","inReplyTo":"xmqqczxr35xs.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 1/7] bisect--helper: reimplement `bisect_log` shell function in C","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-27T14:10:04Z","receivedAt":"2021-01-27T14:12:46Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Hi,\n\nEl mar, 26 ene 2021 a las 19:32, Junio C Hamano () escribió:\n>\n> \"Miriam R.\" writes:\n>\n> >> Although I compiled and did small test on the above code snippet, don't\n> >> trust it blindly and perform your own test and judge whether this is the\n> >> best way to implement this shortcoming.\n> > Ok, thank you.\n> > I am not the original author of this subcommand reimplementation\n> > and I don't know if there is a reason for the difference with the\n> > error message. Maybe we can wait for some other reviewers opinion.\n>\n> Sorry I missed this thread.\n>\n> My understanding is that this topic is an attempt to \"reimplement\"\n> what is there in the scripted version, so any deviation of behaviour\n> obserbable from outside, which is *not* justified, should by\n> definition be treated as a bug.\n>\n> If the original author did not explain why the behaviour difference\n> exists and defend why the new behaviour in the reimplementation is\n> better, and if you do not think of a good reason why the behaviour\n> should be different and the new behaviour is better, then let's\n> treat it in a bug and fix it.\n>\nOk, I will send another patch series adding this.\nThank you.\nMiriam.\n> Thanks.\n"},{"id":"415424","messageId":"CAN7CjDCMe6xFmguqPNkjbZeB0mMieVVuZ61m+Cre1geKs3_22w@mail.gmail.com","threadId":"55036","inReplyTo":"xmqq4kj3342v.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 0/7] Finish converting git bisect to C part 3","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2021-01-27T14:11:46Z","receivedAt":"2021-01-27T14:14:48Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"El mar, 26 ene 2021 a las 20:12, Junio C Hamano () escribió:\n>\n> \"Miriam R.\" writes:\n>\n> >> cf.\n> >> https://github.com/git/git/runs/1766239699?check_suite_focus=true#step:5:814\n> >\n> > I have seen that is \"t3415-rebase-autosquash.sh\" the test that fails in CI jobs.\n>\n> Sorry, a false alarm. This I think was caused by another topic,\n> cm/rebase-i, and not the bisect topic.\n>\nOk.\nBest,\nMiriam.\n"}]}