{"thread":{"id":"41144","subject":"Bug: Branch Deletion Doesn't Clean Up","startedAt":"2016-01-07T19:24:41Z","lastAt":"2016-02-04T08:29:11Z","messageCount":7,"participants":["Karl Moskowski","Jeff King","Mike Hommey"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"275499","messageId":"4AC354E1-9E78-42A8-9828-BF1C084A6CBB@me.com","threadId":"41144","inReplyTo":null,"subject":"Bug: Branch Deletion Doesn't Clean Up","fromName":"Karl Moskowski","fromEmail":"kmoskowski@me.com","sentAt":"2016-01-07T19:24:41Z","receivedAt":"2016-01-07T19:24:41Z","isPatch":false,"sender":{"key":"kmoskowski@me.com","avatar":null},"body":"Apologies if this is a known issue; I was unable to find anything similar in the archive.\n\nThe problem is, deleting a branch whose name contains slashes doesn’t delete the directories in .git/refs/heads/.\n\nI’ve tried both git 2.5.4 and 2.6.4 on OS X 10.11 with HFS+ (case-insensitive, case-preserving). Here’s the sequence of commands:\n\n% mkdir Foobar\n% cd Foobar/\n% git init\n% git checkout -b master\n% touch file.txt\n% git add file.txt\n% git commit -m \"add a file\"\n% git checkout -b another/branch\n\nAt this point, there's file .git/refs/heads/another/branch\n\n% git checkout master\n% git branch -d another/branch\n\nAt this point, there's an empty directory .git/refs/heads/another/. Only the “leaf” file noted above is deleted, not the directory above it.\n\n% git checkout -b Another/branch\n\nAt this point, the file .git/refs/heads/another/branch is re-created. Note the case of its enclosing directory.\n\n% git status\nOn branch Another/branch\nnothing to commit, working directory clean\n\n% git branch -l\n another/branch\n master\n\nNote the difference in case between the last two commands' output. In addition, there's no asterisk next to another/branch indicating it's current.\n\nBecause the first branch deletion didn't also delete the enclosing directory tree below .git/refs/heads/, the new branch gets created beneath it. However, there's inconsistent info between the file system and the git database.\n\nIt seems like git branch -d ascend the hierarchy (up to .git/refs/heads/), deleting any empty directories.\n\n----\nKarl Moskowski \n\n"},{"id":"277371","messageId":"20160204075402.GA21112@sigill.intra.peff.net","threadId":"41144","inReplyTo":"4AC354E1-9E78-42A8-9828-BF1C084A6CBB@me.com","subject":"Re: Bug: Branch Deletion Doesn't Clean Up","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-04T07:54:02Z","receivedAt":"2016-02-04T07:54:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 07, 2016 at 02:24:41PM -0500, Karl Moskowski wrote:\n\n> The problem is, deleting a branch whose name contains slashes doesn’t\n> delete the directories in .git/refs/heads/.\n\nYes. Git's model is to leave the directories in place, and remove them\nonly when they get in the way of creating another ref. In theory, the\neffect is the same as deleting the directories proactively.\n\nBut as you noticed, it does funny things with case-preserving\nfilesystems. It also can cause performance problems if you have a very\nlarge number of empty directories (because git has to open each of them\njust to find out they're empty).\n\nWe can into the latter case at GitHub. Michael Haggerty (cc'd) worked up\nsome patches recently for this, but I don't now if they're yet polished\nenough to send upstream.\n\n> It seems like git branch -d ascend the hierarchy (up to\n> .git/refs/heads/), deleting any empty directories.\n\nYes, though it needs to be coupled with making the branch-creation\nprocess more robust to races (since we might create \"refs/heads/foo\" in\norder to make \"refs/heads/foo/bar\" while somebody else is deleting it to\nget rid of \"refs/heads/foo/baz\").\n\n-Peff\n"},{"id":"277372","messageId":"20160204075500.GA21161@sigill.intra.peff.net","threadId":"41144","inReplyTo":"4AC354E1-9E78-42A8-9828-BF1C084A6CBB@me.com","subject":"Re: Bug: Branch Deletion Doesn't Clean Up","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-04T07:55:00Z","receivedAt":"2016-02-04T07:55:00Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"[resend; sorry, I forgot to cc Michael on the first one]\n\nOn Thu, Jan 07, 2016 at 02:24:41PM -0500, Karl Moskowski wrote:\n\n> The problem is, deleting a branch whose name contains slashes doesn’t\n> delete the directories in .git/refs/heads/.\n\nYes. Git's model is to leave the directories in place, and remove them\nonly when they get in the way of creating another ref. In theory, the\neffect is the same as deleting the directories proactively.\n\nBut as you noticed, it does funny things with case-preserving\nfilesystems. It also can cause performance problems if you have a very\nlarge number of empty directories (because git has to open each of them\njust to find out they're empty).\n\nWe can into the latter case at GitHub. Michael Haggerty (cc'd) worked up\nsome patches recently for this, but I don't now if they're yet polished\nenough to send upstream.\n\n> It seems like git branch -d ascend the hierarchy (up to\n> .git/refs/heads/), deleting any empty directories.\n\nYes, though it needs to be coupled with making the branch-creation\nprocess more robust to races (since we might create \"refs/heads/foo\" in\norder to make \"refs/heads/foo/bar\" while somebody else is deleting it to\nget rid of \"refs/heads/foo/baz\").\n\n-Peff\n"},{"id":"277376","messageId":"20160204081220.GA11434@glandium.org","threadId":"41144","inReplyTo":"20160204075500.GA21161@sigill.intra.peff.net","subject":"Re: Bug: Branch Deletion Doesn't Clean Up","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2016-02-04T08:12:20Z","receivedAt":"2016-02-04T08:12:20Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Thu, Feb 04, 2016 at 02:55:00AM -0500, Jeff King wrote:\n> [resend; sorry, I forgot to cc Michael on the first one]\n> \n> On Thu, Jan 07, 2016 at 02:24:41PM -0500, Karl Moskowski wrote:\n> \n> > The problem is, deleting a branch whose name contains slashes doesn’t\n> > delete the directories in .git/refs/heads/.\n> \n> Yes. Git's model is to leave the directories in place, and remove them\n> only when they get in the way of creating another ref. In theory, the\n> effect is the same as deleting the directories proactively.\n> \n> But as you noticed, it does funny things with case-preserving\n> filesystems. It also can cause performance problems if you have a very\n> large number of empty directories (because git has to open each of them\n> just to find out they're empty).\n> \n> We can into the latter case at GitHub. Michael Haggerty (cc'd) worked up\n> some patches recently for this, but I don't now if they're yet polished\n> enough to send upstream.\n> \n> > It seems like git branch -d ascend the hierarchy (up to\n> > .git/refs/heads/), deleting any empty directories.\n> \n> Yes, though it needs to be coupled with making the branch-creation\n> process more robust to races (since we might create \"refs/heads/foo\" in\n> order to make \"refs/heads/foo/bar\" while somebody else is deleting it to\n> get rid of \"refs/heads/foo/baz\").\n\nCan't we come up with a system that would update packed-refs directly\ninstead of creating files?\n\nMike\n"},{"id":"277377","messageId":"20160204081759.GA23259@sigill.intra.peff.net","threadId":"41144","inReplyTo":"20160204081220.GA11434@glandium.org","subject":"Re: Bug: Branch Deletion Doesn't Clean Up","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-04T08:17:59Z","receivedAt":"2016-02-04T08:17:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 04, 2016 at 05:12:20PM +0900, Mike Hommey wrote:\n\n> > > It seems like git branch -d ascend the hierarchy (up to\n> > > .git/refs/heads/), deleting any empty directories.\n> > \n> > Yes, though it needs to be coupled with making the branch-creation\n> > process more robust to races (since we might create \"refs/heads/foo\" in\n> > order to make \"refs/heads/foo/bar\" while somebody else is deleting it to\n> > get rid of \"refs/heads/foo/baz\").\n> \n> Can't we come up with a system that would update packed-refs directly\n> instead of creating files?\n\nThere are a few reasons not to:\n\n - it breaks backwards compatibility (unless we continue to create the\n directory in order to put the dot-lock in it, but then I don't think\n we've gained anything)\n\n - the usual update method for packed-refs is to take a dot-lock, do a\n whole-file update, and then atomically rename into place. That\n makes writing a ref O(# of refs) instead of O(1), and increases lock\n contention on the packed-refs file.\n\n - if we abandon atomic renames as the update mechanism and just update\n in place via lseek/write, then we need read-locking, or we need to\n hope that a reader will never see a sheared write\n\nBut if we're willing to break compatibility, we should ditch packed-refs\nentirely and move to a _real_ concurrent database. And there is work\nunderway already to do that (see David Turner's ref-backend-lmdb\nseries).\n\n-Peff\n"},{"id":"277378","messageId":"20160204082613.GA14076@glandium.org","threadId":"41144","inReplyTo":"20160204081759.GA23259@sigill.intra.peff.net","subject":"Re: Bug: Branch Deletion Doesn't Clean Up","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2016-02-04T08:26:13Z","receivedAt":"2016-02-04T08:26:13Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Thu, Feb 04, 2016 at 03:17:59AM -0500, Jeff King wrote:\n> On Thu, Feb 04, 2016 at 05:12:20PM +0900, Mike Hommey wrote:\n> \n> > > > It seems like git branch -d ascend the hierarchy (up to\n> > > > .git/refs/heads/), deleting any empty directories.\n> > > \n> > > Yes, though it needs to be coupled with making the branch-creation\n> > > process more robust to races (since we might create \"refs/heads/foo\" in\n> > > order to make \"refs/heads/foo/bar\" while somebody else is deleting it to\n> > > get rid of \"refs/heads/foo/baz\").\n> > \n> > Can't we come up with a system that would update packed-refs directly\n> > instead of creating files?\n> \n> There are a few reasons not to:\n> \n> - it breaks backwards compatibility (unless we continue to create the\n> directory in order to put the dot-lock in it, but then I don't think\n> we've gained anything)\n\nIs that the kind of backwards compatibility that matters, though? I\nmean, I won't claim to know all the internals of how refs are used, but\nyou sound like the theoretical incompatibility would be two different\nversions of git racing for update-ref on the same local repository.\n\nNot that it would change anything about the other reasons below.\n\n> - the usual update method for packed-refs is to take a dot-lock, do a\n> whole-file update, and then atomically rename into place. That\n> makes writing a ref O(# of refs) instead of O(1), and increases lock\n> contention on the packed-refs file.\n> \n> - if we abandon atomic renames as the update mechanism and just update\n> in place via lseek/write, then we need read-locking, or we need to\n> hope that a reader will never see a sheared write\n> \n> But if we're willing to break compatibility, we should ditch packed-refs\n> entirely and move to a _real_ concurrent database. And there is work\n> underway already to do that (see David Turner's ref-backend-lmdb\n> series).\n\nMike\n"},{"id":"277379","messageId":"20160204082911.GA23479@sigill.intra.peff.net","threadId":"41144","inReplyTo":"20160204082613.GA14076@glandium.org","subject":"Re: Bug: Branch Deletion Doesn't Clean Up","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-04T08:29:11Z","receivedAt":"2016-02-04T08:29:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 04, 2016 at 05:26:13PM +0900, Mike Hommey wrote:\n\n> > There are a few reasons not to:\n> > \n> > - it breaks backwards compatibility (unless we continue to create the\n> > directory in order to put the dot-lock in it, but then I don't think\n> > we've gained anything)\n> \n> Is that the kind of backwards compatibility that matters, though? I\n> mean, I won't claim to know all the internals of how refs are used, but\n> you sound like the theoretical incompatibility would be two different\n> versions of git racing for update-ref on the same local repository.\n\nYes, that's true. It's a lesser breakage than \"you cannot go back to the\nold version at all\".\n\n-Peff\n"}]}