Re: [PATCH 1/5] t5312: test object deletion code paths in a corrupted repository

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 19, 2015, 20:04 UTC
Message-ID: <xmqqfv90iwjj.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20150317072844.GA25191@peff.net>

Jeff King <peff@peff.net> writes:

Show 8 quoted lines

> +test_expect_success 'create history with missing tip commit' '
> +	test_tick && git commit --allow-empty -m one &&
> +	recoverable=$(git rev-parse HEAD) &&
> +	git cat-file commit $recoverable >saved &&
> +	test_tick && git commit --allow-empty -m two &&
> +	missing=$(git rev-parse HEAD) &&
> +	# point HEAD elsewhere
> +	git checkout $base &&

Could you spell this as "$base^0" (or "--detach") to clarify the intention? I have been scraching my head for a few minutes just now, trying to figure out what you are doing here. I _think_ you wanted master to point at the missing "two" and wanted to make sure all other refs (including HEAD) to point away from it.

Mental note: At this point, the history looks like

    base   one    two
    o------o------o
     \
      o bogus

and because the reference to two is still there but two itself is missing, pruning may well end up losing one, because the reference to it is only through master pointing at two.

Show 9 quoted lines

> +	rm .git/objects/$(echo $missing | sed "s,..,&/,") &&
> +	test_must_fail git cat-file -e $missing
> +'
> +
> +test_expect_failure 'pruning with a corrupted tip does not drop history' '
> +	test_when_finished "git hash-object -w -t commit saved" &&
> +	test_might_fail git prune --expire=now &&
> +	verbose git cat-file -e $recoverable
> +'

Mental note: OK, this demonstrates that the missing two makes us lose the only reference to one (aka $recoverable in saved).

Show 16 quoted lines

> +test_expect_success 'pack-refs does not silently delete broken loose ref' '
> +	git pack-refs --all --prune &&
> +	echo $missing >expect &&
> +	git rev-parse refs/heads/master >actual &&
> +	test_cmp expect actual
> +'
> +
> +# we do not want to count on running pack-refs to
> +# actually pack it, as it is perfectly reasonable to
> +# skip processing a broken ref
> +test_expect_success 'create packed-refs file with broken ref' '
> +	rm -f .git/refs/heads/master &&
> +	cat >.git/packed-refs <<-EOF
> +	$missing refs/heads/master
> +	$recoverable refs/heads/other
> +	EOF

I do not know offhand if the lack of the pack-refs feature header matters here; I assume it does not?

A safer check may be to pack and then make it missing, I guess, but I do not know if the difference matters.