{"thread":{"id":"58956","subject":"[PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c","startedAt":"2022-12-15T09:49:10Z","lastAt":"2023-01-12T15:27:52Z","messageCount":16,"participants":["Ævar Arnfjörð Bjarmason","Christian Couder","Đoàn Trần Công Danh"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"469100","messageId":"cover-0.6-00000000000-20221215T094038Z-avarab@gmail.com","threadId":"58956","inReplyTo":null,"subject":"[PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-15T09:47:43Z","receivedAt":"2022-12-15T09:49:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The js/bisect-in-c topic got replaced by dd/bisect-helper-subcommand\nand later dd/git-bisect-builtin to fix a regression in \"bisect\".\n\nThis small set of fixes is a cherry-pick of various miscellanious\nfixes that were part of js/bisect-in-c that are still worth\nhaving. Johannes and I have coordinated my submission of this\nfollow-up topic off-list.\n\nThe only \"new\" commit here is the 2/6. It's a cherry-pick of\nJohannes's commit to do the same, but as the change to the base was so\nextensive (the range diff won't even pick it up anymore with the\ndefault --creation-factor) he suggested I change the authorship, which\nI've done here.\n\nThere's still other stuff worth picking up from js/bisect-in-c. In\nparticular we should be using parse_options() for the various\nsub-commands of \"bisect\". But as those changes had much more extensive\nconflicts let's leave them for later.\n\nThe range-diff here is to pr-1132/dscho/bisect-in-c-v6[1]. My own\nbranch & CI for this is at [2].\n\n1. https://lore.kernel.org/git/pull.1132.v6.git.1661885419.gitgitgadget@gmail.com/\n2. https://github.com/avar/git/tree/avar-js/bisect-in-c-rebased\n\nJohannes Schindelin (5):\n bisect--helper: simplify exit code computation\n bisect: verify that a bogus option won't try to start a bisection\n bisect run: fix the error message\n bisect: remove Cogito-related code\n bisect: no longer try to clean up left-over `.git/head-name` files\n\nÆvar Arnfjörð Bjarmason (1):\n bisect--helper: make the order consistently `argc, argv`\n\n bisect.c | 3 ---\n builtin/bisect.c | 52 ++++++++++++++-----------------------\n t/t6030-bisect-porcelain.sh | 21 ++++++++++++++-\n 3 files changed, 40 insertions(+), 36 deletions(-)\n\nRange-diff:\n 1: 05262b6a7d1 < -: ----------- bisect--helper: retire the --no-log option\n 2: 1e43148864a < -: ----------- bisect--helper: really retire --bisect-next-check\n 3: 1a1649d9d0d < -: ----------- bisect--helper: really retire `--bisect-autostart`\n 4: 9ab30552c6a ! 1: c8c648e4b8c bisect--helper: simplify exit code computation\n @@ Commit message\n Let's use it instead of duplicating the logic.\n \n Signed-off-by: Johannes Schindelin \n + Signed-off-by: Ævar Arnfjörð Bjarmason \n \n - ## builtin/bisect--helper.c ##\n -@@ builtin/bisect--helper.c: int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n + ## builtin/bisect.c ##\n +@@ builtin/bisect.c: int cmd_bisect(int argc, const char **argv, const char *prefix)\n + \t\tres = fn(argc, argv, prefix);\n \t}\n - \tfree_terms(&terms);\n \n -\t/*\n -\t * Handle early success\n 5: 92b3b116ef8 < -: ----------- bisect--helper: make `terms` an explicit singleton\n 6: c9dc0281e38 < -: ----------- bisect--helper: make the order consistently `argc, argv`\n 7: e97e187bbec < -: ----------- bisect--helper: migrate to OPT_SUBCOMMAND()\n -: ----------- > 2: a0de7ad6836 bisect--helper: make the order consistently `argc, argv`\n 8: 30c87f2e92e ! 3: e1e31278fef bisect: verify that a bogus option won't try to start a bisection\n @@ Commit message\n by \"git bisect start\"` and fail if it was found.\n \n Signed-off-by: Johannes Schindelin \n + Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## t/t6030-bisect-porcelain.sh ##\n @@ t/t6030-bisect-porcelain.sh: test_expect_success 'bisect start with one term1 and term2' '\n 9: 4696652b99c ! 4: 59a8a3085b1 bisect run: fix the error message\n @@ Commit message\n \n Helped-by: Elijah Newren \n Signed-off-by: Johannes Schindelin \n + Signed-off-by: Ævar Arnfjörð Bjarmason \n \n - ## builtin/bisect--helper.c ##\n -@@ builtin/bisect--helper.c: static int cmd_bisect_run(int argc, const char **argv, const char *prefix)\n - \t\t\tprintf(_(\"bisect found first bad commit\"));\n + ## builtin/bisect.c ##\n +@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n + \t\t\tputs(_(\"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\terror(_(\"bisect run failed: 'git bisect\"\n - \t\t\t\" %s' exited with error code %d\"), new_state, res);\n +-\t\t\terror(_(\"bisect run failed: 'bisect-state %s'\"\n ++\t\t\terror(_(\"bisect run failed: 'git bisect %s'\"\n + \t\t\t\t\" exited with error code %d\"), new_state, res);\n \t\t} else {\n \t\t\tcontinue;\n \n10: b202a0e386c < -: ----------- bisect: avoid double-quoting when printing the failed command\n11: 3376b450867 < -: ----------- bisect--helper: calling `bisect_state()` without an argument is a bug\n12: e7623508f90 < -: ----------- bisect--helper: make `state` optional\n13: 3f052580c95 < -: ----------- bisect: move even the command-line parsing to `bisect--helper`\n14: a83fe3dc3c2 < -: ----------- Turn `git bisect` into a full built-in\n15: f2132b61ff7 ! 5: 1b70cd79cae bisect: remove Cogito-related code\n @@ Commit message\n remove the last remnant of Cogito-accommodating code.\n \n Signed-off-by: Johannes Schindelin \n + Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## builtin/bisect.c ##\n @@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_expected_rev, \"BISECT_EXPECTED_REV\")\n @@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_expected_rev, \"BISECT_EXP\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 -@@ builtin/bisect.c: static int cmd_bisect_start(int argc, const char **argv, const char *prefix)\n +@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,\n \t\t\tstrbuf_addstr(&start_head, oid_to_hex(&head_oid));\n \t\t} else if (!get_oid(head, &head_oid) &&\n \t\t\t skip_prefix(head, \"refs/heads/\", &head)) {\n16: 4f93692e071 ! 6: 2ad89aca728 bisect: no longer try to clean up left-over `.git/head-name` files\n @@ Commit message\n So let's remove that code, at long last.\n \n Signed-off-by: Johannes Schindelin \n + Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## bisect.c ##\n @@ bisect.c: static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n-- \n2.39.0.rc2.1048.g0e5493b8d5b\n\n"},{"id":"469101","messageId":"patch-1.6-c8c648e4b8c-20221215T094038Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-0.6-00000000000-20221215T094038Z-avarab@gmail.com","subject":"[PATCH 1/6] bisect--helper: simplify exit code computation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-15T09:47:44Z","receivedAt":"2022-12-15T09:49:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Schindelin \n\nWe _already_ have a function to determine whether a given `enum\nbisect_error` value is non-zero but still _actually_ indicates success.\n\nLet's use it instead of duplicating the logic.\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/bisect.c | 9 +--------\n 1 file changed, 1 insertion(+), 8 deletions(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex cc9483e8515..09505fc4dce 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -1440,12 +1440,5 @@ int cmd_bisect(int argc, const char **argv, const char *prefix)\n \t\tres = fn(argc, argv, prefix);\n \t}\n \n-\t/*\n-\t * Handle early success\n-\t * From check_merge_bases > check_good_are_ancestors_of_bad > bisect_next_all\n-\t */\n-\tif ((res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) || (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND))\n-\t\tres = BISECT_OK;\n-\n-\treturn -res;\n+\treturn is_bisect_success(res) ? 0 : -res;\n }\n-- \n2.39.0.rc2.1048.g0e5493b8d5b\n\n"},{"id":"469102","messageId":"patch-2.6-a0de7ad6836-20221215T094038Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-0.6-00000000000-20221215T094038Z-avarab@gmail.com","subject":"[PATCH 2/6] bisect--helper: make the order consistently `argc, argv`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-15T09:47:45Z","receivedAt":"2022-12-15T09:49:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In C, the natural order is for `argc` to come before `argv` by virtue of\nthe `main()` function declaring the parameters in precisely that order.\n\nIt is confusing & distracting, then, when readers familiar with the C\nlanguage read code where that order is switched around.\n\nLet's just change the order and avoid that type of developer friction.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Johannes Schindelin \n---\n builtin/bisect.c | 33 ++++++++++++++++++---------------\n 1 file changed, 18 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 09505fc4dce..9fc8db06944 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -678,7 +678,8 @@ static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char\n \treturn bisect_next(terms, prefix);\n }\n \n-static enum bisect_error bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n+static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,\n+\t\t\t\t const char **argv)\n {\n \tint no_checkout = 0;\n \tint first_parent_only = 0;\n@@ -908,13 +909,13 @@ static int bisect_autostart(struct bisect_terms *terms)\n \tyesno = git_prompt(_(\"Do you want me to do it for you \"\n \t\t\t \"[Y/n]? \"), PROMPT_ECHO);\n \tres = tolower(*yesno) == 'n' ?\n-\t\t-1 : bisect_start(terms, empty_strvec, 0);\n+\t\t-1 : bisect_start(terms, 0, empty_strvec);\n \n \treturn res;\n }\n \n-static enum bisect_error bisect_state(struct bisect_terms *terms, const char **argv,\n-\t\t\t\t int argc)\n+static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,\n+\t\t\t\t const char **argv)\n {\n \tconst char *state;\n \tint i, verify_expected = 1;\n@@ -1033,7 +1034,7 @@ static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)\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\tres = bisect_start(terms, argv.nr, argv.v);\n \t\tstrvec_clear(&argv);\n \t\treturn res;\n \t}\n@@ -1083,7 +1084,8 @@ 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+static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc,\n+\t\t\t\t const char **argv)\n {\n \tint i;\n \tenum bisect_error res;\n@@ -1113,13 +1115,14 @@ static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **ar\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+\tres = bisect_state(terms, argv_state.nr, argv_state.v);\n \n \tstrvec_clear(&argv_state);\n \treturn res;\n }\n \n-static int bisect_visualize(struct bisect_terms *terms, const char **argv, int argc)\n+static int bisect_visualize(struct bisect_terms *terms, int argc,\n+\t\t\t const char **argv)\n {\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -1202,7 +1205,7 @@ static int verify_good(const struct bisect_terms *terms, const char *command)\n \treturn rc;\n }\n \n-static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)\n+static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n {\n \tint res = BISECT_OK;\n \tstruct strbuf command = STRBUF_INIT;\n@@ -1271,7 +1274,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)\n \t\tsaved_stdout = dup(1);\n \t\tdup2(temporary_stdout_fd, 1);\n \n-\t\tres = bisect_state(terms, &new_state, 1);\n+\t\tres = bisect_state(terms, 1, &new_state);\n \n \t\tfflush(stdout);\n \t\tdup2(saved_stdout, 1);\n@@ -1328,7 +1331,7 @@ static int cmd_bisect__start(int argc, const char **argv, const char *prefix UNU\n \tstruct bisect_terms terms = { 0 };\n \n \tset_terms(&terms, \"bad\", \"good\");\n-\tres = bisect_start(&terms, argv, argc);\n+\tres = bisect_start(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n }\n@@ -1372,7 +1375,7 @@ static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUS\n \n \tset_terms(&terms, \"bad\", \"good\");\n \tget_terms(&terms);\n-\tres = bisect_skip(&terms, argv, argc);\n+\tres = bisect_skip(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n }\n@@ -1383,7 +1386,7 @@ static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix\n \tstruct bisect_terms terms = { 0 };\n \n \tget_terms(&terms);\n-\tres = bisect_visualize(&terms, argv, argc);\n+\tres = bisect_visualize(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n }\n@@ -1396,7 +1399,7 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE\n \tif (!argc)\n \t\treturn error(_(\"'%s' failed: no command provided.\"), \"git bisect run\");\n \tget_terms(&terms);\n-\tres = bisect_run(&terms, argv, argc);\n+\tres = bisect_run(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n }\n@@ -1432,7 +1435,7 @@ int cmd_bisect(int argc, const char **argv, const char *prefix)\n \t\tif (check_and_set_terms(&terms, argv[0]))\n \t\t\tusage_msg_optf(_(\"unknown command: '%s'\"), git_bisect_usage,\n \t\t\t\t options, argv[0]);\n-\t\tres = bisect_state(&terms, argv, argc);\n+\t\tres = bisect_state(&terms, argc, argv);\n \t\tfree_terms(&terms);\n \t} else {\n \t\targc--;\n-- \n2.39.0.rc2.1048.g0e5493b8d5b\n\n"},{"id":"469103","messageId":"patch-3.6-e1e31278fef-20221215T094038Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-0.6-00000000000-20221215T094038Z-avarab@gmail.com","subject":"[PATCH 3/6] bisect: verify that a bogus option won't try to start a bisection","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-15T09:47:46Z","receivedAt":"2022-12-15T09:49:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Schindelin \n\nWe do not want `git bisect --bogus-option` to start a bisection. To\nverify that, we look for the tell-tale error message `You need to start\nby \"git bisect start\"` and fail if it was found.\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t6030-bisect-porcelain.sh | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 98a72ff78a7..9e56b42b5da 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -1058,6 +1058,16 @@ test_expect_success 'bisect start with one term1 and term2' '\n \tgit bisect reset\n '\n \n+test_expect_success 'bogus command does not start bisect' '\n+\tgit bisect reset &&\n+\ttest_must_fail git bisect --bisect-terms 1 2 2>out &&\n+\t! grep \"You need to start\" out &&\n+\ttest_must_fail git bisect --bisect-terms 2>out &&\n+\t! grep \"You need to start\" out &&\n+\tgrep \"git bisect.*visualize\" out &&\n+\tgit bisect reset\n+'\n+\n test_expect_success 'bisect replay with term1 and term2' '\n \tgit bisect replay log_to_replay.txt >bisect_result &&\n \tgrep \"$HASH2 is the first term1 commit\" bisect_result &&\n-- \n2.39.0.rc2.1048.g0e5493b8d5b\n\n"},{"id":"469104","messageId":"patch-4.6-59a8a3085b1-20221215T094038Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-0.6-00000000000-20221215T094038Z-avarab@gmail.com","subject":"[PATCH 4/6] bisect run: fix the error message","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-15T09:47:47Z","receivedAt":"2022-12-15T09:49:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Schindelin \n\nIn d1bbbe45df8 (bisect--helper: reimplement `bisect_run` shell function\nin C, 2021-09-13), we ported the `bisect run` subcommand to C, including\nthe part that prints out an error message when the implicit `git bisect\nbad` or `git bisect good` failed.\n\nHowever, the error message was supposed to print out whether the state\nwas \"good\" or \"bad\", but used a bogus (because non-populated) `args`\nvariable for it. This was fixed in 80c2e9657f2 (bisect--helper: report\nactual bisect_state() argument on error, 2022-01-18), but the error\nmessage still talks about `bisect--helper`, which is an implementation\ndetail that should not concern end users.\n\nFix that, and add a regression test to ensure that the intended form of\nthe error message.\n\nHelped-by: Elijah Newren \nSigned-off-by: Johannes Schindelin \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/bisect.c | 2 +-\n t/t6030-bisect-porcelain.sh | 10 ++++++++++\n 2 files changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 9fc8db06944..0786ebf4012 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -1292,7 +1292,7 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n \t\t\tputs(_(\"bisect found first bad commit\"));\n \t\t\tres = BISECT_OK;\n \t\t} else if (res) {\n-\t\t\terror(_(\"bisect run failed: 'bisect-state %s'\"\n+\t\t\terror(_(\"bisect run failed: 'git bisect %s'\"\n \t\t\t\t\" exited with error code %d\"), new_state, res);\n \t\t} else {\n \t\t\tcontinue;\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 9e56b42b5da..0a62ea2b3ce 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -1221,4 +1221,14 @@ test_expect_success 'bisect state output with bad commit' '\n \tgrep -F \"waiting for good commit(s), bad commit known\" output\n '\n \n+test_expect_success 'verify correct error message' '\n+\tgit bisect reset &&\n+\tgit bisect start $HASH4 $HASH1 &&\n+\twrite_script test_script.sh <<-\\EOF &&\n+\trm .git/BISECT*\n+\tEOF\n+\ttest_must_fail git bisect run ./test_script.sh 2>error &&\n+\tgrep \"git bisect good.*exited with error code\" error\n+'\n+\n test_done\n-- \n2.39.0.rc2.1048.g0e5493b8d5b\n\n"},{"id":"469105","messageId":"patch-5.6-1b70cd79cae-20221215T094038Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-0.6-00000000000-20221215T094038Z-avarab@gmail.com","subject":"[PATCH 5/6] bisect: remove Cogito-related code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-15T09:47:48Z","receivedAt":"2022-12-15T09:49:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Schindelin \n\nOnce upon a time, there was this idea that Git would not actually be a\nsingle coherent program, but rather a set of low-level programs that\nusers cobble together via shell scripts, or develop high-level user\ninterfaces for Git, or both.\n\nCogito was such a high-level user interface, incidentally implemented\nvia shell scripts that cobble together Git calls.\n\nIt did turn out relatively quickly that Git would much rather provide a\nuseful high-level user interface itself.\n\nAs of April 19th, 2007, Cogito was therefore discontinued (see\nhttps://lore.kernel.org/git/20070419124648.GL4489@pasky.or.cz/).\n\nNevertheless, for almost 15 years after that announcement, Git carried\nspecial code in `git bisect` to accommodate Cogito.\n\nSince it is beyond doubt that there are no more Cogito users, let's\nremove the last remnant of Cogito-accommodating code.\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/bisect.c | 8 --------\n 1 file changed, 8 deletions(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 0786ebf4012..73017402671 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -15,7 +15,6 @@ static GIT_PATH_FUNC(git_path_bisect_expected_rev, \"BISECT_EXPECTED_REV\")\n static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, \"BISECT_ANCESTORS_OK\")\n static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n 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@@ -808,13 +807,6 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,\n \t\t\tstrbuf_addstr(&start_head, oid_to_hex(&head_oid));\n \t\t} else if (!get_oid(head, &head_oid) &&\n \t\t\t skip_prefix(head, \"refs/heads/\", &head)) {\n-\t\t\t/*\n-\t\t\t * This error message should only be triggered by\n-\t\t\t * cogito usage, and cogito users should understand\n-\t\t\t * it relates to cg-seek.\n-\t\t\t */\n-\t\t\tif (!is_empty_or_missing_file(git_path_head_name()))\n-\t\t\t\treturn error(_(\"won't bisect on cg-seek'ed tree\"));\n \t\t\tstrbuf_addstr(&start_head, head);\n \t\t} else {\n \t\t\treturn error(_(\"bad HEAD - strange symbolic ref\"));\n-- \n2.39.0.rc2.1048.g0e5493b8d5b\n\n"},{"id":"469106","messageId":"patch-6.6-2ad89aca728-20221215T094038Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-0.6-00000000000-20221215T094038Z-avarab@gmail.com","subject":"[PATCH 6/6] bisect: no longer try to clean up left-over `.git/head-name` files","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-15T09:47:49Z","receivedAt":"2022-12-15T09:49:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Schindelin \n\nAs per the code comment, the `.git/head-name` files were cleaned up for\nbackwards-compatibility: an old version of `git bisect` could have left\nthem behind.\n\nNow, just how old would such a version be? As of 0f497e75f05 (Eliminate\nconfusing \"won't bisect on seeked tree\" failure, 2008-02-23), `git\nbisect` does not write that file anymore. Which corresponds to Git\nv1.5.4.4.\n\nEven if the likelihood is non-nil that there might still be users out\nthere who use such an old version to start a bisection, but then decide\nto continue bisecting with a current Git version, it is highly\nimprobable.\n\nSo let's remove that code, at long last.\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n bisect.c | 3 ---\n t/t6030-bisect-porcelain.sh | 1 -\n 2 files changed, 4 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex ec7487e6836..ef5ee5a6436 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -472,7 +472,6 @@ static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_bisect_terms, \"BISECT_TERMS\")\n static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n-static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n \n static void read_bisect_paths(struct strvec *array)\n {\n@@ -1188,8 +1187,6 @@ int bisect_clean_state(void)\n \tunlink_or_warn(git_path_bisect_run());\n \tunlink_or_warn(git_path_bisect_terms());\n \tunlink_or_warn(git_path_bisect_first_parent());\n-\t/* Cleanup head-name if it got left by an old version of git-bisect */\n-\tunlink_or_warn(git_path_head_name());\n \t/*\n \t * Cleanup BISECT_START last to support the --no-checkout option\n \t * introduced in the commit 4796e823a.\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 0a62ea2b3ce..3ba4fdf6153 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -1158,7 +1158,6 @@ test_expect_success 'git bisect reset cleans bisection state properly' '\n \ttest_path_is_missing \".git/BISECT_LOG\" &&\n \ttest_path_is_missing \".git/BISECT_RUN\" &&\n \ttest_path_is_missing \".git/BISECT_TERMS\" &&\n-\ttest_path_is_missing \".git/head-name\" &&\n \ttest_path_is_missing \".git/BISECT_HEAD\" &&\n \ttest_path_is_missing \".git/BISECT_START\"\n '\n-- \n2.39.0.rc2.1048.g0e5493b8d5b\n\n"},{"id":"469117","messageId":"CAP8UFD2KT-GnBa1jGjXGc=bH_hQHhO9k0Qw6qBwmsXdsaHxnKA@mail.gmail.com","threadId":"58956","inReplyTo":"cover-0.6-00000000000-20221215T094038Z-avarab@gmail.com","subject":"Re: [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-12-15T10:45:17Z","receivedAt":"2022-12-15T10:45:38Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Dec 15, 2022 at 11:27 AM Ævar Arnfjörð Bjarmason\n wrote:\n\n> This small set of fixes is a cherry-pick of various miscellanious\n\ns/miscellanious/miscellaneous/\n\n> fixes that were part of js/bisect-in-c that are still worth\n> having.\n\nThanks! All the patches make sense and look good to me.\n\n> Johannes Schindelin (5):\n> bisect--helper: simplify exit code computation\n> bisect: verify that a bogus option won't try to start a bisection\n> bisect run: fix the error message\n\nI just noticed a nit in the commit message of the above patch. It contains:\n\n\"Fix that, and add a regression test to ensure that the intended form of\nthe error message.\"\n\nAnd it seems to me that something like \"is printed\" is missing from\nthe end of that sentence.\n\n> bisect: remove Cogito-related code\n> bisect: no longer try to clean up left-over `.git/head-name` files\n>\n> Ævar Arnfjörð Bjarmason (1):\n> bisect--helper: make the order consistently `argc, argv`\n"},{"id":"469128","messageId":"Y5s4Zy14TbGalfIl@danh.dev","threadId":"58956","inReplyTo":"patch-4.6-59a8a3085b1-20221215T094038Z-avarab@gmail.com","subject":"Re: [PATCH 4/6] bisect run: fix the error message","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2022-12-15T15:08:23Z","receivedAt":"2022-12-15T15:08:56Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2022-12-15 10:47:47+0100, Ævar Arnfjörð Bjarmason wrote:\n> From: Johannes Schindelin \n> \n> In d1bbbe45df8 (bisect--helper: reimplement `bisect_run` shell function\n> in C, 2021-09-13), we ported the `bisect run` subcommand to C, including\n> the part that prints out an error message when the implicit `git bisect\n> bad` or `git bisect good` failed.\n> \n> However, the error message was supposed to print out whether the state\n> was \"good\" or \"bad\", but used a bogus (because non-populated) `args`\n> variable for it. This was fixed in 80c2e9657f2 (bisect--helper: report\n> actual bisect_state() argument on error, 2022-01-18), but the error\n> message still talks about `bisect--helper`, which is an implementation\n> detail that should not concern end users.\n\nI don't think the error still talks about `bisect--helper`.\nThe error is talking about some \"bisect-state\" which is not it Git's\nglossary, though.\n\n> Fix that, and add a regression test to ensure that the intended form of\n> the error message.\n> \n> Helped-by: Elijah Newren \n> Signed-off-by: Johannes Schindelin \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> builtin/bisect.c | 2 +-\n> t/t6030-bisect-porcelain.sh | 10 ++++++++++\n> 2 files changed, 11 insertions(+), 1 deletion(-)\n> \n> diff --git a/builtin/bisect.c b/builtin/bisect.c\n> index 9fc8db06944..0786ebf4012 100644\n> --- a/builtin/bisect.c\n> +++ b/builtin/bisect.c\n> @@ -1292,7 +1292,7 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n> \t\t\tputs(_(\"bisect found first bad commit\"));\n> \t\t\tres = BISECT_OK;\n> \t\t} else if (res) {\n> -\t\t\terror(_(\"bisect run failed: 'bisect-state %s'\"\n> +\t\t\terror(_(\"bisect run failed: 'git bisect %s'\"\n> \t\t\t\t\" exited with error code %d\"), new_state, res);\n> \t\t} else {\n> \t\t\tcontinue;\n> diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\n> index 9e56b42b5da..0a62ea2b3ce 100755\n> --- a/t/t6030-bisect-porcelain.sh\n> +++ b/t/t6030-bisect-porcelain.sh\n> @@ -1221,4 +1221,14 @@ test_expect_success 'bisect state output with bad commit' '\n> \tgrep -F \"waiting for good commit(s), bad commit known\" output\n> '\n> \n> +test_expect_success 'verify correct error message' '\n> +\tgit bisect reset &&\n> +\tgit bisect start $HASH4 $HASH1 &&\n> +\twrite_script test_script.sh <<-\\EOF &&\n> +\trm .git/BISECT*\n> +\tEOF\n> +\ttest_must_fail git bisect run ./test_script.sh 2>error &&\n> +\tgrep \"git bisect good.*exited with error code\" error\n> +'\n> +\n> test_done\n> -- \n> 2.39.0.rc2.1048.g0e5493b8d5b\n> \n\n-- \nDanh\n"},{"id":"470193","messageId":"patch-v2-1.6-32c45bbf851-20230112T151651Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-v2-0.6-00000000000-20230112T151651Z-avarab@gmail.com","subject":"[PATCH v2 1/6] bisect--helper: simplify exit code computation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T15:19:09Z","receivedAt":"2023-01-12T15:27:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Schindelin \n\nWe _already_ have a function to determine whether a given `enum\nbisect_error` value is non-zero but still _actually_ indicates success.\n\nLet's use it instead of duplicating the logic.\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/bisect.c | 9 +--------\n 1 file changed, 1 insertion(+), 8 deletions(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex cc9483e8515..09505fc4dce 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -1440,12 +1440,5 @@ int cmd_bisect(int argc, const char **argv, const char *prefix)\n \t\tres = fn(argc, argv, prefix);\n \t}\n \n-\t/*\n-\t * Handle early success\n-\t * From check_merge_bases > check_good_are_ancestors_of_bad > bisect_next_all\n-\t */\n-\tif ((res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) || (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND))\n-\t\tres = BISECT_OK;\n-\n-\treturn -res;\n+\treturn is_bisect_success(res) ? 0 : -res;\n }\n-- \n2.39.0.1215.g1ba3f685d4f\n\n"},{"id":"470194","messageId":"patch-v2-2.6-1f4449dd081-20230112T151651Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-v2-0.6-00000000000-20230112T151651Z-avarab@gmail.com","subject":"[PATCH v2 2/6] bisect--helper: make the order consistently `argc, argv`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T15:19:10Z","receivedAt":"2023-01-12T15:27:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In C, the natural order is for `argc` to come before `argv` by virtue of\nthe `main()` function declaring the parameters in precisely that order.\n\nIt is confusing & distracting, then, when readers familiar with the C\nlanguage read code where that order is switched around.\n\nLet's just change the order and avoid that type of developer friction.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Johannes Schindelin \n---\n builtin/bisect.c | 33 ++++++++++++++++++---------------\n 1 file changed, 18 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 09505fc4dce..9fc8db06944 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -678,7 +678,8 @@ static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char\n \treturn bisect_next(terms, prefix);\n }\n \n-static enum bisect_error bisect_start(struct bisect_terms *terms, const char **argv, int argc)\n+static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,\n+\t\t\t\t const char **argv)\n {\n \tint no_checkout = 0;\n \tint first_parent_only = 0;\n@@ -908,13 +909,13 @@ static int bisect_autostart(struct bisect_terms *terms)\n \tyesno = git_prompt(_(\"Do you want me to do it for you \"\n \t\t\t \"[Y/n]? \"), PROMPT_ECHO);\n \tres = tolower(*yesno) == 'n' ?\n-\t\t-1 : bisect_start(terms, empty_strvec, 0);\n+\t\t-1 : bisect_start(terms, 0, empty_strvec);\n \n \treturn res;\n }\n \n-static enum bisect_error bisect_state(struct bisect_terms *terms, const char **argv,\n-\t\t\t\t int argc)\n+static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,\n+\t\t\t\t const char **argv)\n {\n \tconst char *state;\n \tint i, verify_expected = 1;\n@@ -1033,7 +1034,7 @@ static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)\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\tres = bisect_start(terms, argv.nr, argv.v);\n \t\tstrvec_clear(&argv);\n \t\treturn res;\n \t}\n@@ -1083,7 +1084,8 @@ 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+static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc,\n+\t\t\t\t const char **argv)\n {\n \tint i;\n \tenum bisect_error res;\n@@ -1113,13 +1115,14 @@ static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **ar\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+\tres = bisect_state(terms, argv_state.nr, argv_state.v);\n \n \tstrvec_clear(&argv_state);\n \treturn res;\n }\n \n-static int bisect_visualize(struct bisect_terms *terms, const char **argv, int argc)\n+static int bisect_visualize(struct bisect_terms *terms, int argc,\n+\t\t\t const char **argv)\n {\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -1202,7 +1205,7 @@ static int verify_good(const struct bisect_terms *terms, const char *command)\n \treturn rc;\n }\n \n-static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)\n+static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n {\n \tint res = BISECT_OK;\n \tstruct strbuf command = STRBUF_INIT;\n@@ -1271,7 +1274,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)\n \t\tsaved_stdout = dup(1);\n \t\tdup2(temporary_stdout_fd, 1);\n \n-\t\tres = bisect_state(terms, &new_state, 1);\n+\t\tres = bisect_state(terms, 1, &new_state);\n \n \t\tfflush(stdout);\n \t\tdup2(saved_stdout, 1);\n@@ -1328,7 +1331,7 @@ static int cmd_bisect__start(int argc, const char **argv, const char *prefix UNU\n \tstruct bisect_terms terms = { 0 };\n \n \tset_terms(&terms, \"bad\", \"good\");\n-\tres = bisect_start(&terms, argv, argc);\n+\tres = bisect_start(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n }\n@@ -1372,7 +1375,7 @@ static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUS\n \n \tset_terms(&terms, \"bad\", \"good\");\n \tget_terms(&terms);\n-\tres = bisect_skip(&terms, argv, argc);\n+\tres = bisect_skip(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n }\n@@ -1383,7 +1386,7 @@ static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix\n \tstruct bisect_terms terms = { 0 };\n \n \tget_terms(&terms);\n-\tres = bisect_visualize(&terms, argv, argc);\n+\tres = bisect_visualize(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n }\n@@ -1396,7 +1399,7 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE\n \tif (!argc)\n \t\treturn error(_(\"'%s' failed: no command provided.\"), \"git bisect run\");\n \tget_terms(&terms);\n-\tres = bisect_run(&terms, argv, argc);\n+\tres = bisect_run(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n }\n@@ -1432,7 +1435,7 @@ int cmd_bisect(int argc, const char **argv, const char *prefix)\n \t\tif (check_and_set_terms(&terms, argv[0]))\n \t\t\tusage_msg_optf(_(\"unknown command: '%s'\"), git_bisect_usage,\n \t\t\t\t options, argv[0]);\n-\t\tres = bisect_state(&terms, argv, argc);\n+\t\tres = bisect_state(&terms, argc, argv);\n \t\tfree_terms(&terms);\n \t} else {\n \t\targc--;\n-- \n2.39.0.1215.g1ba3f685d4f\n\n"},{"id":"470195","messageId":"cover-v2-0.6-00000000000-20230112T151651Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-0.6-00000000000-20221215T094038Z-avarab@gmail.com","subject":"[PATCH v2 0/6] bisect: follow-up fixes from js/bisect-in-c","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T15:19:08Z","receivedAt":"2023-01-12T15:27:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"A trivial update to this topic of follow-up fixes to \"bisect\", which\nis now a built-in. See\nhttps://lore.kernel.org/git/cover-0.6-00000000000-20221215T094038Z-avarab@gmail.com/\nfor the v1 & general summary.\n\nChange since v1:\n\n * Rephrased & updated a commit message which was outdated as of\n f37d0bdd42d, see the range-diff. Thanks to Đoàn Trần Công Danh for\n spotting it.\n\nJohannes Schindelin (5):\n bisect--helper: simplify exit code computation\n bisect: verify that a bogus option won't try to start a bisection\n bisect run: fix the error message\n bisect: remove Cogito-related code\n bisect: no longer try to clean up left-over `.git/head-name` files\n\nÆvar Arnfjörð Bjarmason (1):\n bisect--helper: make the order consistently `argc, argv`\n\n bisect.c | 3 ---\n builtin/bisect.c | 52 ++++++++++++++-----------------------\n t/t6030-bisect-porcelain.sh | 21 ++++++++++++++-\n 3 files changed, 40 insertions(+), 36 deletions(-)\n\nRange-diff against v1:\n1: c8c648e4b8c = 1: 32c45bbf851 bisect--helper: simplify exit code computation\n2: a0de7ad6836 = 2: 1f4449dd081 bisect--helper: make the order consistently `argc, argv`\n3: e1e31278fef = 3: 0cfb7dc572c bisect: verify that a bogus option won't try to start a bisection\n4: 59a8a3085b1 ! 4: 4dda1019767 bisect run: fix the error message\n @@ Commit message\n \n However, the error message was supposed to print out whether the state\n was \"good\" or \"bad\", but used a bogus (because non-populated) `args`\n - variable for it. This was fixed in 80c2e9657f2 (bisect--helper: report\n - actual bisect_state() argument on error, 2022-01-18), but the error\n - message still talks about `bisect--helper`, which is an implementation\n - detail that should not concern end users.\n + variable for it. This was fixed in [1], but as of [2] (when\n + `bisect--helper` was changed to the present `bisect-state') the error\n + message still talks about implementation details that should not\n + concern end users.\n \n Fix that, and add a regression test to ensure that the intended form of\n the error message.\n \n + 1. 80c2e9657f2 (bisect--helper: report actual bisect_state() argument\n + on error, 2022-01-18\n + 2. f37d0bdd42d (bisect: fix output regressions in v2.30.0, 2022-11-10)\n +\n Helped-by: Elijah Newren \n Signed-off-by: Johannes Schindelin \n Signed-off-by: Ævar Arnfjörð Bjarmason \n5: 1b70cd79cae = 5: 1600ef41608 bisect: remove Cogito-related code\n6: 2ad89aca728 = 6: 817fe726b4b bisect: no longer try to clean up left-over `.git/head-name` files\n-- \n2.39.0.1215.g1ba3f685d4f\n\n"},{"id":"470196","messageId":"patch-v2-3.6-0cfb7dc572c-20230112T151651Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-v2-0.6-00000000000-20230112T151651Z-avarab@gmail.com","subject":"[PATCH v2 3/6] bisect: verify that a bogus option won't try to start a bisection","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T15:19:11Z","receivedAt":"2023-01-12T15:27:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Schindelin \n\nWe do not want `git bisect --bogus-option` to start a bisection. To\nverify that, we look for the tell-tale error message `You need to start\nby \"git bisect start\"` and fail if it was found.\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t6030-bisect-porcelain.sh | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 98a72ff78a7..9e56b42b5da 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -1058,6 +1058,16 @@ test_expect_success 'bisect start with one term1 and term2' '\n \tgit bisect reset\n '\n \n+test_expect_success 'bogus command does not start bisect' '\n+\tgit bisect reset &&\n+\ttest_must_fail git bisect --bisect-terms 1 2 2>out &&\n+\t! grep \"You need to start\" out &&\n+\ttest_must_fail git bisect --bisect-terms 2>out &&\n+\t! grep \"You need to start\" out &&\n+\tgrep \"git bisect.*visualize\" out &&\n+\tgit bisect reset\n+'\n+\n test_expect_success 'bisect replay with term1 and term2' '\n \tgit bisect replay log_to_replay.txt >bisect_result &&\n \tgrep \"$HASH2 is the first term1 commit\" bisect_result &&\n-- \n2.39.0.1215.g1ba3f685d4f\n\n"},{"id":"470197","messageId":"patch-v2-5.6-1600ef41608-20230112T151651Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-v2-0.6-00000000000-20230112T151651Z-avarab@gmail.com","subject":"[PATCH v2 5/6] bisect: remove Cogito-related code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T15:19:13Z","receivedAt":"2023-01-12T15:27:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Schindelin \n\nOnce upon a time, there was this idea that Git would not actually be a\nsingle coherent program, but rather a set of low-level programs that\nusers cobble together via shell scripts, or develop high-level user\ninterfaces for Git, or both.\n\nCogito was such a high-level user interface, incidentally implemented\nvia shell scripts that cobble together Git calls.\n\nIt did turn out relatively quickly that Git would much rather provide a\nuseful high-level user interface itself.\n\nAs of April 19th, 2007, Cogito was therefore discontinued (see\nhttps://lore.kernel.org/git/20070419124648.GL4489@pasky.or.cz/).\n\nNevertheless, for almost 15 years after that announcement, Git carried\nspecial code in `git bisect` to accommodate Cogito.\n\nSince it is beyond doubt that there are no more Cogito users, let's\nremove the last remnant of Cogito-accommodating code.\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/bisect.c | 8 --------\n 1 file changed, 8 deletions(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 0786ebf4012..73017402671 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -15,7 +15,6 @@ static GIT_PATH_FUNC(git_path_bisect_expected_rev, \"BISECT_EXPECTED_REV\")\n static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, \"BISECT_ANCESTORS_OK\")\n static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n 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@@ -808,13 +807,6 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,\n \t\t\tstrbuf_addstr(&start_head, oid_to_hex(&head_oid));\n \t\t} else if (!get_oid(head, &head_oid) &&\n \t\t\t skip_prefix(head, \"refs/heads/\", &head)) {\n-\t\t\t/*\n-\t\t\t * This error message should only be triggered by\n-\t\t\t * cogito usage, and cogito users should understand\n-\t\t\t * it relates to cg-seek.\n-\t\t\t */\n-\t\t\tif (!is_empty_or_missing_file(git_path_head_name()))\n-\t\t\t\treturn error(_(\"won't bisect on cg-seek'ed tree\"));\n \t\t\tstrbuf_addstr(&start_head, head);\n \t\t} else {\n \t\t\treturn error(_(\"bad HEAD - strange symbolic ref\"));\n-- \n2.39.0.1215.g1ba3f685d4f\n\n"},{"id":"470198","messageId":"patch-v2-6.6-817fe726b4b-20230112T151651Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-v2-0.6-00000000000-20230112T151651Z-avarab@gmail.com","subject":"[PATCH v2 6/6] bisect: no longer try to clean up left-over `.git/head-name` files","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T15:19:14Z","receivedAt":"2023-01-12T15:27:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Schindelin \n\nAs per the code comment, the `.git/head-name` files were cleaned up for\nbackwards-compatibility: an old version of `git bisect` could have left\nthem behind.\n\nNow, just how old would such a version be? As of 0f497e75f05 (Eliminate\nconfusing \"won't bisect on seeked tree\" failure, 2008-02-23), `git\nbisect` does not write that file anymore. Which corresponds to Git\nv1.5.4.4.\n\nEven if the likelihood is non-nil that there might still be users out\nthere who use such an old version to start a bisection, but then decide\nto continue bisecting with a current Git version, it is highly\nimprobable.\n\nSo let's remove that code, at long last.\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n bisect.c | 3 ---\n t/t6030-bisect-porcelain.sh | 1 -\n 2 files changed, 4 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex ec7487e6836..ef5ee5a6436 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -472,7 +472,6 @@ static GIT_PATH_FUNC(git_path_bisect_start, \"BISECT_START\")\n static GIT_PATH_FUNC(git_path_bisect_log, \"BISECT_LOG\")\n static GIT_PATH_FUNC(git_path_bisect_terms, \"BISECT_TERMS\")\n static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n-static GIT_PATH_FUNC(git_path_head_name, \"head-name\")\n \n static void read_bisect_paths(struct strvec *array)\n {\n@@ -1188,8 +1187,6 @@ int bisect_clean_state(void)\n \tunlink_or_warn(git_path_bisect_run());\n \tunlink_or_warn(git_path_bisect_terms());\n \tunlink_or_warn(git_path_bisect_first_parent());\n-\t/* Cleanup head-name if it got left by an old version of git-bisect */\n-\tunlink_or_warn(git_path_head_name());\n \t/*\n \t * Cleanup BISECT_START last to support the --no-checkout option\n \t * introduced in the commit 4796e823a.\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 0a62ea2b3ce..3ba4fdf6153 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -1158,7 +1158,6 @@ test_expect_success 'git bisect reset cleans bisection state properly' '\n \ttest_path_is_missing \".git/BISECT_LOG\" &&\n \ttest_path_is_missing \".git/BISECT_RUN\" &&\n \ttest_path_is_missing \".git/BISECT_TERMS\" &&\n-\ttest_path_is_missing \".git/head-name\" &&\n \ttest_path_is_missing \".git/BISECT_HEAD\" &&\n \ttest_path_is_missing \".git/BISECT_START\"\n '\n-- \n2.39.0.1215.g1ba3f685d4f\n\n"},{"id":"470199","messageId":"patch-v2-4.6-4dda1019767-20230112T151651Z-avarab@gmail.com","threadId":"58956","inReplyTo":"cover-v2-0.6-00000000000-20230112T151651Z-avarab@gmail.com","subject":"[PATCH v2 4/6] bisect run: fix the error message","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T15:19:12Z","receivedAt":"2023-01-12T15:27:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Schindelin \n\nIn d1bbbe45df8 (bisect--helper: reimplement `bisect_run` shell function\nin C, 2021-09-13), we ported the `bisect run` subcommand to C, including\nthe part that prints out an error message when the implicit `git bisect\nbad` or `git bisect good` failed.\n\nHowever, the error message was supposed to print out whether the state\nwas \"good\" or \"bad\", but used a bogus (because non-populated) `args`\nvariable for it. This was fixed in [1], but as of [2] (when\n`bisect--helper` was changed to the present `bisect-state') the error\nmessage still talks about implementation details that should not\nconcern end users.\n\nFix that, and add a regression test to ensure that the intended form of\nthe error message.\n\n1. 80c2e9657f2 (bisect--helper: report actual bisect_state() argument\n on error, 2022-01-18\n2. f37d0bdd42d (bisect: fix output regressions in v2.30.0, 2022-11-10)\n\nHelped-by: Elijah Newren \nSigned-off-by: Johannes Schindelin \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/bisect.c | 2 +-\n t/t6030-bisect-porcelain.sh | 10 ++++++++++\n 2 files changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 9fc8db06944..0786ebf4012 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -1292,7 +1292,7 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n \t\t\tputs(_(\"bisect found first bad commit\"));\n \t\t\tres = BISECT_OK;\n \t\t} else if (res) {\n-\t\t\terror(_(\"bisect run failed: 'bisect-state %s'\"\n+\t\t\terror(_(\"bisect run failed: 'git bisect %s'\"\n \t\t\t\t\" exited with error code %d\"), new_state, res);\n \t\t} else {\n \t\t\tcontinue;\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 9e56b42b5da..0a62ea2b3ce 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -1221,4 +1221,14 @@ test_expect_success 'bisect state output with bad commit' '\n \tgrep -F \"waiting for good commit(s), bad commit known\" output\n '\n \n+test_expect_success 'verify correct error message' '\n+\tgit bisect reset &&\n+\tgit bisect start $HASH4 $HASH1 &&\n+\twrite_script test_script.sh <<-\\EOF &&\n+\trm .git/BISECT*\n+\tEOF\n+\ttest_must_fail git bisect run ./test_script.sh 2>error &&\n+\tgrep \"git bisect good.*exited with error code\" error\n+'\n+\n test_done\n-- \n2.39.0.1215.g1ba3f685d4f\n\n"}]}