{"thread":{"id":"60512","subject":"git-bisect reset not deleting .git/BISECT_LOG","startedAt":"2023-11-13T20:52:16Z","lastAt":"2023-12-07T06:53:42Z","messageCount":3,"participants":["Janik Haag","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"484814","messageId":"bef9d5b3-bb64-4662-8952-d000872c5244@aq0.de","threadId":"60512","inReplyTo":null,"subject":"git-bisect reset not deleting .git/BISECT_LOG","fromName":"Janik Haag","fromEmail":"janik@aq0.de","sentAt":"2023-11-13T20:42:09Z","receivedAt":"2023-11-13T20:52:16Z","isPatch":false,"sender":{"key":"janik@aq0.de","avatar":null},"body":"Thank you for filling out a Git bug report!\nPlease answer the following questions to help us understand your issue.\n\nWhat did you do before the bug happened? (Steps to reproduce your issue)\n```bash\ngit init /tmp/reproduce-bisect-warning\ncd /tmp/reproduce-bisect-warning\ntouch .git/BISECT_LOG\ngit bisect reset\ngit switch -c log\n```\n\nWhat did you expect to happen? (Expected behavior)\n\n`git-bisect reset` should have deleted .git/BISECT_LOG\n\nWhat happened instead? (Actual behavior)\n\n.git/BISECT_LOG is still there\n\nWhat's different between what you expected and what actually happened?\n\ngit switch and some external tools like shell prompts are relying on \nthat file to know if we are doing a bisect so switch printed a warning \nthat we are still bisecting. Funnily enough git checkout doesn't print \nthat warning.\n\nAnything else you want to add:\n\nIn case you are wondering why I created the .git/BISECT_LOG file with \ntouch instead of using the git cli, I forgot how I ended up there in the \nfirst place that was like a year ago and the warning didn't really \nbother me, today someone told me to look for anything in .git namend \nBISECT* and after deleting the log by hand it worked. So I figured \ngit-bisect reset should probably have done that after I ended up in that \nunkown state. the content I had in BISECT_LOG before deleting it was:\n```\n# good: [e24028141f2] qdmr: fixup\ngit bisect good e24028141f278590407dd5389330f23d01c89cff\n```\n\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n\n[System Info]\ngit version:\ngit version 2.42.0\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: \n/nix/store/lf0wpjrj8yx4gsmw2s3xfl58ixmqk8qa-bash-5.2-p15/bin/bash\nuname: Linux 6.1.61 #1-NixOS SMP PREEMPT_DYNAMIC Thu Nov  2 08:35:33 UTC \n2023 x86_64\ncompiler info: gnuc: 12.3\nlibc info: glibc: 2.38\n$SHELL (typically, interactive shell): /run/current-system/sw/bin/zsh\n\n\n[Enabled Hooks]\n\n"},{"id":"484816","messageId":"20231113210820.GB2028092@coredump.intra.peff.net","threadId":"60512","inReplyTo":"bef9d5b3-bb64-4662-8952-d000872c5244@aq0.de","subject":"Re: git-bisect reset not deleting .git/BISECT_LOG","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-13T21:08:20Z","receivedAt":"2023-11-13T21:08:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 13, 2023 at 09:42:09PM +0100, Janik Haag wrote:\n\n> Thank you for filling out a Git bug report!\n> Please answer the following questions to help us understand your issue.\n> \n> What did you do before the bug happened? (Steps to reproduce your issue)\n> ```bash\n> git init /tmp/reproduce-bisect-warning\n> cd /tmp/reproduce-bisect-warning\n> touch .git/BISECT_LOG\n> git bisect reset\n> git switch -c log\n> ```\n> \n> What did you expect to happen? (Expected behavior)\n> \n> `git-bisect reset` should have deleted .git/BISECT_LOG\n> \n> What happened instead? (Actual behavior)\n> \n> .git/BISECT_LOG is still there\n\nI don't think this is really specific to BISECT_LOG. In \"bisect reset\",\nwe'll call bisect_clean_state(), which removes a bunch of files. The\nproblem in your example is that \"bisect reset\" sees that we are not\nbisecting and bails early.\n\nWhich I can kind of see, as part of the \"reset\" process is to reset to\nthe original pre-bisect commit. If it's not given on the command line,\nthe default is to use BISECT_START, which of course does not exist.\n\n  As a side note, \"git bisect reset HEAD\" will do what you want, because\n  it skips the BISECT_START check. But obviously that's not something\n  normal users should need to know about, and I think is even an\n  accidental side-effect of the conversion in 5e82c3dd22\n  (bisect--helper: `bisect_reset` shell function in C, 2019-01-02).\n\nSo really, you just want the \"clean\" part of \"bisect reset\", but not the\n\"reset\". We could have a separate \"bisect clean\" that would do what you\nwant (clean state without trying to reset HEAD). But I don't think it\nwould be unreasonable to \"reset\" to just unconditionally clean. I think\nit would probably just be a few lines in bisect_reset() to avoid the\nearly return, and skip the call to \"checkout\" when we have no branch to\ngo back to.\n\nMaybe a good simple patch for somebody interested in getting into Git\ndevelopment?\n\n-Peff\n"},{"id":"485413","messageId":"20231207065341.GA778781@coredump.intra.peff.net","threadId":"60512","inReplyTo":"20231113210820.GB2028092@coredump.intra.peff.net","subject":"[PATCH] bisect: always clean on reset","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-12-07T06:53:41Z","receivedAt":"2023-12-07T06:53:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 13, 2023 at 04:08:20PM -0500, Jeff King wrote:\n\n> So really, you just want the \"clean\" part of \"bisect reset\", but not the\n> \"reset\". We could have a separate \"bisect clean\" that would do what you\n> want (clean state without trying to reset HEAD). But I don't think it\n> would be unreasonable to \"reset\" to just unconditionally clean. I think\n> it would probably just be a few lines in bisect_reset() to avoid the\n> early return, and skip the call to \"checkout\" when we have no branch to\n> go back to.\n> \n> Maybe a good simple patch for somebody interested in getting into Git\n> development?\n\nI didn't want to forget about this, so I rolled it up into a patch.\n\n-- >8 --\nSubject: [PATCH] bisect: always clean on reset\n\nUsually \"bisect reset\" cleans up any refs/bisect/ refs, along with\nmeta-files like .git/BISECT_LOG. But it only does so after deciding that\na bisection is active, which it does by reading BISECT_START. This is\nusually fine, but it's possible to get into a confusing state if the\nBISECT_START file is gone, but other cruft is left (this might be due to\na bug, or a system crash, etc).\n\nAnd since \"bisect reset\" refuses to do anything in this state, the user\nhas no easy way to clean up the leftover cruft. While another \"bisect\nstart\" would clear the state, in the interim it can be annoying, as\nother tools (like our bash prompt code) think we are bisecting, and\nfor-each-ref output may be polluted with refs/bisect/ entries.\n\nFurther adding to the confusion is that running \"bisect reset $some_ref\"\nskips the BISECT_START check. So it never realizes that there's no\nbisection active and does the cleanup anyway!\n\nSo let's just make sure we always do the cleanup, whether we looked at\nBISECT_START or not. If the user doesn't give us a commit to reset to,\nwe'll still say \"We are not bisecting\" and skip the call to \"git\ncheckout\".\n\nReported-by: Janik Haag <janik@aq0.de>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/bisect.c            | 9 ++++-----\n t/t6030-bisect-porcelain.sh | 6 ++++++\n 2 files changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 35938b05fd..c5565686bf 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -233,11 +233,10 @@ static int bisect_reset(const char *commit)\n \tstruct strbuf branch = STRBUF_INIT;\n \n \tif (!commit) {\n-\t\tif (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1) {\n+\t\tif (!strbuf_read_file(&branch, git_path_bisect_start(), 0))\n \t\t\tprintf(_(\"We are not bisecting.\\n\"));\n-\t\t\treturn 0;\n-\t\t}\n-\t\tstrbuf_rtrim(&branch);\n+\t\telse\n+\t\t\tstrbuf_rtrim(&branch);\n \t} else {\n \t\tstruct object_id oid;\n \n@@ -246,7 +245,7 @@ static int bisect_reset(const char *commit)\n \t\tstrbuf_addstr(&branch, commit);\n \t}\n \n-\tif (!ref_exists(\"BISECT_HEAD\")) {\n+\tif (branch.len && !ref_exists(\"BISECT_HEAD\")) {\n \t\tstruct child_process cmd = CHILD_PROCESS_INIT;\n \n \t\tcmd.git_cmd = 1;\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 2a5b7d8379..7b24d1684e 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -170,6 +170,12 @@ test_expect_success 'bisect reset when not bisecting' '\n \tcmp branch.expect branch.output\n '\n \n+test_expect_success 'bisect reset cleans up even when not bisecting' '\n+\techo garbage >.git/BISECT_LOG &&\n+\tgit bisect reset &&\n+\ttest_path_is_missing .git/BISECT_LOG\n+'\n+\n test_expect_success 'bisect reset removes packed refs' '\n \tgit bisect reset &&\n \tgit bisect start &&\n-- \n2.43.0.664.ga12c899002\n\n"}]}