On Wed, Jun 13, 2018 at 06:43:04PM +0000, Kirill Smelkov wrote:
> From: Kirill Smelkov <kirr@xxxxxxxxxx>
> Date: Wed, 13 Jun 2018 12:28:21 +0300
> Subject: [PATCH v2] fetch-pack: test explicitly that --all can fetch tag
> references pointing to non-commits
>
> Fetch-pack --all became broken with respect to unusual tags in
> 5f0fc64513 (fetch-pack: eliminate spurious error messages, 2012-09-09),
> and was fixed only recently in e9502c0a7f (fetch-pack: don't try to fetch
> peel values with --all, 2018-06-11). However the test added in
> e9502c0a7f does not explicitly cover all funky cases.
>
> In order to be sure fetching funky tags will never break, let's
> explicitly test all relevant cases with 4 tag objects pointing to 1) a
> blob, 2) a tree, 3) a commit, and 4) another tag objects. The referenced
> tag objects themselves are referenced from under regular refs/tags/*
> namespace. Before e9502c0a7f `fetch-pack --all` was failing e.g. this way:
>
> .../git/t/trash directory.t5500-fetch-pack/fetchall$ git ls-remote ..
> 44085874... HEAD
> ...
> bc4e9e1f... refs/tags/tag-to-blob
> 038f48ad... refs/tags/tag-to-blob^{} # peeled
> 520db1f5... refs/tags/tag-to-tree
> 7395c100... refs/tags/tag-to-tree^{} # peeled
>
> .../git/t/trash directory.t5500-fetch-pack/fetchall$ git fetch-pack --all ..
> fatal: A git upload-pack: not our ref 038f48ad...
> fatal: The remote end hung up unexpectedly
TBH, I do not find this snippet all that compelling. We know that
e9502c0a7f already fixed the bug, and that it had nothing to do with
non-commits at all.
The primary reason to add these tests is that in general we do not cover
fetch-pack over tags to non-commits. And I think the reason to use
otherwise unreferenced objects is that it they are more likely to have
detectable symptoms if they tickle a bug.
So why don't we say that, instead of re-hashing output from the earlier
fix?
-Peff
