Re: [PATCH 2/2] read-cache.c: fix timestamp comparison

On Mon, 21 Jan 2008, Junio C Hamano wrote:

The mtime recorded in the cache entry is not time_t anymore but
of type (unsigned int).  This casts the comparison and also adds
fuzz factor of 1 second.

I really don't think this is the right fix.

The real problem seems to be that the whole "racy_timestamp()" thing 
*hides* the bug, and your change actually makes racy_timestamp() just hide 
things even more (by making it trigger all the time).

Adding a "sleep 1" to your test-case (appended) actually makes it fail 
reliably (without your hack - with your "+1" hack I assume you'd need to 
make the sleep longer).

So the problem is that the test-suite actually *hid* the bug by doing 
thigns so fast that the racy code triggered, and that in turn somehow 
fixed things up. 

Will experiment more now that I have a case that reliably fails. The 
commit that causes this literally shouldn't have caused any semantic 
changes at all, so this is rather interesting.

		Linus

---
#!/bin/sh

test_description='reduced 7501'

. ./test-lib.sh

test_expect_success setup '

        >file &&
        git add file &&
        test_tick &&
        git commit -m initial

'

if test -f ../trace.log
then
        mv ../trace.log ../trace.log.old
fi

test_expect_success 'partial commit that involves removal (1)' '

        test_tick &&
        git rm --cached file &&
        mv file elif &&
	sleep 1 &&
        git add elif &&
	sleep 1 &&

        strace -v -o ../trace.log git commit -m "Partial: add elif" elif &&

        git diff-tree --name-status HEAD^ HEAD >current &&
        echo "A	elif" >expected &&
        diff expected current

'

test_done