{"thread":{"id":"41659","subject":"Bug: git branch -D can be used to delete branch which is currently checked out - Part 2","startedAt":"2016-03-10T08:39:56Z","lastAt":"2016-03-10T22:43:59Z","messageCount":6,"participants":["Marcus Kida","Johannes Schindelin","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"280535","messageId":"F6588CBF-D13E-475F-847F-6154370F4A2E@gmail.com","threadId":"41659","inReplyTo":null,"subject":"Bug: git branch -D can be used to delete branch which is currently checked out - Part 2","fromName":"Marcus Kida","fromEmail":"marcus.kida@gmail.com","sentAt":"2016-03-10T08:39:56Z","receivedAt":"2016-03-10T08:39:56Z","isPatch":false,"sender":{"key":"marcus.kida@gmail.com","avatar":"https://gravatar.com/avatar/5dfe073efa74b6a101366eb3bc2fd074b0809e203205237ac57514ece7871a84?d=mp&s=160"},"body":"Proposed solution:\n\nUse `strcasecmp`, `stricmp`, `strcmpi` here: https://github.com/git/git/blob/f02fbc4f9433937ee0463d0342d6d7d97e1f6f1e/builtin/branch.c#L218\n\nNot sure if/which one of this will work on POSIX as well as MS too though.\n\nThank you.\n\nCheers,\nMarcus"},{"id":"280551","messageId":"alpine.DEB.2.20.1603101313220.4690@virtualbox","threadId":"41659","inReplyTo":"F6588CBF-D13E-475F-847F-6154370F4A2E@gmail.com","subject":"Re: Bug: git branch -D can be used to delete branch which is currently checked out - Part 2","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-03-10T12:15:16Z","receivedAt":"2016-03-10T12:15:16Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Marcus,\n\nOn Thu, 10 Mar 2016, Marcus Kida wrote:\n\n> Proposed solution:\n> \n> Use `strcasecmp`, `stricmp`, `strcmpi` here: https://github.com/git/git/blob/f02fbc4f9433937ee0463d0342d6d7d97e1f6f1e/builtin/branch.c#L218\n> \n> Not sure if/which one of this will work on POSIX as well as MS too though.\n\nThis is not quite a solution (it is not a patch). And you *definitely*\nwant to make the use of strcasecmp() contingent on ignore_case. You are\nnot alone on this world, after all, and other people have case-sensitive\nfilesystems. It's totally doable, of course.\n\nSo, do you feel up to the task?\n\nCiao,\nJohannes\n"},{"id":"280565","messageId":"9EFA8642-FA02-4ADB-B37B-530E611C9C1F@gmail.com","threadId":"41659","inReplyTo":"alpine.DEB.2.20.1603101313220.4690@virtualbox","subject":"Re: Bug: git branch -D can be used to delete branch which is currently checked out - Part 2","fromName":"Marcus Kida","fromEmail":"marcus.kida@gmail.com","sentAt":"2016-03-10T18:30:00Z","receivedAt":"2016-03-10T18:30:00Z","isPatch":false,"sender":{"key":"marcus.kida@gmail.com","avatar":"https://gravatar.com/avatar/5dfe073efa74b6a101366eb3bc2fd074b0809e203205237ac57514ece7871a84?d=mp&s=160"},"body":"Hi Johannes,\n\nthank you for the feedback.\nI will fix this, test it and send a patch.\n\nCheers,\nMarcus\n\n> On 10 Mar 2016, at 11:15 PM, Johannes Schindelin wrote:\n> \n> Hi Marcus,\n> \n>> On Thu, 10 Mar 2016, Marcus Kida wrote:\n>> \n>> Proposed solution:\n>> \n>> Use `strcasecmp`, `stricmp`, `strcmpi` here: https://github.com/git/git/blob/f02fbc4f9433937ee0463d0342d6d7d97e1f6f1e/builtin/branch.c#L218\n>> \n>> Not sure if/which one of this will work on POSIX as well as MS too though.\n> \n> This is not quite a solution (it is not a patch). And you *definitely*\n> want to make the use of strcasecmp() contingent on ignore_case. You are\n> not alone on this world, after all, and other people have case-sensitive\n> filesystems. It's totally doable, of course.\n> \n> So, do you feel up to the task?\n> \n> Ciao,\n> Johannes\n"},{"id":"280574","messageId":"41E8F374-9E8A-48AE-A5D5-A3C24B79ED7E@gmail.com","threadId":"41659","inReplyTo":"alpine.DEB.2.20.1603101313220.4690@virtualbox","subject":"Re: Bug: git branch -D can be used to delete branch which is currently checked out - Part 2","fromName":"Marcus Kida","fromEmail":"marcus.kida@gmail.com","sentAt":"2016-03-10T21:20:38Z","receivedAt":"2016-03-10T21:20:38Z","isPatch":false,"sender":{"key":"marcus.kida@gmail.com","avatar":"https://gravatar.com/avatar/5dfe073efa74b6a101366eb3bc2fd074b0809e203205237ac57514ece7871a84?d=mp&s=160"},"body":"Hi Johannes,\nHi Junio,\n\nhere you’ll find a patch to hotfix the “delete-a-branch-you’re-on\" issue.\nAs Junio already stated there’s many more places where case (in)sensitivity is not handled correctly but this patch would at least prevent you from deleting the branch you’re currently working on (which happened to me yesterday and lead to quite some additional stress).\n\nIf you think this isn’t worth patching now, please discard my patch.\nIf you think it’s worth patching and implementing a more advanced filesystem backend at a later time, please apply it.\n\nIf tested this and validate it works @ 2.8.0-rc1. \n\nThank you.\n\n---\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 7b45b6b..46bde61 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -215,7 +215,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\tint flags = 0;\n \n \t\tstrbuf_branchname(&bname, argv[i]);\n-\t\tif (kinds == FILTER_REFS_BRANCHES && !strcmp(head, bname.buf)) {\n+\t\tif (kinds == FILTER_REFS_BRANCHES && !strcasecmp(head, bname.buf)) {\n \t\t\terror(_(\"Cannot delete the branch '%s' \"\n \t\t\t \"which you are currently on.\"), bname.buf);\n \t\t\tret = 1;\n---\n\nCheers,\nMarcus\n\n> On 10 Mar 2016, at 11:15 PM, Johannes Schindelin wrote:\n> \n> Hi Marcus,\n> \n> On Thu, 10 Mar 2016, Marcus Kida wrote:\n> \n>> Proposed solution:\n>> \n>> Use `strcasecmp`, `stricmp`, `strcmpi` here: https://github.com/git/git/blob/f02fbc4f9433937ee0463d0342d6d7d97e1f6f1e/builtin/branch.c#L218\n>> \n>> Not sure if/which one of this will work on POSIX as well as MS too though.\n> \n> This is not quite a solution (it is not a patch). And you *definitely*\n> want to make the use of strcasecmp() contingent on ignore_case. You are\n> not alone on this world, after all, and other people have case-sensitive\n> filesystems. It's totally doable, of course.\n> \n> So, do you feel up to the task?\n> \n> Ciao,\n> Johannes\n"},{"id":"280588","messageId":"20160310224158.GA2772@sigill.intra.peff.net","threadId":"41659","inReplyTo":"9EFA8642-FA02-4ADB-B37B-530E611C9C1F@gmail.com","subject":"Re: Bug: git branch -D can be used to delete branch which is currently checked out - Part 2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-10T22:41:58Z","receivedAt":"2016-03-10T22:41:58Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 11, 2016 at 05:30:00AM +1100, Marcus Kida wrote:\n\n> thank you for the feedback.\n> I will fix this, test it and send a patch.\n\nUnfortunately, I think this issue is a little more complicated.\n\nThere's some prior discussion in\n\n http://thread.gmane.org/gmane.comp.version-control.git/284022\n\nand\n\n http://thread.gmane.org/gmane.comp.version-control.git/276456/focus=276506\n\nThe latter, in particular, shows a case where this approach will do the\nwrong thing. The fundamental issue is that refs are potentially stored\nin _two_ places: the filesystem, and the packed-refs file. And the\nlatter is always case-sensitive, while the former sometimes is and\nsometimes isn't. But because the storage all happens behind the scenes,\nthe user has no way of reliably disambiguating (e.g., does \"foo\" refer\nto your checked-out \"FOO\", or are you intentionally trying to delete an\nextraneous \"FOO\" that ended up in the packed-refs file?).\n\n-Peff\n"},{"id":"280589","messageId":"02FB9587-5849-4B3A-A199-6483377C3EED@gmail.com","threadId":"41659","inReplyTo":"20160310224158.GA2772@sigill.intra.peff.net","subject":"Re: Bug: git branch -D can be used to delete branch which is currently checked out - Part 2","fromName":"Marcus Kida","fromEmail":"marcus.kida@gmail.com","sentAt":"2016-03-10T22:43:59Z","receivedAt":"2016-03-10T22:43:59Z","isPatch":false,"sender":{"key":"marcus.kida@gmail.com","avatar":"https://gravatar.com/avatar/5dfe073efa74b6a101366eb3bc2fd074b0809e203205237ac57514ece7871a84?d=mp&s=160"},"body":"Fair enough,\n\nthank you. I’m going to take a look at the previous threads.\nI’d also be keen to help working on those issues.\n\n> On 11 Mar 2016, at 9:41 AM, Jeff King wrote:\n> \n> On Fri, Mar 11, 2016 at 05:30:00AM +1100, Marcus Kida wrote:\n> \n>> thank you for the feedback.\n>> I will fix this, test it and send a patch.\n> \n> Unfortunately, I think this issue is a little more complicated.\n> \n> There's some prior discussion in\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/284022\n> \n> and\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/276456/focus=276506\n> \n> The latter, in particular, shows a case where this approach will do the\n> wrong thing. The fundamental issue is that refs are potentially stored\n> in _two_ places: the filesystem, and the packed-refs file. And the\n> latter is always case-sensitive, while the former sometimes is and\n> sometimes isn't. But because the storage all happens behind the scenes,\n> the user has no way of reliably disambiguating (e.g., does \"foo\" refer\n> to your checked-out \"FOO\", or are you intentionally trying to delete an\n> extraneous \"FOO\" that ended up in the packed-refs file?).\n> \n> -Peff\n"}]}