From: Jonathan Nieder <jrnieder@gmail.com>
Date: Wed, 05 Apr 2017 18:41:19 GMT
Subject: Re: [PATCH v6] read-cache: force_verify_index_checksum
Message-ID: <20170405184119.GF8741@aiede.mtv.corp.google.com>
In-Reply-To: <20170405180625.12439-2-git@jeffhostetler.com>

Hi,

git@jeffhostetler.com wrote:

> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -689,4 +689,17 @@ test_expect_success 'bogus head does not fallback to all heads' '
>  	! grep $blob out
>  '
>  
> +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 &&

Why is the "rm" before "mv" needed?  I would have expected "mv" to
replace the file.

If that doesn't work, I expect there are a large number of other tests
that would need fixing...

> +	test_when_finished "rm .git/index" &&

Likewise --- this seems redundant next to the "mv" enqueued previously.

> +	test_must_fail git fsck --cache

That said,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>