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

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Jan 21, 2008, 18:47 UTC
Message-ID: <alpine.LFD.1.00.0801211022350.2957@woody.linux-foundation.org>
In-Reply-To: <7vzluzzhud.fsf_-_@gitster.siamese.dyndns.org>

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