Subject: Re: [PATCH v2 12/14] refs: don't store peeled object IDs for invalid tags

From: shejialuo <[email-masked]>
Date: Thu, 9 Oct 2025 00:27:58 +0800

On Wed, Oct 08, 2025 at 05:50:27PM +0200, Patrick Steinhardt wrote:
> Both the "files" and "reftable" backend store peeled object IDs for
> references that point to tags:
> 
>   - The "files" backend stores the value when packing refs, where each
>     peeled object ID is prefixed with "^".
> 
>   - The "reftable" backend stores the value whenever writing a new
>     reference that points to a tag via a special ref record type.
> 
> Both of these backends use `peel_object()` to find the peeled object ID.
> But as explained in the preceding commit, that function does not detect
> the case where the tag's tagged object and its claimed type mismatch.
> 
> The consequence of storing these bogus peeled object IDs is that we're
> less likely to detect such corruption in other parts of Git.
> git-for-each-ref(1) for example does not notice anymore that the tag is
> broken when using "--format=%(*objectname)" to dereference tags.
> 
> One could claim that this is good, because it still allows us to mostly
> use the tag as intended. But the biggest problem here is that we now
> have different behaviour for such a broken tag depending on whether or
> not we have its peeled value in the refdb.
> 
> Fix the issue by verifying the object type when peeling the object. If
> that verification fails we simply skip storing the peeled value in
> either of the reference formats.
> 

I have a design question here: should we just report an error to the
user or just die instead of skipping storing the peeled value? If the
annotated tag is corrupted in the first place, it means the refdb is
also corrupted. And "git-fsck(1)" would definitely report an error to
the user. But here we just ignore the problem and give an illusion that
everything is fine.

Thanks,
Jialuo