git rm bug

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

On Wed, 6 Jun 2018, Thomas Fischer wrote:

  git cannot track empty directories. as that was the *only* content
in that whole hierarchy, the entire hierarchy had to be deleted.

rday

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.

On Wed, 6 Jun 2018, Thomas Fischer wrote:

  why?

  again, why?

  why not?

rday

Thomas Fischer wrote:

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?

On Wed, Jun 06, 2018 at 04:01:38PM -0400, Todd Zullinger wrote:

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).

Yeah, agreed.

-Peff

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 <whatever>"

This would obviate the need for a new flag to switch behaviours.

~ Tim

On Thu, 7 Jun 2018, Timothy Rice wrote:

  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

On Wed, Jun 06 2018, Timothy Rice wrote:

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.

On Wed, Jun 06 2018, Thomas Fischer wrote:

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.

On Wed, Jun 6, 2018 at 9:32 PM, Thomas Fischer <thomasfischer@fastmail.com> wrote:

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.