git rm bug

git rm bug

[Thomas Fischer]

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

Re: git rm bug

[Robert P. J. Day]

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

Re: git rm bug

[Thomas Fischer]

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

Re: git rm bug

[Robert P. J. Day]

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
========================================================================

Re: git rm bug

[Todd Zullinger]

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.

Re: git rm bug

[Timothy Rice]

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

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

~ Tim

Re: git rm bug

[Robert P. J. Day]

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

Re: git rm bug

[Ævar Arnfjörð Bjarmason]

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

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.

Re: git rm bug

[Jeff King]

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

Re: git rm bug

[Ævar Arnfjörð Bjarmason]

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.

Re: git rm bug

[Duy Nguyen]

On Wed, Jun 6, 2018 at 9:32 PM, Thomas Fischer
<thomasfischer@fastmail.com> 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