{"thread":{"id":"56493","subject":"[PATCH v7 0/6] Finish converting git bisect to C part 4","startedAt":"2021-09-13T17:40:04Z","lastAt":"2021-09-13T19:31:58Z","messageCount":8,"participants":["Miriam Rubio","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":7,"patchTotal":6},"messages":[{"id":"435749","messageId":"20210913173905.44438-1-mirucam@gmail.com","threadId":"56493","inReplyTo":null,"subject":"[PATCH v7 0/6] Finish converting git bisect to C part 4","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-09-13T17:38:58Z","receivedAt":"2021-09-13T17:40:04Z","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-v7.\n\nI would like to thank Junio Hamano and Johannes Schindelin for reviewing\nthis patch series.\n\nSpecific changes\n----------------\n\n[1/6] t6030-bisect-porcelain: add tests to control bisect run exit cases\n* Remove unnecessary redirections to /dev/null and my_bisect_log.txt\n---\n\n[2/6]t6030-bisect-porcelain: add test for bisect visualize\n* Remove redirection and add double quotes to test's filename\n\n---\n\n[3/6]run-command: make `exists_in_PATH()` non-static\n* Amend commit message.\n* Change parameter name.\n---\n\n[4/6]bisect--helper: reimplement `bisect_visualize()` shell function in C\n* Add brackets to an if expression.\n---\n\n[5/6] bisect--helper: reimplement `bisect_run` shell function in C\n* Add two fflush(stdout) in dup2 dance.\n* Rewrite if-else condition.\n\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 | 158 ++++++++++++++++++++++++++++++++++--\n git-bisect.sh | 87 +-------------------\n run-command.c | 4 +-\n run-command.h | 12 +++\n t/t6030-bisect-porcelain.sh | 18 ++++\n 5 files changed, 184 insertions(+), 95 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"435750","messageId":"20210913173905.44438-2-mirucam@gmail.com","threadId":"56493","inReplyTo":"20210913173905.44438-1-mirucam@gmail.com","subject":"[PATCH v7 1/6] t6030-bisect-porcelain: add tests to control bisect run exit cases","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-09-13T17:38:59Z","receivedAt":"2021-09-13T17:40:13Z","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 | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex a1baf4e451..5986fbecd1 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -962,4 +962,15 @@ 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\n+\tEOF\n+\ttest_must_fail git bisect run ./test_script.sh &&\n+\twrite_script test_script.sh <<-\\EOF &&\n+\texit 255\n+\tEOF\n+\ttest_must_fail git bisect run ./test_script.sh\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"435751","messageId":"20210913173905.44438-3-mirucam@gmail.com","threadId":"56493","inReplyTo":"20210913173905.44438-1-mirucam@gmail.com","subject":"[PATCH v7 2/6] t6030-bisect-porcelain: add test for bisect visualize","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-09-13T17:39:00Z","receivedAt":"2021-09-13T17:40:16Z","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 5986fbecd1..1be85d064e 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -973,4 +973,11 @@ test_expect_success 'bisect run fails with exit code equals or greater than 128'\n \ttest_must_fail git bisect run ./test_script.sh\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\"\n+'\n+\n test_done\n-- \n2.29.2\n\n"},{"id":"435752","messageId":"20210913173905.44438-4-mirucam@gmail.com","threadId":"56493","inReplyTo":"20210913173905.44438-1-mirucam@gmail.com","subject":"[PATCH v7 3/6] run-command: make `exists_in_PATH()` non-static","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-09-13T17:39:01Z","receivedAt":"2021-09-13T17:40:22Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nRemove the `static` keyword from `exists_in_PATH()` function\nand declare 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 | 4 ++--\n run-command.h | 12 ++++++++++++\n 2 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex f72e72cce7..da02553f44 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -210,9 +210,9 @@ 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 *command)\n {\n-\tchar *r = locate_in_PATH(file);\n+\tchar *r = locate_in_PATH(command);\n \tint found = r != NULL;\n \tfree(r);\n \treturn found;\ndiff --git a/run-command.h b/run-command.h\nindex af1296769f..aad027984d 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+ * Check if the command exists on $PATH. 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 command 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 *command);\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":"435753","messageId":"20210913173905.44438-5-mirucam@gmail.com","threadId":"56493","inReplyTo":"20210913173905.44438-1-mirucam@gmail.com","subject":"[PATCH v7 4/6] bisect--helper: reimplement `bisect_visualize()` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-09-13T17:39:02Z","receivedAt":"2021-09-13T17:40:26Z","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..1465310734 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\t} else {\n+\t\t\tstrvec_push(&args, \"log\");\n+\t\t\tflags |= RUN_GIT_CMD;\n+\t\t}\n+\t} else {\n+\t\tif (argv[0][0] == '-') {\n+\t\t\tstrvec_push(&args, \"log\");\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":"435754","messageId":"20210913173905.44438-7-mirucam@gmail.com","threadId":"56493","inReplyTo":"20210913173905.44438-1-mirucam@gmail.com","subject":"[PATCH v7 6/6] bisect--helper: retire `--bisect-next-check` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-09-13T17:39:04Z","receivedAt":"2021-09-13T17:40:27Z","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 ea966268df..bc210b23c8 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -22,7 +22,6 @@ static GIT_PATH_FUNC(git_path_bisect_run, \"BISECT_RUN\")\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@@ -1230,12 +1229,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":"435755","messageId":"20210913173905.44438-6-mirucam@gmail.com","threadId":"56493","inReplyTo":"20210913173905.44438-1-mirucam@gmail.com","subject":"[PATCH v7 5/6] bisect--helper: reimplement `bisect_run` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2021-09-13T17:39:03Z","receivedAt":"2021-09-13T17:40:28Z","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 | 105 +++++++++++++++++++++++++++++++++++++++\n git-bisect.sh | 62 +----------------------\n 2 files changed, 106 insertions(+), 61 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 1465310734..ea966268df 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -18,6 +18,7 @@ static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n static GIT_PATH_FUNC(git_path_bisect_names, \"BISECT_NAMES\")\n static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n+static GIT_PATH_FUNC(git_path_bisect_run, \"BISECT_RUN\")\n \n static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-reset []\"),\n@@ -31,6 +32,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@@ -144,6 +146,19 @@ static int append_to_file(const char *path, const char *format, ...)\n \treturn res;\n }\n \n+static int print_file_to_stdout(const char *path)\n+{\n+\tint fd = open(path, O_RDONLY);\n+\tint ret = 0;\n+\n+\tif (fd < 0)\n+\t\treturn error_errno(_(\"cannot open file '%s' for reading\"), path);\n+\tif (copy_fd(fd, 1) < 0)\n+\t\tret = error_errno(_(\"failed to read '%s'\"), path);\n+\tclose(fd);\n+\treturn ret;\n+}\n+\n static int check_term_format(const char *term, const char *orig_term)\n {\n \tint res;\n@@ -1075,6 +1090,87 @@ 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+\tconst char *new_state;\n+\tint temporary_stdout_fd, saved_stdout;\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\terror(_(\"bisect run failed: no command provided.\"));\n+\t\treturn BISECT_FAILED;\n+\t}\n+\n+\tstrvec_push(&run_args, command.buf);\n+\n+\twhile (1) {\n+\t\tstrvec_clear(&args);\n+\n+\t\tprintf(_(\"running %s\\n\"), 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\tnew_state = \"skip\";\n+\t\telse if (!res)\n+\t\t\tnew_state = terms->term_good;\n+\t\telse\n+\t\t\tnew_state = terms->term_bad;\n+\n+\t\ttemporary_stdout_fd = open(git_path_bisect_run(), O_CREAT | O_WRONLY | O_TRUNC, 0666);\n+\n+\t\tif (temporary_stdout_fd < 0)\n+\t\t\treturn error_errno(_(\"cannot open file '%s' for writing\"), git_path_bisect_run());\n+\n+\t\tfflush(stdout);\n+\t\tsaved_stdout = dup(1);\n+\t\tdup2(temporary_stdout_fd, 1);\n+\n+\t\tres = bisect_state(terms, &new_state, 1);\n+\n+\t\tfflush(stdout);\n+\t\tdup2(saved_stdout, 1);\n+\t\tclose(saved_stdout);\n+\t\tclose(temporary_stdout_fd);\n+\n+\t\tprint_file_to_stdout(git_path_bisect_run());\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\tcontinue;\n+\t\t}\n+\n+\t\tstrbuf_release(&command);\n+\t\tstrvec_clear(&args);\n+\t\tstrvec_clear(&run_args);\n+\t\treturn res;\n+\t}\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -1089,6 +1185,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 +1209,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 +1276,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":"435778","messageId":"875yv446hj.fsf@evledraar.gmail.com","threadId":"56493","inReplyTo":"20210913173905.44438-6-mirucam@gmail.com","subject":"Re: [PATCH v7 5/6] bisect--helper: reimplement `bisect_run` shell function in C","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-13T19:27:11Z","receivedAt":"2021-09-13T19:31:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Sep 13 2021, Miriam Rubio wrote:\n\n> +static int print_file_to_stdout(const char *path)\n> +{\n> +\tint fd = open(path, O_RDONLY);\n> +\tint ret = 0;\n> +\n> +\tif (fd < 0)\n> +\t\treturn error_errno(_(\"cannot open file '%s' for reading\"), path);\n> +\tif (copy_fd(fd, 1) < 0)\n> +\t\tret = error_errno(_(\"failed to read '%s'\"), path);\n> +\tclose(fd);\n> +\treturn ret;\n> +}\n\nReturns int, but that return value is ignored here, and we don't seem to\ngain a caller in 6/6?\n\n> +\tif (argc)\n> +\t\tsq_quote_argv(&command, argv);\n> +\telse {\n> +\t\terror(_(\"bisect run failed: no command provided.\"));\n> +\t\treturn BISECT_FAILED;\n> +\t}\n\nNot new in this series & I see this is v7 already, so ....\n\nJust odd to see this BISECT_FAILED pattern (which is defined to -1),\ninstead of \"return error(...\" like elsewhere.\n\nThen we take that enum and do a \"return -res\" from main(), i.e. it's not\neven that we're somehow guarding everything with these BISECT_* codes in\nthis file (see 30276765c11 (bisect--helper: use '-res' in\n'cmd_bisect__helper' return, 2020-08-28)).\n\nAnyway, can be cleaned up some other time, but...\n\n> +\t\tif (temporary_stdout_fd < 0)\n> +\t\t\treturn error_errno(_(\"cannot open file '%s' for writing\"), git_path_bisect_run());\n\nHere we're doing a \"return error...(\" directly.\n\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\nAlso not a new issue, but if we just covered the BISECT_AUTOSTART case\nhere, then we wouldn't need this default/BUG, the compiler would check\nthat we checked all existing enum arms.\n"}]}