{"thread":{"id":"56305","subject":"[PATCH v4 0/6]Finish converting git bisect to C part 4","startedAt":"2021-08-17T08:15:57Z","lastAt":"2021-08-18T09:43:42Z","messageCount":20,"participants":["Miriam Rubio","Bagas Sanjaya","Christian Couder","Johannes Schindelin","Miriam R."],"isPatch":true,"patchVersion":4,"patchTotal":6},"messages":[{"id":"432910","messageId":"20210817081458.53136-1-mirucam@gmail.com","threadId":"56305","inReplyTo":null,"subject":"[PATCH v4 0/6]Finish converting git bisect to C part 4","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-08-17T08:14:52Z","receivedAt":"2021-08-17T08:15:57Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"These patches correspond to a fourth 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 fourth part is formed by reimplementations of some `git bisect` \nsubcommands, addition of tests and removal of some temporary subcommands.\n\nThese patch series emails were generated from:\nhttps://gitlab.com/mirucam/git/commits/git-bisect-work-part4-v4.1.\n\nI would like to thank Junio Hamano, Andrzej Hunt and Christian Couder \nfor reviewing this patch series.\n\n\nGeneral changes\n---------------\n* Rebase on master branch: 5d213e46bb (Git 2.33-rc2, 2021-08-11) \nto include latest updates in bisect-helper.c file.\n* Add three tests requested by reviewers in v3 patch series in \nt6030-bisect-porcelain.sh file.\n\nSpecific changes\n----------------\n\n\n[5/6] bisect--helper: reimplement `bisect_run` shell function in C\n* Content of the BISECT_RUN file is shown to the user.\n* Use strvec_push() instead of xstrdup().\n* Fix a bug on previous patch series regarding to \nBISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND (-10) return code.\n\n---\n\n\nMiriam Rubio (3):\n t6030-bisect-porcelain: add tests to control bisect run exit cases\n t6030-bisect-porcelain: add test for bisect visualize\n bisect--helper: retire `--bisect-next-check` subcommand\n\nPranit Bauva (2):\n run-command: make `exists_in_PATH()` non-static\n bisect--helper: reimplement `bisect_visualize()`shell function in C\n\nTanushree Tumane (1):\n bisect--helper: reimplement `bisect_run` shell function in C\n\n builtin/bisect--helper.c | 130 +++++++++++++++++++++++++++++++++---\n git-bisect.sh | 87 +-----------------------\n run-command.c | 2 +-\n run-command.h | 12 ++++\n t/t6030-bisect-porcelain.sh | 21 ++++++\n 5 files changed, 158 insertions(+), 94 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"432911","messageId":"20210817081458.53136-2-mirucam@gmail.com","threadId":"56305","inReplyTo":"20210817081458.53136-1-mirucam@gmail.com","subject":"[PATCH v4 1/6] t6030-bisect-porcelain: add tests to control bisect run exit cases","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-08-17T08:14:53Z","receivedAt":"2021-08-17T08:16:02Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"There is a gap on bisect run test coverage related with error exits.\nAdd two tests to control these error cases.\n\nSigned-off-by: Miriam Rubio \n---\n t/t6030-bisect-porcelain.sh | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex a1baf4e451..f41453cc97 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -962,4 +962,18 @@ test_expect_success 'bisect handles annotated tags' '\n \tgrep \"$bad is the first bad commit\" output\n '\n \n+test_expect_success 'bisect run fails with exit code equals or greater than 128' '\n+\twrite_script test_script.sh <<-\\EOF &&\n+\texit 128 >/dev/null\n+\tEOF\n+\ttest_must_fail git bisect run ./test_script.sh > my_bisect_log.txt\n+'\n+\n+test_expect_success 'bisect run fails with exit code smaller than 0' '\n+\twrite_script test_script.sh <<-\\EOF &&\n+\texit -1 >/dev/null\n+\tEOF\n+\ttest_must_fail git bisect run ./test_script.sh > my_bisect_log.txt\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"432912","messageId":"20210817081458.53136-3-mirucam@gmail.com","threadId":"56305","inReplyTo":"20210817081458.53136-1-mirucam@gmail.com","subject":"[PATCH v4 2/6] t6030-bisect-porcelain: add test for bisect visualize","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-08-17T08:14:54Z","receivedAt":"2021-08-17T08:16:03Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Add a test to control breakages in bisect visualize command.\n\nSigned-off-by: Miriam Rubio \n---\n t/t6030-bisect-porcelain.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex f41453cc97..99b7517400 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -976,4 +976,11 @@ test_expect_success 'bisect run fails with exit code smaller than 0' '\n \ttest_must_fail git bisect run ./test_script.sh > my_bisect_log.txt\n '\n \n+test_expect_success 'bisect visualize with a filename with dash and space' '\n+\techo \"My test line\" >> -hello\\ 2 &&\n+\tgit add -- -hello\\ 2 &&\n+\tgit commit --quiet -m \"Add test line\" -- -hello\\ 2 &&\n+\tgit bisect visualize -p -- -hello\\ 2 > my_bisect_log.txt\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"432913","messageId":"20210817081458.53136-4-mirucam@gmail.com","threadId":"56305","inReplyTo":"20210817081458.53136-1-mirucam@gmail.com","subject":"[PATCH v4 3/6] run-command: make `exists_in_PATH()` non-static","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-08-17T08:14:55Z","receivedAt":"2021-08-17T08:16:08Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nRemoves the `static` keyword from `exists_in_PATH()` function\nand declares the function in `run-command.h` file.\nThe function will be used in bisect_visualize() in a later\ncommit.\n\nMentored by: Christian Couder \nMentored by: Johannes Schindelin \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n run-command.c | 2 +-\n run-command.h | 12 ++++++++++++\n 2 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex f72e72cce7..390f46819f 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -210,7 +210,7 @@ static char *locate_in_PATH(const char *file)\n \treturn NULL;\n }\n \n-static int exists_in_PATH(const char *file)\n+int exists_in_PATH(const char *file)\n {\n \tchar *r = locate_in_PATH(file);\n \tint found = r != NULL;\ndiff --git a/run-command.h b/run-command.h\nindex af1296769f..54d74b706f 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -182,6 +182,18 @@ void child_process_clear(struct child_process *);\n \n int is_executable(const char *name);\n \n+/**\n+ * Search if a $PATH for a command exists. This emulates the path search that\n+ * execvp would perform, without actually executing the command so it\n+ * can be used before fork() to prepare to run a command using\n+ * execve() or after execvp() to diagnose why it failed.\n+ *\n+ * The caller should ensure that file contains no directory separators.\n+ *\n+ * Returns 1 if it is found in $PATH or 0 if the command could not be found.\n+ */\n+int exists_in_PATH(const char *file);\n+\n /**\n * Start a sub-process. Takes a pointer to a `struct child_process`\n * that specifies the details and returns pipe FDs (if requested).\n-- \n2.29.2\n\n"},{"id":"432914","messageId":"20210817081458.53136-5-mirucam@gmail.com","threadId":"56305","inReplyTo":"20210817081458.53136-1-mirucam@gmail.com","subject":"[PATCH v4 4/6] bisect--helper: reimplement `bisect_visualize()`shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-08-17T08:14:56Z","receivedAt":"2021-08-17T08:16:08Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nReimplement the `bisect_visualize()` shell function\nin C and also add `--bisect-visualize` subcommand to\n`git bisect--helper` to call it from git-bisect.sh.\n\nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 48 +++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh | 25 +--------------------\n 2 files changed, 48 insertions(+), 25 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex f184eaeac6..4258429c1c 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 (good|old) [...]\"),\n \tN_(\"git bisect--helper --bisect-replay \"),\n \tN_(\"git bisect--helper --bisect-skip [(|)...]\"),\n+\tN_(\"git bisect--helper --bisect-visualize\"),\n \tNULL\n };\n \n@@ -1036,6 +1037,44 @@ static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **ar\n \treturn res;\n }\n \n+static int bisect_visualize(struct bisect_terms *terms, const char **argv, int argc)\n+{\n+\tstruct strvec args = STRVEC_INIT;\n+\tint flags = RUN_COMMAND_NO_STDIN, res = 0;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tif (bisect_next_check(terms, NULL) != 0)\n+\t\treturn BISECT_FAILED;\n+\n+\tif (!argc) {\n+\t\tif ((getenv(\"DISPLAY\") || getenv(\"SESSIONNAME\") || getenv(\"MSYSTEM\") ||\n+\t\t getenv(\"SECURITYSESSIONID\")) && exists_in_PATH(\"gitk\"))\n+\t\t\tstrvec_push(&args, \"gitk\");\n+\t\telse {\n+\t\t\tstrvec_pushl(&args, \"log\", NULL);\n+\t\t\tflags |= RUN_GIT_CMD;\n+\t\t}\n+\t} else {\n+\t\tif (argv[0][0] == '-') {\n+\t\t\tstrvec_pushl(&args, \"log\", NULL);\n+\t\t\tflags |= RUN_GIT_CMD;\n+\t\t} else if (strcmp(argv[0], \"tig\") && !starts_with(argv[0], \"git\"))\n+\t\t\tflags |= RUN_GIT_CMD;\n+\n+\t\tstrvec_pushv(&args, argv);\n+\t}\n+\n+\tstrvec_pushl(&args, \"--bisect\", \"--\", NULL);\n+\n+\tstrbuf_read_file(&sb, git_path_bisect_names(), 0);\n+\tsq_dequote_to_strvec(sb.buf, &args);\n+\tstrbuf_release(&sb);\n+\n+\tres = run_command_v_opt(args.v, flags);\n+\tstrvec_clear(&args);\n+\treturn res;\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -1048,7 +1087,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_STATE,\n \t\tBISECT_LOG,\n \t\tBISECT_REPLAY,\n-\t\tBISECT_SKIP\n+\t\tBISECT_SKIP,\n+\t\tBISECT_VISUALIZE,\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -1070,6 +1110,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\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_CMDMODE(0, \"bisect-visualize\", &cmdmode,\n+\t\t\t N_(\"visualize the bisection\"), BISECT_VISUALIZE),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -1131,6 +1173,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tget_terms(&terms);\n \t\tres = bisect_skip(&terms, argv, argc);\n \t\tbreak;\n+\tcase BISECT_VISUALIZE:\n+\t\tget_terms(&terms);\n+\t\tres = bisect_visualize(&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 6a7afaea8d..95f7f3fb8c 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -39,29 +39,6 @@ _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n TERM_BAD=bad\n TERM_GOOD=good\n \n-bisect_visualize() {\n-\tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n-\n-\tif test $# = 0\n-\tthen\n-\t\tif test -n \"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}\" &&\n-\t\t\ttype gitk >/dev/null 2>&1\n-\t\tthen\n-\t\t\tset gitk\n-\t\telse\n-\t\t\tset git log\n-\t\tfi\n-\telse\n-\t\tcase \"$1\" in\n-\t\tgit*|tig) ;;\n-\t\t-*)\tset git log \"$@\" ;;\n-\t\t*)\tset git \"$@\" ;;\n-\t\tesac\n-\tfi\n-\n-\teval '\"$@\"' --bisect -- $(cat \"$GIT_DIR/BISECT_NAMES\")\n-}\n-\n bisect_run () {\n \tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n \n@@ -152,7 +129,7 @@ case \"$#\" in\n \t\t# Not sure we want \"next\" at the UI level anymore.\n \t\tgit bisect--helper --bisect-next \"$@\" || exit ;;\n \tvisualize|view)\n-\t\tbisect_visualize \"$@\" ;;\n+\t\tgit bisect--helper --bisect-visualize \"$@\" || exit;;\n \treset)\n \t\tgit bisect--helper --bisect-reset \"$@\" ;;\n \treplay)\n-- \n2.29.2\n\n"},{"id":"432915","messageId":"20210817081458.53136-6-mirucam@gmail.com","threadId":"56305","inReplyTo":"20210817081458.53136-1-mirucam@gmail.com","subject":"[PATCH v4 5/6] bisect--helper: reimplement `bisect_run` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-08-17T08:14:57Z","receivedAt":"2021-08-17T08:16:11Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Tanushree Tumane \n\nReimplement the `bisect_run()` shell function\nin C and also add `--bisect-run` subcommand to\n`git bisect--helper` to call it from git-bisect.sh.\n\nMentored-by: Christian Couder \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 75 ++++++++++++++++++++++++++++++++++++++++\n git-bisect.sh | 62 +--------------------------------\n 2 files changed, 76 insertions(+), 61 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 4258429c1c..852e0a30fb 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-replay \"),\n \tN_(\"git bisect--helper --bisect-skip [(|)...]\"),\n \tN_(\"git bisect--helper --bisect-visualize\"),\n+\tN_(\"git bisect--helper --bisect-run ...\"),\n \tNULL\n };\n \n@@ -1075,6 +1076,71 @@ static int bisect_visualize(struct bisect_terms *terms, const char **argv, int a\n \treturn res;\n }\n \n+static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)\n+{\n+\tint res = BISECT_OK;\n+\tstruct strbuf command = STRBUF_INIT;\n+\tstruct strvec args = STRVEC_INIT;\n+\tstruct strvec run_args = STRVEC_INIT;\n+\tint exit = 0;\n+\n+\tif (bisect_next_check(terms, NULL))\n+\t\treturn BISECT_FAILED;\n+\n+\tif (argc)\n+\t\tsq_quote_argv(&command, argv);\n+\telse\n+\t\treturn BISECT_FAILED;\n+\n+\tstrvec_push(&run_args, command.buf);\n+\n+\twhile (1) {\n+\t\tstrvec_clear(&args);\n+\t\texit = 1;\n+\n+\t\tprintf(_(\"running %s\"), command.buf);\n+\t\tres = run_command_v_opt(run_args.v, RUN_USING_SHELL);\n+\n+\t\tif (res < 0 || 128 <= res) {\n+\t\t\terror(_(\"bisect run failed: exit code %d from\"\n+\t\t\t\t\" '%s' is < 0 or >= 128\"), res, command.buf);\n+\t\t\tstrbuf_release(&command);\n+\t\t\treturn res;\n+\t\t}\n+\n+\t\tif (res == 125)\n+\t\t\tstrvec_push(&args, \"skip\");\n+\t\telse if (res > 0)\n+\t\t\tstrvec_push(&args, terms->term_bad);\n+\t\telse\n+\t\t\tstrvec_push(&args, terms->term_good);\n+\n+\t\tres = bisect_state(terms, args.v, args.nr);\n+\n+\t\tif (res == BISECT_ONLY_SKIPPED_LEFT)\n+\t\t\terror(_(\"bisect run cannot continue any more\"));\n+\t\telse if (res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) {\n+\t\t\tprintf(_(\"bisect run success\"));\n+\t\t\tres = BISECT_OK;\n+\t\t} else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {\n+\t\t\tprintf(_(\"bisect found first bad commit\"));\n+\t\t\tres = BISECT_OK;\n+\t\t} else if (res) {\n+\t\t\terror(_(\"bisect run failed:'git bisect--helper --bisect-state\"\n+\t\t\t\" %s' exited with error code %d\"), args.v[0], res);\n+\t\t} else {\n+\t\t\texit = 0;\n+\t\t}\n+\n+\t\tif (exit) {\n+\t\t\tstrbuf_release(&command);\n+\t\t\tstrvec_clear(&args);\n+\t\t\tstrvec_clear(&run_args);\n+\t\t\treturn res;\n+\t\t}\n+\t}\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -1089,6 +1155,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_REPLAY,\n \t\tBISECT_SKIP,\n \t\tBISECT_VISUALIZE,\n+\t\tBISECT_RUN,\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -1112,6 +1179,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"skip some commits for checkout\"), BISECT_SKIP),\n \t\tOPT_CMDMODE(0, \"bisect-visualize\", &cmdmode,\n \t\t\t N_(\"visualize the bisection\"), BISECT_VISUALIZE),\n+\t\tOPT_CMDMODE(0, \"bisect-run\", &cmdmode,\n+\t\t\t N_(\"use ... to automatically bisect.\"), BISECT_RUN),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -1177,6 +1246,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tget_terms(&terms);\n \t\tres = bisect_visualize(&terms, argv, argc);\n \t\tbreak;\n+\tcase BISECT_RUN:\n+\t\tif (!argc)\n+\t\t\treturn error(_(\"bisect run failed: no command provided.\"));\n+\t\tget_terms(&terms);\n+\t\tres = bisect_run(&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 95f7f3fb8c..e83d011e17 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -39,66 +39,6 @@ _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n TERM_BAD=bad\n TERM_GOOD=good\n \n-bisect_run () {\n-\tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n-\n-\ttest -n \"$*\" || die \"$(gettext \"bisect run failed: no command provided.\")\"\n-\n-\twhile true\n-\tdo\n-\t\tcommand=\"$@\"\n-\t\teval_gettextln \"running \\$command\"\n-\t\t\"$@\"\n-\t\tres=$?\n-\n-\t\t# Check for really bad run error.\n-\t\tif [ $res -lt 0 -o $res -ge 128 ]\n-\t\tthen\n-\t\t\teval_gettextln \"bisect run failed:\n-exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n-\t\t\texit $res\n-\t\tfi\n-\n-\t\t# Find current state depending on run success or failure.\n-\t\t# A special exit code of 125 means cannot test.\n-\t\tif [ $res -eq 125 ]\n-\t\tthen\n-\t\t\tstate='skip'\n-\t\telif [ $res -gt 0 ]\n-\t\tthen\n-\t\t\tstate=\"$TERM_BAD\"\n-\t\telse\n-\t\t\tstate=\"$TERM_GOOD\"\n-\t\tfi\n-\n-\t\tgit bisect--helper --bisect-state $state >\"$GIT_DIR/BISECT_RUN\"\n-\t\tres=$?\n-\n-\t\tcat \"$GIT_DIR/BISECT_RUN\"\n-\n-\t\tif sane_grep \"first $TERM_BAD commit could be any of\" \"$GIT_DIR/BISECT_RUN\" \\\n-\t\t\t>/dev/null\n-\t\tthen\n-\t\t\tgettextln \"bisect run cannot continue any more\" >&2\n-\t\t\texit $res\n-\t\tfi\n-\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-\t\t\texit $res\n-\t\tfi\n-\n-\t\tif sane_grep \"is the first $TERM_BAD commit\" \"$GIT_DIR/BISECT_RUN\" >/dev/null\n-\t\tthen\n-\t\t\tgettextln \"bisect run success\"\n-\t\t\texit 0;\n-\t\tfi\n-\n-\tdone\n-}\n-\n get_terms () {\n \tif test -s \"$GIT_DIR/BISECT_TERMS\"\n \tthen\n@@ -137,7 +77,7 @@ case \"$#\" in\n \tlog)\n \t\tgit bisect--helper --bisect-log || exit ;;\n \trun)\n-\t\tbisect_run \"$@\" ;;\n+\t\tgit bisect--helper --bisect-run \"$@\" || exit;;\n \tterms)\n \t\tgit bisect--helper --bisect-terms \"$@\" || exit;;\n \t*)\n-- \n2.29.2\n\n"},{"id":"432916","messageId":"20210817081458.53136-7-mirucam@gmail.com","threadId":"56305","inReplyTo":"20210817081458.53136-1-mirucam@gmail.com","subject":"[PATCH v4 6/6] bisect--helper: retire `--bisect-next-check` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-08-17T08:14:58Z","receivedAt":"2021-08-17T08:16:14Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"After reimplementation of `git bisect run` in C,\n`--bisect-next-check` subcommand is not needed anymore.\n\nLet's remove it from options list and code.\n\nMentored by: Christian Couder \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 7 -------\n 1 file changed, 7 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 852e0a30fb..d749747639 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-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 \t\t\t\t\t \" [--no-checkout] [--first-parent] [ [...]] [--] [...]\"),\n@@ -1200,12 +1199,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 BISECT_NEXT_CHECK:\n-\t\tif (argc != 2 && argc != 3)\n-\t\t\treturn error(_(\"--bisect-next-check requires 2 or 3 arguments\"));\n-\t\tset_terms(&terms, argv[1], argv[0]);\n-\t\tres = bisect_next_check(&terms, argc == 3 ? argv[2] : NULL);\n-\t\tbreak;\n \tcase BISECT_TERMS:\n \t\tif (argc > 1)\n \t\t\treturn error(_(\"--bisect-terms requires 0 or 1 argument\"));\n-- \n2.29.2\n\n"},{"id":"432927","messageId":"3dcf28af-1e75-0934-4663-3691b0efde1d@gmail.com","threadId":"56305","inReplyTo":"20210817081458.53136-2-mirucam@gmail.com","subject":"Re: [PATCH v4 1/6] t6030-bisect-porcelain: add tests to control bisect run exit cases","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-08-17T09:00:14Z","receivedAt":"2021-08-17T09:00:37Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 17/08/21 15.14, Miriam Rubio wrote:\n\n> +test_expect_success 'bisect run fails with exit code equals or greater than 128' '\n> +\twrite_script test_script.sh <<-\\EOF &&\n> +\texit 128 >/dev/null\n> +\tEOF\n> +\ttest_must_fail git bisect run ./test_script.sh > my_bisect_log.txt\n> +'\n\nThis only checks for exit code equals to 128. You should also check for \nexit code greater than 128, for example 255.\n\n> +\n> +test_expect_success 'bisect run fails with exit code smaller than 0' '\n> +\twrite_script test_script.sh <<-\\EOF &&\n> +\texit -1 >/dev/null\n> +\tEOF\n> +\ttest_must_fail git bisect run ./test_script.sh > my_bisect_log.txt\n> +'\n\nThis test looks OK, using -1 as representative of negative exit code. \nHowever, wording of test name can also be 'bisect run fails with \nnegative exit code'.\n\nThanks for reviewing.\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"432928","messageId":"cca9771b-035e-0aca-bcf2-115f7af364e4@gmail.com","threadId":"56305","inReplyTo":"20210817081458.53136-3-mirucam@gmail.com","subject":"Re: [PATCH v4 2/6] t6030-bisect-porcelain: add test for bisect visualize","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-08-17T09:03:14Z","receivedAt":"2021-08-17T09:03:34Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 17/08/21 15.14, Miriam Rubio wrote:\n> Add a test to control breakages in bisect visualize command.\n> \n> Signed-off-by: Miriam Rubio \n> ---\n> t/t6030-bisect-porcelain.sh | 7 +++++++\n> 1 file changed, 7 insertions(+)\n> \n> diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\n> index f41453cc97..99b7517400 100755\n> --- a/t/t6030-bisect-porcelain.sh\n> +++ b/t/t6030-bisect-porcelain.sh\n> @@ -976,4 +976,11 @@ test_expect_success 'bisect run fails with exit code smaller than 0' '\n> \ttest_must_fail git bisect run ./test_script.sh > my_bisect_log.txt\n> '\n> \n> +test_expect_success 'bisect visualize with a filename with dash and space' '\n> +\techo \"My test line\" >> -hello\\ 2 &&\n> +\tgit add -- -hello\\ 2 &&\n> +\tgit commit --quiet -m \"Add test line\" -- -hello\\ 2 &&\n> +\tgit bisect visualize -p -- -hello\\ 2 > my_bisect_log.txt\n> +'\n> +\n> test_done\n> \n\nSeems like you're testing with filename with dash and space. Does git \nbisect visualize have any problems handling such filenames?\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"432931","messageId":"CAP8UFD0RFm=CBsckhdBJKJ9QAw+hWT0yY84J4dNcaXegRp4u0w@mail.gmail.com","threadId":"56305","inReplyTo":"3dcf28af-1e75-0934-4663-3691b0efde1d@gmail.com","subject":"Re: [PATCH v4 1/6] t6030-bisect-porcelain: add tests to control bisect run exit cases","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2021-08-17T09:23:13Z","receivedAt":"2021-08-17T09:23:27Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Aug 17, 2021 at 11:03 AM Bagas Sanjaya wrote:\n>\n> On 17/08/21 15.14, Miriam Rubio wrote:\n>\n> > +test_expect_success 'bisect run fails with exit code equals or greater than 128' '\n> > + write_script test_script.sh <<-\\EOF &&\n> > + exit 128 >/dev/null\n> > + EOF\n> > + test_must_fail git bisect run ./test_script.sh > my_bisect_log.txt\n> > +'\n>\n> This only checks for exit code equals to 128. You should also check for\n> exit code greater than 128, for example 255.\n>\n> > +\n> > +test_expect_success 'bisect run fails with exit code smaller than 0' '\n> > + write_script test_script.sh <<-\\EOF &&\n> > + exit -1 >/dev/null\n> > + EOF\n> > + test_must_fail git bisect run ./test_script.sh > my_bisect_log.txt\n> > +'\n>\n> This test looks OK, using -1 as representative of negative exit code.\n> However, wording of test name can also be 'bisect run fails with\n> negative exit code'.\n\nActually I am not sure that it makes sense to test an exit code\nsmaller than 0, as POSIX exit codes are between 0 and 255 (included).\n\nFor example:\n\n$ bash -c 'exit -1'; echo $?\n255\n\n$ dash -c 'exit -1'; echo $?\ndash: 1: exit: Illegal number: -1\n2\n"},{"id":"432942","messageId":"nycvar.QRO.7.76.6.2108171329241.55@tvgsbejvaqbjf.bet","threadId":"56305","inReplyTo":"20210817081458.53136-5-mirucam@gmail.com","subject":"Re: [PATCH v4 4/6] bisect--helper: reimplement `bisect_visualize()`shell function in C","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-08-17T11:30:42Z","receivedAt":"2021-08-17T11:30:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Miriam,\n\nthis looks good!\n\nJust one suggestion (but I won't insist on it):\n\nOn Tue, 17 Aug 2021, Miriam Rubio wrote:\n\n> @@ -1036,6 +1037,44 @@ static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **ar\n> \treturn res;\n> }\n>\n> +static int bisect_visualize(struct bisect_terms *terms, const char **argv, int argc)\n> +{\n> +\tstruct strvec args = STRVEC_INIT;\n> +\tint flags = RUN_COMMAND_NO_STDIN, res = 0;\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\n> +\tif (bisect_next_check(terms, NULL) != 0)\n> +\t\treturn BISECT_FAILED;\n> +\n> +\tif (!argc) {\n> +\t\tif ((getenv(\"DISPLAY\") || getenv(\"SESSIONNAME\") || getenv(\"MSYSTEM\") ||\n> +\t\t getenv(\"SECURITYSESSIONID\")) && exists_in_PATH(\"gitk\"))\n> +\t\t\tstrvec_push(&args, \"gitk\");\n> +\t\telse {\n> +\t\t\tstrvec_pushl(&args, \"log\", NULL);\n\nThis could be written more concisely as `strvec_push(&args, \"log\")`.\n\n> +\t\t\tflags |= RUN_GIT_CMD;\n> +\t\t}\n> +\t} else {\n> +\t\tif (argv[0][0] == '-') {\n> +\t\t\tstrvec_pushl(&args, \"log\", NULL);\n\nSame here.\n\nOtherwise, it looks good to me!\n\nThank you,\nDscho\n\n> +\t\t\tflags |= RUN_GIT_CMD;\n> +\t\t} else if (strcmp(argv[0], \"tig\") && !starts_with(argv[0], \"git\"))\n> +\t\t\tflags |= RUN_GIT_CMD;\n> +\n> +\t\tstrvec_pushv(&args, argv);\n> +\t}\n> +\n> +\tstrvec_pushl(&args, \"--bisect\", \"--\", NULL);\n> +\n> +\tstrbuf_read_file(&sb, git_path_bisect_names(), 0);\n> +\tsq_dequote_to_strvec(sb.buf, &args);\n> +\tstrbuf_release(&sb);\n> +\n> +\tres = run_command_v_opt(args.v, flags);\n> +\tstrvec_clear(&args);\n> +\treturn res;\n> +}\n> +\n> int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> {\n> \tenum {\n> @@ -1048,7 +1087,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> \t\tBISECT_STATE,\n> \t\tBISECT_LOG,\n> \t\tBISECT_REPLAY,\n> -\t\tBISECT_SKIP\n> +\t\tBISECT_SKIP,\n> +\t\tBISECT_VISUALIZE,\n> \t} cmdmode = 0;\n> \tint res = 0, nolog = 0;\n> \tstruct option options[] = {\n> @@ -1070,6 +1110,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\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_CMDMODE(0, \"bisect-visualize\", &cmdmode,\n> +\t\t\t N_(\"visualize the bisection\"), BISECT_VISUALIZE),\n> \t\tOPT_BOOL(0, \"no-log\", &nolog,\n> \t\t\t N_(\"no log for BISECT_WRITE\")),\n> \t\tOPT_END()\n> @@ -1131,6 +1173,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> \t\tget_terms(&terms);\n> \t\tres = bisect_skip(&terms, argv, argc);\n> \t\tbreak;\n> +\tcase BISECT_VISUALIZE:\n> +\t\tget_terms(&terms);\n> +\t\tres = bisect_visualize(&terms, argv, argc);\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 6a7afaea8d..95f7f3fb8c 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -39,29 +39,6 @@ _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n> TERM_BAD=bad\n> TERM_GOOD=good\n>\n> -bisect_visualize() {\n> -\tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n> -\n> -\tif test $# = 0\n> -\tthen\n> -\t\tif test -n \"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}\" &&\n> -\t\t\ttype gitk >/dev/null 2>&1\n> -\t\tthen\n> -\t\t\tset gitk\n> -\t\telse\n> -\t\t\tset git log\n> -\t\tfi\n> -\telse\n> -\t\tcase \"$1\" in\n> -\t\tgit*|tig) ;;\n> -\t\t-*)\tset git log \"$@\" ;;\n> -\t\t*)\tset git \"$@\" ;;\n> -\t\tesac\n> -\tfi\n> -\n> -\teval '\"$@\"' --bisect -- $(cat \"$GIT_DIR/BISECT_NAMES\")\n> -}\n> -\n> bisect_run () {\n> \tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n>\n> @@ -152,7 +129,7 @@ case \"$#\" in\n> \t\t# Not sure we want \"next\" at the UI level anymore.\n> \t\tgit bisect--helper --bisect-next \"$@\" || exit ;;\n> \tvisualize|view)\n> -\t\tbisect_visualize \"$@\" ;;\n> +\t\tgit bisect--helper --bisect-visualize \"$@\" || exit;;\n> \treset)\n> \t\tgit bisect--helper --bisect-reset \"$@\" ;;\n> \treplay)\n> --\n> 2.29.2\n>\n>\n"},{"id":"432943","messageId":"nycvar.QRO.7.76.6.2108171332370.55@tvgsbejvaqbjf.bet","threadId":"56305","inReplyTo":"20210817081458.53136-6-mirucam@gmail.com","subject":"Re: [PATCH v4 5/6] bisect--helper: reimplement `bisect_run` shell function in C","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-08-17T11:42:06Z","receivedAt":"2021-08-17T11:42:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Miriam,\n\nOn Tue, 17 Aug 2021, Miriam Rubio wrote:\n\n> From: Tanushree Tumane \n>\n> Reimplement the `bisect_run()` shell function\n> in C and also add `--bisect-run` subcommand to\n> `git bisect--helper` to call it from git-bisect.sh.\n>\n> Mentored-by: Christian Couder \n> Signed-off-by: Tanushree Tumane \n> Signed-off-by: Miriam Rubio \n> ---\n> builtin/bisect--helper.c | 75 ++++++++++++++++++++++++++++++++++++++++\n> git-bisect.sh | 62 +--------------------------------\n> 2 files changed, 76 insertions(+), 61 deletions(-)\n>\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index 4258429c1c..852e0a30fb 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-replay \"),\n> \tN_(\"git bisect--helper --bisect-skip [(|)...]\"),\n> \tN_(\"git bisect--helper --bisect-visualize\"),\n> +\tN_(\"git bisect--helper --bisect-run ...\"),\n> \tNULL\n> };\n>\n> @@ -1075,6 +1076,71 @@ static int bisect_visualize(struct bisect_terms *terms, const char **argv, int a\n> \treturn res;\n> }\n>\n> +static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)\n> +{\n> +\tint res = BISECT_OK;\n> +\tstruct strbuf command = STRBUF_INIT;\n> +\tstruct strvec args = STRVEC_INIT;\n> +\tstruct strvec run_args = STRVEC_INIT;\n> +\tint exit = 0;\n> +\n> +\tif (bisect_next_check(terms, NULL))\n> +\t\treturn BISECT_FAILED;\n> +\n> +\tif (argc)\n> +\t\tsq_quote_argv(&command, argv);\n> +\telse\n> +\t\treturn BISECT_FAILED;\n\nDo we want to say something helpful here, e.g. _(\"bisect run failed: no\ncommand provided.\")?\n\n> +\n> +\tstrvec_push(&run_args, command.buf);\n> +\n> +\twhile (1) {\n> +\t\tstrvec_clear(&args);\n> +\t\texit = 1;\n> +\n> +\t\tprintf(_(\"running %s\"), command.buf);\n> +\t\tres = run_command_v_opt(run_args.v, RUN_USING_SHELL);\n> +\n> +\t\tif (res < 0 || 128 <= res) {\n> +\t\t\terror(_(\"bisect run failed: exit code %d from\"\n> +\t\t\t\t\" '%s' is < 0 or >= 128\"), res, command.buf);\n> +\t\t\tstrbuf_release(&command);\n> +\t\t\treturn res;\n> +\t\t}\n> +\n> +\t\tif (res == 125)\n> +\t\t\tstrvec_push(&args, \"skip\");\n> +\t\telse if (res > 0)\n> +\t\t\tstrvec_push(&args, terms->term_bad);\n> +\t\telse\n> +\t\t\tstrvec_push(&args, terms->term_good);\n> +\n> +\t\tres = bisect_state(terms, args.v, args.nr);\n\nSince `args.nr` will always be 1, it would probably be better to use\nsomething like this:\n\n\t\tconst char *new_state;\n\n\t\t[...]\n\t\tif (res == 125)\n\t\t\tnew_state = \"skip\";\n\t\telse\n\t\t\tnew_state = res > 0 ?\n\t\t\t\tterms->term_bad : terms->term_good;\n\n\t\tres = bisect_state(terms, &new_state, 1);\n\nAlso: I think at this stage, an equivalent to `cat \"$GIT_DIR/BISECT_RUN\"`\nis missing.\n\n> +\n> +\t\tif (res == BISECT_ONLY_SKIPPED_LEFT)\n> +\t\t\terror(_(\"bisect run cannot continue any more\"));\n> +\t\telse if (res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) {\n> +\t\t\tprintf(_(\"bisect run success\"));\n> +\t\t\tres = BISECT_OK;\n> +\t\t} else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {\n> +\t\t\tprintf(_(\"bisect found first bad commit\"));\n> +\t\t\tres = BISECT_OK;\n> +\t\t} else if (res) {\n> +\t\t\terror(_(\"bisect run failed:'git bisect--helper --bisect-state\"\n> +\t\t\t\" %s' exited with error code %d\"), args.v[0], res);\n> +\t\t} else {\n> +\t\t\texit = 0;\n\nSince the only purpose of `exit` seems to be that the loop should continue\nif `exit` is set to 0, and it is only set here, how about doing away with\nthe variable altogether and writing `continue;` instead of `exit = 0;`?\nThen the conditional block below does not need to be conditional.\n\nOther than that: well done!\n\nCiao,\nDscho\n\n> +\t\t}\n> +\n> +\t\tif (exit) {\n> +\t\t\tstrbuf_release(&command);\n> +\t\t\tstrvec_clear(&args);\n> +\t\t\tstrvec_clear(&run_args);\n> +\t\t\treturn res;\n> +\t\t}\n> +\t}\n> +}\n> +\n> int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> {\n> \tenum {\n> @@ -1089,6 +1155,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> \t\tBISECT_REPLAY,\n> \t\tBISECT_SKIP,\n> \t\tBISECT_VISUALIZE,\n> +\t\tBISECT_RUN,\n> \t} cmdmode = 0;\n> \tint res = 0, nolog = 0;\n> \tstruct option options[] = {\n> @@ -1112,6 +1179,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> \t\t\t N_(\"skip some commits for checkout\"), BISECT_SKIP),\n> \t\tOPT_CMDMODE(0, \"bisect-visualize\", &cmdmode,\n> \t\t\t N_(\"visualize the bisection\"), BISECT_VISUALIZE),\n> +\t\tOPT_CMDMODE(0, \"bisect-run\", &cmdmode,\n> +\t\t\t N_(\"use ... to automatically bisect.\"), BISECT_RUN),\n> \t\tOPT_BOOL(0, \"no-log\", &nolog,\n> \t\t\t N_(\"no log for BISECT_WRITE\")),\n> \t\tOPT_END()\n> @@ -1177,6 +1246,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> \t\tget_terms(&terms);\n> \t\tres = bisect_visualize(&terms, argv, argc);\n> \t\tbreak;\n> +\tcase BISECT_RUN:\n> +\t\tif (!argc)\n> +\t\t\treturn error(_(\"bisect run failed: no command provided.\"));\n> +\t\tget_terms(&terms);\n> +\t\tres = bisect_run(&terms, argv, argc);\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 95f7f3fb8c..e83d011e17 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -39,66 +39,6 @@ _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n> TERM_BAD=bad\n> TERM_GOOD=good\n>\n> -bisect_run () {\n> -\tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n> -\n> -\ttest -n \"$*\" || die \"$(gettext \"bisect run failed: no command provided.\")\"\n> -\n> -\twhile true\n> -\tdo\n> -\t\tcommand=\"$@\"\n> -\t\teval_gettextln \"running \\$command\"\n> -\t\t\"$@\"\n> -\t\tres=$?\n> -\n> -\t\t# Check for really bad run error.\n> -\t\tif [ $res -lt 0 -o $res -ge 128 ]\n> -\t\tthen\n> -\t\t\teval_gettextln \"bisect run failed:\n> -exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n> -\t\t\texit $res\n> -\t\tfi\n> -\n> -\t\t# Find current state depending on run success or failure.\n> -\t\t# A special exit code of 125 means cannot test.\n> -\t\tif [ $res -eq 125 ]\n> -\t\tthen\n> -\t\t\tstate='skip'\n> -\t\telif [ $res -gt 0 ]\n> -\t\tthen\n> -\t\t\tstate=\"$TERM_BAD\"\n> -\t\telse\n> -\t\t\tstate=\"$TERM_GOOD\"\n> -\t\tfi\n> -\n> -\t\tgit bisect--helper --bisect-state $state >\"$GIT_DIR/BISECT_RUN\"\n> -\t\tres=$?\n> -\n> -\t\tcat \"$GIT_DIR/BISECT_RUN\"\n> -\n> -\t\tif sane_grep \"first $TERM_BAD commit could be any of\" \"$GIT_DIR/BISECT_RUN\" \\\n> -\t\t\t>/dev/null\n> -\t\tthen\n> -\t\t\tgettextln \"bisect run cannot continue any more\" >&2\n> -\t\t\texit $res\n> -\t\tfi\n> -\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> -\t\t\texit $res\n> -\t\tfi\n> -\n> -\t\tif sane_grep \"is the first $TERM_BAD commit\" \"$GIT_DIR/BISECT_RUN\" >/dev/null\n> -\t\tthen\n> -\t\t\tgettextln \"bisect run success\"\n> -\t\t\texit 0;\n> -\t\tfi\n> -\n> -\tdone\n> -}\n> -\n> get_terms () {\n> \tif test -s \"$GIT_DIR/BISECT_TERMS\"\n> \tthen\n> @@ -137,7 +77,7 @@ case \"$#\" in\n> \tlog)\n> \t\tgit bisect--helper --bisect-log || exit ;;\n> \trun)\n> -\t\tbisect_run \"$@\" ;;\n> +\t\tgit bisect--helper --bisect-run \"$@\" || exit;;\n> \tterms)\n> \t\tgit bisect--helper --bisect-terms \"$@\" || exit;;\n> \t*)\n> --\n> 2.29.2\n>\n>\n"},{"id":"432945","messageId":"nycvar.QRO.7.76.6.2108171357170.55@tvgsbejvaqbjf.bet","threadId":"56305","inReplyTo":"20210817081458.53136-7-mirucam@gmail.com","subject":"Re: [PATCH v4 6/6] bisect--helper: retire `--bisect-next-check` subcommand","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-08-17T11:57:51Z","receivedAt":"2021-08-17T11:57:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Miriam,\n\nOn Tue, 17 Aug 2021, Miriam Rubio wrote:\n\n> After reimplementation of `git bisect run` in C,\n> `--bisect-next-check` subcommand is not needed anymore.\n>\n> Let's remove it from options list and code.\n>\n> Mentored by: Christian Couder \n> Signed-off-by: Miriam Rubio \n> ---\n> builtin/bisect--helper.c | 7 -------\n> 1 file changed, 7 deletions(-)\n\nExciting! This is inching closer and closer to a fully-built-in `git\nbisect`.\n\nThank you so much!\nDscho\n\n>\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index 852e0a30fb..d749747639 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-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> \t\t\t\t\t \" [--no-checkout] [--first-parent] [ [...]] [--] [...]\"),\n> @@ -1200,12 +1199,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 BISECT_NEXT_CHECK:\n> -\t\tif (argc != 2 && argc != 3)\n> -\t\t\treturn error(_(\"--bisect-next-check requires 2 or 3 arguments\"));\n> -\t\tset_terms(&terms, argv[1], argv[0]);\n> -\t\tres = bisect_next_check(&terms, argc == 3 ? argv[2] : NULL);\n> -\t\tbreak;\n> \tcase BISECT_TERMS:\n> \t\tif (argc > 1)\n> \t\t\treturn error(_(\"--bisect-terms requires 0 or 1 argument\"));\n> --\n> 2.29.2\n>\n>\n"},{"id":"433024","messageId":"CAN7CjDD8n-P0-UD4e1w0VPLc+CRnho47KcA_7xRs96Hu8CQeTA@mail.gmail.com","threadId":"56305","inReplyTo":"cca9771b-035e-0aca-bcf2-115f7af364e4@gmail.com","subject":"Re: [PATCH v4 2/6] t6030-bisect-porcelain: add test for bisect visualize","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2021-08-17T20:17:33Z","receivedAt":"2021-08-17T20:21:31Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Hi Bagas,\n\nEl mar, 17 ago 2021 a las 11:03, Bagas Sanjaya\n() escribió:\n>\n> On 17/08/21 15.14, Miriam Rubio wrote:\n> > Add a test to control breakages in bisect visualize command.\n> >\n> > Signed-off-by: Miriam Rubio \n> > ---\n> > t/t6030-bisect-porcelain.sh | 7 +++++++\n> > 1 file changed, 7 insertions(+)\n> >\n> > diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\n> > index f41453cc97..99b7517400 100755\n> > --- a/t/t6030-bisect-porcelain.sh\n> > +++ b/t/t6030-bisect-porcelain.sh\n> > @@ -976,4 +976,11 @@ test_expect_success 'bisect run fails with exit code smaller than 0' '\n> > test_must_fail git bisect run ./test_script.sh > my_bisect_log.txt\n> > '\n> >\n> > +test_expect_success 'bisect visualize with a filename with dash and space' '\n> > + echo \"My test line\" >> -hello\\ 2 &&\n> > + git add -- -hello\\ 2 &&\n> > + git commit --quiet -m \"Add test line\" -- -hello\\ 2 &&\n> > + git bisect visualize -p -- -hello\\ 2 > my_bisect_log.txt\n> > +'\n> > +\n> > test_done\n> >\n>\n> Seems like you're testing with filename with dash and space. Does git\n> bisect visualize have any problems handling such filenames?\n>\nIt was a suggestion of a reviewer in the previous version to detect\npossible breakages:\nhttps://lore.kernel.org/git/xmqq35vwh8qk.fsf@gitster.g/\n\nThanks for reviewing,\nMiriam\n\n> --\n> An old man doll... just what I always wanted! - Clara\n"},{"id":"433025","messageId":"CAN7CjDBJMLn=MkJHnFFBmTsMR0dy65+D1UMWObyHV8=qoNfOHg@mail.gmail.com","threadId":"56305","inReplyTo":"CAP8UFD0RFm=CBsckhdBJKJ9QAw+hWT0yY84J4dNcaXegRp4u0w@mail.gmail.com","subject":"Re: [PATCH v4 1/6] t6030-bisect-porcelain: add tests to control bisect run exit cases","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2021-08-17T20:19:20Z","receivedAt":"2021-08-17T20:22:01Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Hi,\n\nEl mar, 17 ago 2021 a las 11:23, Christian Couder\n() escribió:\n>\n> On Tue, Aug 17, 2021 at 11:03 AM Bagas Sanjaya wrote:\n> >\n> > On 17/08/21 15.14, Miriam Rubio wrote:\n> >\n> > > +test_expect_success 'bisect run fails with exit code equals or greater than 128' '\n> > > + write_script test_script.sh <<-\\EOF &&\n> > > + exit 128 >/dev/null\n> > > + EOF\n> > > + test_must_fail git bisect run ./test_script.sh > my_bisect_log.txt\n> > > +'\n> >\n> > This only checks for exit code equals to 128. You should also check for\n> > exit code greater than 128, for example 255.\n> >\nNoted.\nThank you for reviewing, Bagas.\n> > > +\n> > > +test_expect_success 'bisect run fails with exit code smaller than 0' '\n> > > + write_script test_script.sh <<-\\EOF &&\n> > > + exit -1 >/dev/null\n> > > + EOF\n> > > + test_must_fail git bisect run ./test_script.sh > my_bisect_log.txt\n> > > +'\n> >\n> > This test looks OK, using -1 as representative of negative exit code.\n> > However, wording of test name can also be 'bisect run fails with\n> > negative exit code'.\n>\n> Actually I am not sure that it makes sense to test an exit code\n> smaller than 0, as POSIX exit codes are between 0 and 255 (included).\n>\n> For example:\n>\n> $ bash -c 'exit -1'; echo $?\n> 255\n>\n> $ dash -c 'exit -1'; echo $?\n> dash: 1: exit: Illegal number: -1\n> 2\nOk, I will remove this test. No problem.\nThanks, Christian.\n"},{"id":"433026","messageId":"CAN7CjDC-ND-NtEc31+M=GuW+XczM7R6kAmTq0MfxgupU5Msr=A@mail.gmail.com","threadId":"56305","inReplyTo":"nycvar.QRO.7.76.6.2108171329241.55@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v4 4/6] bisect--helper: reimplement `bisect_visualize()`shell function in C","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2021-08-17T20:19:58Z","receivedAt":"2021-08-17T20:22:03Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Hi Johannes,\n\nEl mar, 17 ago 2021 a las 13:30, Johannes Schindelin\n() escribió:\n>\n> Hi Miriam,\n>\n> this looks good!\n>\n> Just one suggestion (but I won't insist on it):\n>\n> On Tue, 17 Aug 2021, Miriam Rubio wrote:\n>\n> > @@ -1036,6 +1037,44 @@ static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **ar\n> > return res;\n> > }\n> >\n> > +static int bisect_visualize(struct bisect_terms *terms, const char **argv, int argc)\n> > +{\n> > + struct strvec args = STRVEC_INIT;\n> > + int flags = RUN_COMMAND_NO_STDIN, res = 0;\n> > + struct strbuf sb = STRBUF_INIT;\n> > +\n> > + if (bisect_next_check(terms, NULL) != 0)\n> > + return BISECT_FAILED;\n> > +\n> > + if (!argc) {\n> > + if ((getenv(\"DISPLAY\") || getenv(\"SESSIONNAME\") || getenv(\"MSYSTEM\") ||\n> > + getenv(\"SECURITYSESSIONID\")) && exists_in_PATH(\"gitk\"))\n> > + strvec_push(&args, \"gitk\");\n> > + else {\n> > + strvec_pushl(&args, \"log\", NULL);\n>\n> This could be written more concisely as `strvec_push(&args, \"log\")`.\n>\n> > + flags |= RUN_GIT_CMD;\n> > + }\n> > + } else {\n> > + if (argv[0][0] == '-') {\n> > + strvec_pushl(&args, \"log\", NULL);\n>\n> Same here.\nSure, I will change it in both cases.\nThank you for reviewing,\nMiriam.\n\n>\n> Otherwise, it looks good to me!\n>\n> Thank you,\n> Dscho\n>\n> > + flags |= RUN_GIT_CMD;\n> > + } else if (strcmp(argv[0], \"tig\") && !starts_with(argv[0], \"git\"))\n> > + flags |= RUN_GIT_CMD;\n> > +\n> > + strvec_pushv(&args, argv);\n> > + }\n> > +\n> > + strvec_pushl(&args, \"--bisect\", \"--\", NULL);\n> > +\n> > + strbuf_read_file(&sb, git_path_bisect_names(), 0);\n> > + sq_dequote_to_strvec(sb.buf, &args);\n> > + strbuf_release(&sb);\n> > +\n> > + res = run_command_v_opt(args.v, flags);\n> > + strvec_clear(&args);\n> > + return res;\n> > +}\n> > +\n> > int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> > {\n> > enum {\n> > @@ -1048,7 +1087,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> > BISECT_STATE,\n> > BISECT_LOG,\n> > BISECT_REPLAY,\n> > - BISECT_SKIP\n> > + BISECT_SKIP,\n> > + BISECT_VISUALIZE,\n> > } cmdmode = 0;\n> > int res = 0, nolog = 0;\n> > struct option options[] = {\n> > @@ -1070,6 +1110,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> > N_(\"replay the bisection process from the given file\"), BISECT_REPLAY),\n> > OPT_CMDMODE(0, \"bisect-skip\", &cmdmode,\n> > N_(\"skip some commits for checkout\"), BISECT_SKIP),\n> > + OPT_CMDMODE(0, \"bisect-visualize\", &cmdmode,\n> > + N_(\"visualize the bisection\"), BISECT_VISUALIZE),\n> > OPT_BOOL(0, \"no-log\", &nolog,\n> > N_(\"no log for BISECT_WRITE\")),\n> > OPT_END()\n> > @@ -1131,6 +1173,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> > get_terms(&terms);\n> > res = bisect_skip(&terms, argv, argc);\n> > break;\n> > + case BISECT_VISUALIZE:\n> > + get_terms(&terms);\n> > + res = bisect_visualize(&terms, argv, argc);\n> > + break;\n> > default:\n> > BUG(\"unknown subcommand %d\", cmdmode);\n> > }\n> > diff --git a/git-bisect.sh b/git-bisect.sh\n> > index 6a7afaea8d..95f7f3fb8c 100755\n> > --- a/git-bisect.sh\n> > +++ b/git-bisect.sh\n> > @@ -39,29 +39,6 @@ _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n> > TERM_BAD=bad\n> > TERM_GOOD=good\n> >\n> > -bisect_visualize() {\n> > - git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n> > -\n> > - if test $# = 0\n> > - then\n> > - if test -n \"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}\" &&\n> > - type gitk >/dev/null 2>&1\n> > - then\n> > - set gitk\n> > - else\n> > - set git log\n> > - fi\n> > - else\n> > - case \"$1\" in\n> > - git*|tig) ;;\n> > - -*) set git log \"$@\" ;;\n> > - *) set git \"$@\" ;;\n> > - esac\n> > - fi\n> > -\n> > - eval '\"$@\"' --bisect -- $(cat \"$GIT_DIR/BISECT_NAMES\")\n> > -}\n> > -\n> > bisect_run () {\n> > git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n> >\n> > @@ -152,7 +129,7 @@ case \"$#\" in\n> > # Not sure we want \"next\" at the UI level anymore.\n> > git bisect--helper --bisect-next \"$@\" || exit ;;\n> > visualize|view)\n> > - bisect_visualize \"$@\" ;;\n> > + git bisect--helper --bisect-visualize \"$@\" || exit;;\n> > reset)\n> > git bisect--helper --bisect-reset \"$@\" ;;\n> > replay)\n> > --\n> > 2.29.2\n> >\n> >\n"},{"id":"433027","messageId":"CAN7CjDDEv6vGPKZo3sxz8bgfN2Nzqh0HChR-tGrjDGbkhKZo=A@mail.gmail.com","threadId":"56305","inReplyTo":"nycvar.QRO.7.76.6.2108171332370.55@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v4 5/6] bisect--helper: reimplement `bisect_run` shell function in C","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2021-08-17T20:22:34Z","receivedAt":"2021-08-17T20:22:58Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Hi Johannes,\n\nEl mar, 17 ago 2021 a las 13:42, Johannes Schindelin\n() escribió:\n>\n> Hi Miriam,\n>\n> On Tue, 17 Aug 2021, Miriam Rubio wrote:\n>\n> > From: Tanushree Tumane \n> >\n> > Reimplement the `bisect_run()` shell function\n> > in C and also add `--bisect-run` subcommand to\n> > `git bisect--helper` to call it from git-bisect.sh.\n> >\n> > Mentored-by: Christian Couder \n> > Signed-off-by: Tanushree Tumane \n> > Signed-off-by: Miriam Rubio \n> > ---\n> > builtin/bisect--helper.c | 75 ++++++++++++++++++++++++++++++++++++++++\n> > git-bisect.sh | 62 +--------------------------------\n> > 2 files changed, 76 insertions(+), 61 deletions(-)\n> >\n> > diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> > index 4258429c1c..852e0a30fb 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> > N_(\"git bisect--helper --bisect-replay \"),\n> > N_(\"git bisect--helper --bisect-skip [(|)...]\"),\n> > N_(\"git bisect--helper --bisect-visualize\"),\n> > + N_(\"git bisect--helper --bisect-run ...\"),\n> > NULL\n> > };\n> >\n> > @@ -1075,6 +1076,71 @@ static int bisect_visualize(struct bisect_terms *terms, const char **argv, int a\n> > return res;\n> > }\n> >\n> > +static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)\n> > +{\n> > + int res = BISECT_OK;\n> > + struct strbuf command = STRBUF_INIT;\n> > + struct strvec args = STRVEC_INIT;\n> > + struct strvec run_args = STRVEC_INIT;\n> > + int exit = 0;\n> > +\n> > + if (bisect_next_check(terms, NULL))\n> > + return BISECT_FAILED;\n> > +\n> > + if (argc)\n> > + sq_quote_argv(&command, argv);\n> > + else\n> > + return BISECT_FAILED;\n>\n> Do we want to say something helpful here, e.g. _(\"bisect run failed: no\n> command provided.\")?\n>\nOk, noted\n> > +\n> > + strvec_push(&run_args, command.buf);\n> > +\n> > + while (1) {\n> > + strvec_clear(&args);\n> > + exit = 1;\n> > +\n> > + printf(_(\"running %s\"), command.buf);\n> > + res = run_command_v_opt(run_args.v, RUN_USING_SHELL);\n> > +\n> > + if (res < 0 || 128 <= res) {\n> > + error(_(\"bisect run failed: exit code %d from\"\n> > + \" '%s' is < 0 or >= 128\"), res, command.buf);\n> > + strbuf_release(&command);\n> > + return res;\n> > + }\n> > +\n> > + if (res == 125)\n> > + strvec_push(&args, \"skip\");\n> > + else if (res > 0)\n> > + strvec_push(&args, terms->term_bad);\n> > + else\n> > + strvec_push(&args, terms->term_good);\n> > +\n> > + res = bisect_state(terms, args.v, args.nr);\n>\n> Since `args.nr` will always be 1, it would probably be better to use\n> something like this:\n>\n> const char *new_state;\n>\n> [...]\n> if (res == 125)\n> new_state = \"skip\";\n> else\n> new_state = res > 0 ?\n> terms->term_bad : terms->term_good;\n>\n> res = bisect_state(terms, &new_state, 1);\n>\nYes, indeed. I will change it.\n> Also: I think at this stage, an equivalent to `cat \"$GIT_DIR/BISECT_RUN\"`\n> is missing.\nIn the previous patch series (v3), I implemented the equivalent to the\ncat command but I understood\nreviewers wanted to print the output to the user, so I reverted my\nchanges for this version.\nhttps://lore.kernel.org/git/20210411095538.34129-4-mirucam@gmail.com/\n\n>\n> > +\n> > + if (res == BISECT_ONLY_SKIPPED_LEFT)\n> > + error(_(\"bisect run cannot continue any more\"));\n> > + else if (res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) {\n> > + printf(_(\"bisect run success\"));\n> > + res = BISECT_OK;\n> > + } else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {\n> > + printf(_(\"bisect found first bad commit\"));\n> > + res = BISECT_OK;\n> > + } else if (res) {\n> > + error(_(\"bisect run failed:'git bisect--helper --bisect-state\"\n> > + \" %s' exited with error code %d\"), args.v[0], res);\n> > + } else {\n> > + exit = 0;\n>\n> Since the only purpose of `exit` seems to be that the loop should continue\n> if `exit` is set to 0, and it is only set here, how about doing away with\n> the variable altogether and writing `continue;` instead of `exit = 0;`?\n> Then the conditional block below does not need to be conditional.\n>\nNoted.\n> Other than that: well done!\n>\nThank you for reviewing!,\nMiriam\n\n\n> Ciao,\n> Dscho\n>\n> > + }\n> > +\n> > + if (exit) {\n> > + strbuf_release(&command);\n> > + strvec_clear(&args);\n> > + strvec_clear(&run_args);\n> > + return res;\n> > + }\n> > + }\n> > +}\n> > +\n> > int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> > {\n> > enum {\n> > @@ -1089,6 +1155,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> > BISECT_REPLAY,\n> > BISECT_SKIP,\n> > BISECT_VISUALIZE,\n> > + BISECT_RUN,\n> > } cmdmode = 0;\n> > int res = 0, nolog = 0;\n> > struct option options[] = {\n> > @@ -1112,6 +1179,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> > N_(\"skip some commits for checkout\"), BISECT_SKIP),\n> > OPT_CMDMODE(0, \"bisect-visualize\", &cmdmode,\n> > N_(\"visualize the bisection\"), BISECT_VISUALIZE),\n> > + OPT_CMDMODE(0, \"bisect-run\", &cmdmode,\n> > + N_(\"use ... to automatically bisect.\"), BISECT_RUN),\n> > OPT_BOOL(0, \"no-log\", &nolog,\n> > N_(\"no log for BISECT_WRITE\")),\n> > OPT_END()\n> > @@ -1177,6 +1246,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> > get_terms(&terms);\n> > res = bisect_visualize(&terms, argv, argc);\n> > break;\n> > + case BISECT_RUN:\n> > + if (!argc)\n> > + return error(_(\"bisect run failed: no command provided.\"));\n> > + get_terms(&terms);\n> > + res = bisect_run(&terms, argv, argc);\n> > + break;\n> > default:\n> > BUG(\"unknown subcommand %d\", cmdmode);\n> > }\n> > diff --git a/git-bisect.sh b/git-bisect.sh\n> > index 95f7f3fb8c..e83d011e17 100755\n> > --- a/git-bisect.sh\n> > +++ b/git-bisect.sh\n> > @@ -39,66 +39,6 @@ _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n> > TERM_BAD=bad\n> > TERM_GOOD=good\n> >\n> > -bisect_run () {\n> > - git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n> > -\n> > - test -n \"$*\" || die \"$(gettext \"bisect run failed: no command provided.\")\"\n> > -\n> > - while true\n> > - do\n> > - command=\"$@\"\n> > - eval_gettextln \"running \\$command\"\n> > - \"$@\"\n> > - res=$?\n> > -\n> > - # Check for really bad run error.\n> > - if [ $res -lt 0 -o $res -ge 128 ]\n> > - then\n> > - eval_gettextln \"bisect run failed:\n> > -exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n> > - exit $res\n> > - fi\n> > -\n> > - # Find current state depending on run success or failure.\n> > - # A special exit code of 125 means cannot test.\n> > - if [ $res -eq 125 ]\n> > - then\n> > - state='skip'\n> > - elif [ $res -gt 0 ]\n> > - then\n> > - state=\"$TERM_BAD\"\n> > - else\n> > - state=\"$TERM_GOOD\"\n> > - fi\n> > -\n> > - git bisect--helper --bisect-state $state >\"$GIT_DIR/BISECT_RUN\"\n> > - res=$?\n> > -\n> > - cat \"$GIT_DIR/BISECT_RUN\"\n> > -\n> > - if sane_grep \"first $TERM_BAD commit could be any of\" \"$GIT_DIR/BISECT_RUN\" \\\n> > - >/dev/null\n> > - then\n> > - gettextln \"bisect run cannot continue any more\" >&2\n> > - exit $res\n> > - fi\n> > -\n> > - if [ $res -ne 0 ]\n> > - then\n> > - eval_gettextln \"bisect run failed:\n> > -'bisect-state \\$state' exited with error code \\$res\" >&2\n> > - exit $res\n> > - fi\n> > -\n> > - if sane_grep \"is the first $TERM_BAD commit\" \"$GIT_DIR/BISECT_RUN\" >/dev/null\n> > - then\n> > - gettextln \"bisect run success\"\n> > - exit 0;\n> > - fi\n> > -\n> > - done\n> > -}\n> > -\n> > get_terms () {\n> > if test -s \"$GIT_DIR/BISECT_TERMS\"\n> > then\n> > @@ -137,7 +77,7 @@ case \"$#\" in\n> > log)\n> > git bisect--helper --bisect-log || exit ;;\n> > run)\n> > - bisect_run \"$@\" ;;\n> > + git bisect--helper --bisect-run \"$@\" || exit;;\n> > terms)\n> > git bisect--helper --bisect-terms \"$@\" || exit;;\n> > *)\n> > --\n> > 2.29.2\n> >\n> >\n"},{"id":"433031","messageId":"nycvar.QRO.7.76.6.2108172332050.55@tvgsbejvaqbjf.bet","threadId":"56305","inReplyTo":"CAN7CjDDEv6vGPKZo3sxz8bgfN2Nzqh0HChR-tGrjDGbkhKZo=A@mail.gmail.com","subject":"Re: [PATCH v4 5/6] bisect--helper: reimplement `bisect_run` shell function in C","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-08-17T21:36:49Z","receivedAt":"2021-08-17T21:37:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Miriam,\n\nOn Tue, 17 Aug 2021, Miriam R. wrote:\n\n> El mar, 17 ago 2021 a las 13:42, Johannes Schindelin\n> () escribió:\n> >\n> > On Tue, 17 Aug 2021, Miriam Rubio wrote:\n> >\n> > > From: Tanushree Tumane \n> > >\n> > > [...]\n> > > +\n> > > + if (res == 125)\n> > > + strvec_push(&args, \"skip\");\n> > > + else if (res > 0)\n> > > + strvec_push(&args, terms->term_bad);\n> > > + else\n> > > + strvec_push(&args, terms->term_good);\n> > > +\n> > > + res = bisect_state(terms, args.v, args.nr);\n> >\n> > Since `args.nr` will always be 1, it would probably be better to use\n> > something like this:\n> >\n> > const char *new_state;\n> >\n> > [...]\n> > if (res == 125)\n> > new_state = \"skip\";\n> > else\n> > new_state = res > 0 ?\n> > terms->term_bad : terms->term_good;\n> >\n> > res = bisect_state(terms, &new_state, 1);\n> >\n>\n> Yes, indeed. I will change it.\n>\n> > Also: I think at this stage, an equivalent to `cat\n> > \"$GIT_DIR/BISECT_RUN\"` is missing.\n>\n> In the previous patch series (v3), I implemented the equivalent to the\n> cat command but I understood reviewers wanted to print the output to the\n> user, so I reverted my changes for this version.\n> https://lore.kernel.org/git/20210411095538.34129-4-mirucam@gmail.com/\n\nI am a bit confused: doesn't `bisect_state()` write to the `BISECT_RUN`\nfile? If so, I think we do need to show the contents by opening the file\nand piping it to `stdout`.\n\nFWIW I read\nhttps://lore.kernel.org/git/CAP8UFD3X24F3qgefHpi00PM-KUk+vcqxwy2Dbngbyj7ciavCVQ@mail.gmail.com/\nto mean the same thing, although I have to admit that I am not 100%\ncertain.\n\nJust to make sure: with this patch, at the end of a `git bisect` run, the\nuser is shown the commit message of the first bad commit?\n\nCiao,\nDscho\n"},{"id":"433046","messageId":"CAP8UFD2PE0-8AH7-RH1Xv_cZ5s2bOfR3_KYEhBTdNqYc-Zs5-Q@mail.gmail.com","threadId":"56305","inReplyTo":"nycvar.QRO.7.76.6.2108172332050.55@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v4 5/6] bisect--helper: reimplement `bisect_run` shell function in C","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2021-08-18T08:33:44Z","receivedAt":"2021-08-18T08:34:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Aug 17, 2021 at 11:36 PM Johannes Schindelin\n wrote:\n>\n> Hi Miriam,\n>\n> On Tue, 17 Aug 2021, Miriam R. wrote:\n>\n> > El mar, 17 ago 2021 a las 13:42, Johannes Schindelin\n\n> > > Also: I think at this stage, an equivalent to `cat\n> > > \"$GIT_DIR/BISECT_RUN\"` is missing.\n> >\n> > In the previous patch series (v3), I implemented the equivalent to the\n> > cat command but I understood reviewers wanted to print the output to the\n> > user, so I reverted my changes for this version.\n> > https://lore.kernel.org/git/20210411095538.34129-4-mirucam@gmail.com/\n>\n> I am a bit confused: doesn't `bisect_state()` write to the `BISECT_RUN`\n> file? If so, I think we do need to show the contents by opening the file\n> and piping it to `stdout`.\n>\n> FWIW I read\n> https://lore.kernel.org/git/CAP8UFD3X24F3qgefHpi00PM-KUk+vcqxwy2Dbngbyj7ciavCVQ@mail.gmail.com/\n> to mean the same thing, although I have to admit that I am not 100%\n> certain.\n\nI agree that, after `bisect_state()` has written into the `BISECT_RUN`\nfile, we should indeed be opening it and piping it to `stdout`. That's\nwhat I meant in the above message.\n"},{"id":"433053","messageId":"CAN7CjDBfHrH_BPfNpwyAn6LSeSu_o2C5v7rR-_SnpMf-=UUeow@mail.gmail.com","threadId":"56305","inReplyTo":"CAP8UFD2PE0-8AH7-RH1Xv_cZ5s2bOfR3_KYEhBTdNqYc-Zs5-Q@mail.gmail.com","subject":"Re: [PATCH v4 5/6] bisect--helper: reimplement `bisect_run` shell function in C","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2021-08-18T09:43:26Z","receivedAt":"2021-08-18T09:43:42Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Hi,\n\nEl mié, 18 ago 2021 a las 10:33, Christian Couder\n() escribió:\n>\n> On Tue, Aug 17, 2021 at 11:36 PM Johannes Schindelin\n> wrote:\n> >\n> > Hi Miriam,\n> >\n> > On Tue, 17 Aug 2021, Miriam R. wrote:\n> >\n> > > El mar, 17 ago 2021 a las 13:42, Johannes Schindelin\n>\n> > > > Also: I think at this stage, an equivalent to `cat\n> > > > \"$GIT_DIR/BISECT_RUN\"` is missing.\n> > >\n> > > In the previous patch series (v3), I implemented the equivalent to the\n> > > cat command but I understood reviewers wanted to print the output to the\n> > > user, so I reverted my changes for this version.\n> > > https://lore.kernel.org/git/20210411095538.34129-4-mirucam@gmail.com/\n> >\n> > I am a bit confused: doesn't `bisect_state()` write to the `BISECT_RUN`\n> > file? If so, I think we do need to show the contents by opening the file\n> > and piping it to `stdout`.\n> >\n> > FWIW I read\n> > https://lore.kernel.org/git/CAP8UFD3X24F3qgefHpi00PM-KUk+vcqxwy2Dbngbyj7ciavCVQ@mail.gmail.com/\n> > to mean the same thing, although I have to admit that I am not 100%\n> > certain.\n>\n> I agree that, after `bisect_state()` has written into the `BISECT_RUN`\n> file, we should indeed be opening it and piping it to `stdout`. That's\n> what I meant in the above message.\n\nSorry for the confusion, I was understanding that reviewers wanted a\ndifferent approach, one thing or the other, not both.\nI will do both then.\nThank you for the clarification!\nBest,\nMiriam.\n"}]}