Re: [PATCHv2] git-mv: Keep moved index entries inact

Hi,

On Mon, 28 Jul 2008, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

On Mon, 28 Jul 2008, SZEDER Gábor wrote:

quoted

there is a race somewhere in these 'git-mv: Keep moved index entries
inact' changes.

The test cases 'git mv should overwrite symlink to a file' or 'git mv
should overwrite file with a symlink' fail occasionaly.  It's quite
non-deterministic:  I have run t7001-mv.sh in a loop (see below) and
one or the other usually fails around 50 runs (but sometimes only
after 150).  Adding some tracing echos to the tests shows that both
tests fail when running 'git diff-files' at the end.

To make it more convenient to test: with this patch it fails all the time:

It's because we rename(2) but do not read back ctime, and reuse the cached
data from the old path that was renamed.  After the failed test that moves
a regular file "move" to "symlink":

$ stat symlink
  File: `symlink'
  Size: 2               Blocks: 8          IO Block: 4096   regular file
Device: 30ah/778d       Inode: 18104337    Links: 1
Access: (0664/-rw-rw-r--)  Uid: ( 1012/   junio)   Gid: (   40/     src)
Access: 2008-07-28 11:49:55.000000000 -0700
Modify: 2008-07-28 11:48:41.000000000 -0700
Change: 2008-07-28 11:48:42.000000000 -0700

But the cached stat information looks like this:

$ ../../git-ls-files --stat
ctime=1217270921, mtime=1217270921, ino=18104337, mode=100644, uid=1012, gid=40symlink

We need to refresh the entry to pick up potential ctime changes.

Yep.

Tested-by: me

BTW I have no idea how we could test for this, short of introducing the 
"sleep 1" I did earlier.  Maybe guard it with a TEST_EXPENSIVE_CTIME 
variable or something similar.  Dunno.

And my suggestion to use test-chmtime: please just forget about it, and 
just assume that I had some very good wiid(1) in my pipe(2).

Ciao,
Dscho