possible bug of git stash deleting uncommitted files in corner case

Hi,

my coworker today claimed git stash deleted his files, I made him explain me what he did and I think it could actually be a bug even if happening only in a corner case better explained with code to reproduce:

git init echo 'X' > foo git add foo git commit -m 'foo file committed'

what follow is the corner case I'm talking about: he deleted the file, created a directory with THE SAME name and added lot of files in it, in this example I'll only add one

rm foo mkdir foo echo 'B' > foo/bar

at this point the working directory looks like this:

 $ tree
.
└── foo
    └── bar

1 directory, 1 file

but apparently git status does not seem to see foo directory at all:

$ git status On branch master Changes not staged for commit:

          deleted:    foo

no changes added to commit (use "git add" and/or "git commit -a")

I expected something more like:

$ git status On branch master Changes not staged for commit:

          deleted: foo

Untracked files:

          foo/

no changes added to commit (use "git add" and/or "git commit -a")

Anyway he went on and decided to stash his work:

git stash

at this point stash deleted the "bar" file, in his case all the work on the previous couple of hours, but he didn't know yet

$ git stash show stash@{0} foo | 1 - 1 file changed, 1 deletion(-)

the foo directory was gone, replaced by the foo file

git stash pop

did not complained and deleted foo file again

I know my co-worker shouldn't had created a directory with the same file he was deleting but I also think git shouldn't have allowed him to stash at all, or should have been cleaver enough to actually stash the directory with its files.

Regards, Daniele Segato

sorry I forgot:

 $ git --version
git version 1.9.1

On Fri, Apr 22, 2016 at 1:46 PM, Daniele Segato <daniele.segato@gmail.com> wrote:

Daniele Segato <daniele.segato@gmail.com> wrote:

AFAIK it is a bug.
Without even going to the stash command,
    git init
    echo 'X' >foo
    git add foo
    git commit -m "Adding foo"
    rm foo
    mkdir foo
    echo 'B' >foo/bar
    git status
At this point the state is incorrect, the file foo/bar isn't there, as
you mentionned.

Contrary to what I expected, this seems to still be the case with:
  $ git --version
  git version 2.8.0.rc2

Being at $daywork right now, I am unable to test this any further.

Thanks, Rémi

Remi Galan Alfonso <remi.galan-alfonso@ensimag.grenoble-inp.fr> writes:

I do not think "git stash" has been updated in any major way to address correctness (including its corner case behaviour) ever since it was originally written, so it is very likely that any bug you see would be with it since the very old days.

Junio C Hamano <gitster@pobox.com> writes:

For this bug it doesn't seem to be specifically linked to git stash, since 'git status' doesn't display correct informations in the first place (it doesn't show foo/bar as an untracked file).

I tried something quickly, based on Daniele's case:
    git init
    echo 'X' >foo
    git add foo
    git commit -m "Added foo"
    rm foo
    mkdir foo
    echo 'B' >foo/bar

    git status # foo/bar not shown in Untracked files

    git add foo/bar

git status then shows as expected: # On branch master # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # deleted: foo # new file: foo/bar

However git stash fails this time: # error: foo: is a directory - add individual files instead # fatal: Unable to process path foo # Cannot save the current worktree state

I am not sure what should be the correct behaviour here. This however might be one of the corner cases you mentionned.

Thanks, Rémi

On Fri, Apr 22, 2016 at 09:04:25PM +0200, Remi Galan Alfonso wrote:

Before you "git add foo/bar", try "git status -uall", which asks git to descend into directories when looking for untracked files. It _does_ show foo/bar as untracked.

So I think what is happening is that in the case without "-uall", we see "foo" as an untracked file, but then check that with the index to say "ah, it is not untracked, there is an entry in the index". But of course the earlier mention in "not staged for commit" will not say anything about the new directory "foo", because we only diff actual files there.

Likewise, if you ask "ls-files -o", it mentions "foo/bar".

So I think the bug is in the way dir.c handles DIR_SHOW_OTHER_DIRECTORIES, or possibly in the way that wt_status_collect_untracked handles its results.

It may be that the latter needs to special-case its cache_name_is_other() check, and make sure we mention an "other" file we found that is a directory, even if it has an index entry.

I suspect the actual stash bug is separate from the bug above. It just looks like update-index is not smart enough to realize that a file being replaced with a directory is effectively an index deletion of that entry. That's just a guess, though.

-Peff