# git rm bug 11 messages from 2018-06-06 to 2018-06-06. Participants: Thomas Fischer, Robert P. J. Day, Duy Nguyen, Todd Zullinger, Jeff King, Timothy Rice, Ævar Arnfjörð Bjarmason. Thread: https://gitlist.dev/t/48664 ## Thomas Fischer, 2018-06-06 19:32 Subject: git rm bug Message-ID: <1528313557.2662346.1398855328.3E0F8023@webmail.messagingengine.com> URL: https://gitlist.dev/e/1528313557.2662346.1398855328.3E0F8023%40webmail.messagingengine.com ``` OVERVIEW "git rm" will remove more files than specified. This is either a bug or undocumented behavior (not in the man pages). SETUP 1. In a git repository, create an empty directory OR a chain of empty directories $ mkdir -p path/to/some/ 2. Create a file in the deepest directory and add it to tracking $ touch path/to/some/file $ git add path/to/some/file $ git commit -m 'add path/to/some/file' THE BUG Run 'git rm' on the tracked file. EXPECTED BEHAVIOR $ git rm path/to/some/file rm 'path/to/some/file' $ ls path to/ $ ls path/to some/ Note that path/, path/to/, and path/to/some/ still exist. ACTUAL BEHAVIOR $ git rm path/to/some/file rm 'path/to/some/file' $ ls path ls: cannot access 'path': No such file or directory The entire chain of empty directories is removed, despite the fact the git outputs only "rm 'path/to/some/file'". This ONLY occurs when all the directories in the chain are empty after the tracked file has been removed. This behavior is NOT documented in the man pages. I propose that 'rmdir' statements are added to 'git rm' output, or that the man pages be updated to reflect this behavior. Best, Thomas Fischer ``` ## Robert P. J. Day, 2018-06-06 19:33 Subject: Re: git rm bug Message-ID: URL: https://gitlist.dev/e/alpine.LFD.2.21.1806061532420.9259%40localhost.localdomain In-Reply-To: <1528313557.2662346.1398855328.3E0F8023@webmail.messagingengine.com> ``` On Wed, 6 Jun 2018, Thomas Fischer wrote: > OVERVIEW > > "git rm" will remove more files than specified. This is either a bug or undocumented behavior (not in the man pages). > > SETUP > > 1. In a git repository, create an empty directory OR a chain of empty directories > > $ mkdir -p path/to/some/ > > 2. Create a file in the deepest directory and add it to tracking > > $ touch path/to/some/file > $ git add path/to/some/file > $ git commit -m 'add path/to/some/file' > > THE BUG > > Run 'git rm' on the tracked file. > > EXPECTED BEHAVIOR > > $ git rm path/to/some/file > rm 'path/to/some/file' > $ ls path > to/ > $ ls path/to > some/ > > Note that path/, path/to/, and path/to/some/ still exist. > > ACTUAL BEHAVIOR > > $ git rm path/to/some/file > rm 'path/to/some/file' > $ ls path > ls: cannot access 'path': No such file or directory > > The entire chain of empty directories is removed, despite the fact > the git outputs only "rm 'path/to/some/file'". git cannot track empty directories. as that was the *only* content in that whole hierarchy, the entire hierarchy had to be deleted. rday ``` ## Thomas Fischer, 2018-06-06 19:47 Subject: Re: git rm bug Message-ID: <1528314421.2665575.1398886712.579F37CE@webmail.messagingengine.com> URL: https://gitlist.dev/e/1528314421.2665575.1398886712.579F37CE%40webmail.messagingengine.com In-Reply-To: ``` I agree that the entire chain of empty directories should not be tracked, as git tracks content, not files. However, when I run 'rm path/to/some/file', I expect path/to/some/ to still exist. Similarly, when I run 'git rm path/to/some/file', I expect path/to/some/ to exist, *albeit untracked*. I do NOT expect git to *track* empty directories. But I also do NOT expect it to remove untracked directories. -- Thomas Fischer thomasfischer@fastmail.com On Wed, Jun 6, 2018, at 2:33 PM, Robert P. J. Day wrote: > On Wed, 6 Jun 2018, Thomas Fischer wrote: > > > OVERVIEW > > > > "git rm" will remove more files than specified. This is either a bug or undocumented behavior (not in the man pages). > > > > SETUP > > > > 1. In a git repository, create an empty directory OR a chain of empty directories > > > > $ mkdir -p path/to/some/ > > > > 2. Create a file in the deepest directory and add it to tracking > > > > $ touch path/to/some/file > > $ git add path/to/some/file > > $ git commit -m 'add path/to/some/file' > > > > THE BUG > > > > Run 'git rm' on the tracked file. > > > > EXPECTED BEHAVIOR > > > > $ git rm path/to/some/file > > rm 'path/to/some/file' > > $ ls path > > to/ > > $ ls path/to > > some/ > > > > Note that path/, path/to/, and path/to/some/ still exist. > > > > ACTUAL BEHAVIOR > > > > $ git rm path/to/some/file > > rm 'path/to/some/file' > > $ ls path > > ls: cannot access 'path': No such file or directory > > > > The entire chain of empty directories is removed, despite the fact > > the git outputs only "rm 'path/to/some/file'". > > git cannot track empty directories. as that was the *only* content > in that whole hierarchy, the entire hierarchy had to be deleted. > > rday ``` ## Robert P. J. Day, 2018-06-06 19:51 Subject: Re: git rm bug Message-ID: URL: https://gitlist.dev/e/alpine.LFD.2.21.1806061550530.9562%40localhost.localdomain In-Reply-To: <1528314421.2665575.1398886712.579F37CE@webmail.messagingengine.com> ``` On Wed, 6 Jun 2018, Thomas Fischer wrote: > I agree that the entire chain of empty directories should not be > tracked, as git tracks content, not files. > > However, when I run 'rm path/to/some/file', I expect path/to/some/ > to still exist. why? > Similarly, when I run 'git rm path/to/some/file', I expect > path/to/some/ to exist, *albeit untracked*. again, why? > I do NOT expect git to *track* empty directories. But I also do NOT > expect it to remove untracked directories. why not? rday -- ======================================================================== Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca/dokuwiki Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ======================================================================== ``` ## Duy Nguyen, 2018-06-06 19:54 Subject: Re: git rm bug Message-ID: URL: https://gitlist.dev/e/CACsJy8CpsF8rtiJVtLK4d%3D7RdtPBy5QRy7XURFvpaWtQCynJkA%40mail.gmail.com In-Reply-To: <1528313557.2662346.1398855328.3E0F8023@webmail.messagingengine.com> ``` On Wed, Jun 6, 2018 at 9:32 PM, Thomas Fischer wrote: > OVERVIEW > > "git rm" will remove more files than specified. This is either a bug or undocumented behavior (not in the man pages). The behavior is intended, with a question mark. This change is introduced in d9b814cc97 (Add builtin "git rm" command - 2006-05-19). I quote the relevant paragraph from that commit The other question is what to do with leading directories. The old "git rm" script didn't do anything, which is somewhat inconsistent. This one will actually clean up directories that have become empty as a result of removing the last file, but maybe we want to have a flag to decide the behaviour? To me we definitely should document this (patches welcome!) then maybe revisit this "have a flag to decide the behavior" question from 12 years ago. > SETUP > > 1. In a git repository, create an empty directory OR a chain of empty directories > > $ mkdir -p path/to/some/ > > 2. Create a file in the deepest directory and add it to tracking > > $ touch path/to/some/file > $ git add path/to/some/file > $ git commit -m 'add path/to/some/file' > > THE BUG > > Run 'git rm' on the tracked file. > > EXPECTED BEHAVIOR > > $ git rm path/to/some/file > rm 'path/to/some/file' > $ ls path > to/ > $ ls path/to > some/ > > Note that path/, path/to/, and path/to/some/ still exist. > > ACTUAL BEHAVIOR > > $ git rm path/to/some/file > rm 'path/to/some/file' > $ ls path > ls: cannot access 'path': No such file or directory > > The entire chain of empty directories is removed, despite the fact the git outputs only "rm 'path/to/some/file'". > > This ONLY occurs when all the directories in the chain are empty after the tracked file has been removed. > > This behavior is NOT documented in the man pages. > > I propose that 'rmdir' statements are added to 'git rm' output, or that the man pages be updated to reflect this behavior. > > Best, > Thomas Fischer -- Duy ``` ## Todd Zullinger, 2018-06-06 20:01 Subject: Re: git rm bug Message-ID: <20180606200137.GJ3094@zaya.teonanacatl.net> URL: https://gitlist.dev/e/20180606200137.GJ3094%40zaya.teonanacatl.net In-Reply-To: <1528314421.2665575.1398886712.579F37CE@webmail.messagingengine.com> ``` Thomas Fischer wrote: > I agree that the entire chain of empty directories should not be tracked, as git tracks content, not files. > > However, when I run 'rm path/to/some/file', I expect path/to/some/ to still exist. > > Similarly, when I run 'git rm path/to/some/file', I expect path/to/some/ to exist, *albeit untracked*. > > I do NOT expect git to *track* empty directories. But I also do NOT expect it to remove untracked directories. It looks like this behavior has been in place for many years, since d9b814cc97 ("Add builtin "git rm" command", 2006-05-19). Interestingly, Linus noted in the commit message that the removal of leading directories was different than when git-rm was a shell script. And he wondered if it might be worth having an option to control that behavior. I imagine that most users either want the current behavior or they rarely run across this and are surprised, given how long git rm has worked this way. It does seem like something which could be noted in the git rm docs. Perhaps you'd care to take a stab at a patch to add a note to Documentation/git-rm.txt Thomas? Maybe a note at the end of the DISCUSSION section? -- Todd ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ If everything seems to be going well, you have obviously overlooked something. ``` ## Jeff King, 2018-06-06 20:11 Subject: Re: git rm bug Message-ID: <20180606201144.GA32098@sigill.intra.peff.net> URL: https://gitlist.dev/e/20180606201144.GA32098%40sigill.intra.peff.net In-Reply-To: <20180606200137.GJ3094@zaya.teonanacatl.net> ``` On Wed, Jun 06, 2018 at 04:01:38PM -0400, Todd Zullinger wrote: > Thomas Fischer wrote: > > I agree that the entire chain of empty directories should not be tracked, as git tracks content, not files. > > > > However, when I run 'rm path/to/some/file', I expect path/to/some/ to still exist. > > > > Similarly, when I run 'git rm path/to/some/file', I expect path/to/some/ to exist, *albeit untracked*. > > > > I do NOT expect git to *track* empty directories. But I also do NOT expect it to remove untracked directories. > > It looks like this behavior has been in place for many > years, since d9b814cc97 ("Add builtin "git rm" command", > 2006-05-19). Interestingly, Linus noted in the commit > message that the removal of leading directories was > different than when git-rm was a shell script. And he > wondered if it might be worth having an option to control > that behavior. > > I imagine that most users either want the current behavior > or they rarely run across this and are surprised, given how > long git rm has worked this way. It's also consistent with other parts of Git that remove files. E.g., "git checkout" to a state that does not have the file will remove the leading directories (if they're empty, of course). > It does seem like something which could be noted in the git > rm docs. Perhaps you'd care to take a stab at a patch to > add a note to Documentation/git-rm.txt Thomas? Maybe a note > at the end of the DISCUSSION section? Yeah, agreed. -Peff ``` ## Timothy Rice, 2018-06-06 20:10 Subject: Re: git rm bug Message-ID: <20180606201012.GA2454@sleipnir> URL: https://gitlist.dev/e/20180606201012.GA2454%40sleipnir In-Reply-To: <20180606200137.GJ3094@zaya.teonanacatl.net> ``` > It does seem like something which could be noted in the git > rm docs. Perhaps you'd care to take a stab at a patch to > add a note to Documentation/git-rm.txt Thomas? Maybe a note > at the end of the DISCUSSION section? That same documentation could mention a common workaround for when someone does really want to keep the empty directories: $ touch ./path/to/empty/dir/.keep $ git add ./path/to/empty/dir/.keep $ git commit -m "Keep that empty directory because it is needed for " This would obviate the need for a new flag to switch behaviours. ~ Tim ``` ## Robert P. J. Day, 2018-06-06 20:20 Subject: Re: git rm bug Message-ID: URL: https://gitlist.dev/e/alpine.LFD.2.21.1806061609440.10418%40localhost.localdomain In-Reply-To: <20180606201012.GA2454@sleipnir> ``` On Thu, 7 Jun 2018, Timothy Rice wrote: > > It does seem like something which could be noted in the git > > rm docs. Perhaps you'd care to take a stab at a patch to > > add a note to Documentation/git-rm.txt Thomas? Maybe a note > > at the end of the DISCUSSION section? > > That same documentation could mention a common workaround for when > someone does really want to keep the empty directories: > > $ touch ./path/to/empty/dir/.keep > $ git add ./path/to/empty/dir/.keep > $ git commit -m "Keep that empty directory because it is needed for " > > This would obviate the need for a new flag to switch behaviours. i'm going to be contrarian and obstinate and suggest that the current behaviour is fine, since there is no compelling rationale for any *other* behaviour. invariably, every defense for hanging on to empty directories boils down to, "i might do something in the future that expects those directories to exist." well, if that's the case, then create them when you need them -- nothing you do should ever simply *assume* the existence of essential directories. in addition, by "untracking" those directories, you're suggesting that git quietly do what should normally be done by "git rm --cached". if i want that behaviour, i would prefer to have to type it myself. i'm fine with just adding a note to "git rm" making it clear what happens in this case. i see no value in supporting extra options that simply encourage bad behaviour. rday -- ======================================================================== Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca/dokuwiki Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ======================================================================== ``` ## Ævar Arnfjörð Bjarmason, 2018-06-06 22:50 Subject: Re: git rm bug Message-ID: <87bmcnzg0l.fsf@evledraar.gmail.com> URL: https://gitlist.dev/e/87bmcnzg0l.fsf%40evledraar.gmail.com In-Reply-To: <1528314421.2665575.1398886712.579F37CE@webmail.messagingengine.com> ``` On Wed, Jun 06 2018, Thomas Fischer wrote: > I agree that the entire chain of empty directories should not be > tracked, as git tracks content, not files. > > However, when I run 'rm path/to/some/file', I expect path/to/some/ to > still exist. > > Similarly, when I run 'git rm path/to/some/file', I expect > path/to/some/ to exist, *albeit untracked*. > > I do NOT expect git to *track* empty directories. But I also do NOT > expect it to remove untracked directories. Others have said why, but here's an edge case you probably haven't thought of: ( rm -rf /tmp/repo && git init /tmp/repo && cd /tmp/repo && mkdir -p foo/bar/baz && git status ) If you just have empty directories "git status" will report nothing, although "git clean -dxfn" will show what would be cleaned up. So if this worked as you're suggesting then someone could 'git rm" some file, then everything would report that they're on commit XYZ, but if they re-cloned at that commit they'd get a tree that would look different. No git command should behave in such a way as to leave the tree in a state when moving from commit X->Y that you wouldn't get the same Y if you re-cloned. ``` ## Ævar Arnfjörð Bjarmason, 2018-06-06 22:51 Subject: Re: git rm bug Message-ID: <87a7s7zfz0.fsf@evledraar.gmail.com> URL: https://gitlist.dev/e/87a7s7zfz0.fsf%40evledraar.gmail.com In-Reply-To: <20180606201012.GA2454@sleipnir> ``` On Wed, Jun 06 2018, Timothy Rice wrote: >> It does seem like something which could be noted in the git >> rm docs. Perhaps you'd care to take a stab at a patch to >> add a note to Documentation/git-rm.txt Thomas? Maybe a note >> at the end of the DISCUSSION section? > > That same documentation could mention a common workaround for when someone > does really want to keep the empty directories: > > $ touch ./path/to/empty/dir/.keep > $ git add ./path/to/empty/dir/.keep > $ git commit -m "Keep that empty directory because it is needed for " nit: The .gitkeep convention seems to be much more common in the wild than .keep, and it's probably time we documented that somewhere, even though it's not a magic .git* file like .gitattributes et al. ```