Re: [PATCH] reopen_tempfile(): truncate opened file

From: Duy Nguyen <pclouds@gmail.com>
Date: Sep 5, 2018, 15:27 UTC
Message-ID: <CACsJy8Ax4S9Sms6TY1dMV8M9-=hakEW8TCqn8yxb73Vbrpy_MQ@mail.gmail.com>
In-Reply-To: <20180904233643.GA9156@sigill.intra.peff.net>

On Wed, Sep 5, 2018 at 1:36 AM Jeff King <peff@peff.net> wrote:

> It turned out not to be too bad to write a test. It feels a little like
> black magic, since I empirically determined a way in which the
> cache-tree happens to shrink with the current code.

Aha! I attempted to reproduce with a verylongpathname and failed, but then I had the main index portion in mind, not cache-tree.

Show 14 quoted lines

> diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
> index 7de40141ca..94fcb4a78e 100755
> --- a/t/t0090-cache-tree.sh
> +++ b/t/t0090-cache-tree.sh
> @@ -161,6 +161,24 @@ test_expect_success PERL 'commit --interactive gives cache-tree on partial commi
>         test_cache_tree
>  '
>
> +test_expect_success PERL 'commit -p with shrinking cache-tree' '
> +       mkdir -p deep/subdir &&
> +       echo content >deep/subdir/file &&
> +       git add deep &&
> +       git commit -m add &&
> +       git rm -r deep &&

OK so I guess at this step, we invalidate some cache-tree blocks, but we write the same blocks down (with "invalid" flag), so pretty much the same size as before.

> +
> +       before=$(wc -c <.git/index) &&
> +       git commit -m delete -p &&

Then inside this command we recompute cache-tree and throw away the invalidated cache-tree blocks for deep and deep/subdir, which shrinks the index. Makes sense.

Show 7 quoted lines

> +       after=$(wc -c <.git/index) &&
> +
> +       # double check that the index shrank
> +       test $before -gt $after &&
> +
> +       # and that our index was not corrupted
> +       git fsck

If the index is not shrunk, we parse remaining rubbish as extensions. If by chance the rubbish extension name is in uppercase, then we ignore (and not flag it as error). But then the chances of the next 4 bytes being the "right" extension size is so small that we would end up flagging it as bad extension anyway. So it's good. But if you want to be even stricter (not necessary in my opinion), make sure that stderr is empty.

The rest of the patch is obviously correct.

-- 
Duy