From: Jeff Hostetler <git@jeffhostetler.com>
Date: Wed, 05 Apr 2017 19:06:14 GMT
Subject: Re: [PATCH v6] read-cache: force_verify_index_checksum
Message-ID: <91d35c27-f0e3-0dca-dc87-5ab086e18e59@jeffhostetler.com>
In-Reply-To: <20170405182533.a5divj63zfkfejui@sigill.intra.peff.net>



On 4/5/2017 2:25 PM, Jeff King wrote:
> On Wed, Apr 05, 2017 at 06:06:25PM +0000, git@jeffhostetler.com wrote:
>
>> ---
>>  builtin/fsck.c  |  1 +
>>  cache.h         |  2 ++
>>  read-cache.c    |  7 +++++++
>>  t/t1450-fsck.sh | 13 +++++++++++++
>>  4 files changed, 23 insertions(+)
>
> This version is delightfully simple. Thanks for sticking with it.
>
>> +test_expect_success 'detect corrupt index file in fsck' '
>> +	cp .git/index .git/index.backup &&
>> +	test_when_finished "mv .git/index.backup .git/index" &&
>> +	echo zzzzzzzz >zzzzzzzz &&
>> +	test_when_finished "rm zzzzzzzz" &&
>> +	git add zzzzzzzz &&
>> +	sed -e "s/zzzzzzzz/yyyyyyyy/" .git/index >.git/index.yyy &&
>> +	rm .git/index &&
>> +	mv .git/index.yyy .git/index &&
>> +	test_when_finished "rm .git/index" &&
>> +	test_must_fail git fsck --cache
>> +'
>
> I was surprised by the second when-finished "rm". The earlier "mv"
> should be enough to restore everything. I guess you're trying to
> overcome the tendency of "mv" to complain about overwriting on some
> platforms.

Yeah, just being cautious. (I spend a lot of time on Windows.)

>
> But wouldn't that make the first when-finished "mv" potentially a
> problem, if it happens before the "rm .git/index" line?
>
> Perhaps "mv -f" would make it all simpler (ordinarily I'd suggest just
> "git reset --hard", but since we're corrupting the index, it may be
> better not to rely on git for the restore). So maybe just:
>
>   cp .git/index .git/index.backup &&
>   test_when_finished "mv -f .git/index.backup .git/index" &&
>   ...
>
> I think the "rm zzzzzzzz" can go, too. We don't typically worry too much
> about accumulating random files in the working tree. The important thing
> is that the index is restored to a sane state.
>
> -Peff
>

Good point.  Let me take another stab at it.
Thanks
Jeff