Subject: Re: [PATCH 4/6] fsck: check tag objects' headers

From: Jeff King <[email-masked]>
Date: Fri, 29 Aug 2014 19:46:41 -0400

On Thu, Aug 28, 2014 at 02:36:22PM -0700, Junio C Hamano wrote:

> Junio C Hamano <[email-masked]> writes:
> 
> >> +	if (check_refname_format(buffer, REFNAME_ALLOW_ONELEVEL))
> >> +		ret = error_func(&tag->object, FSCK_ERROR, "invalid 'tag' name: %s", buffer);
> >> +	*eol = '\n';
> >
> > I actually think this check is harmful.
> 
> Let me take this one back; we do a moral equivalent when we create a
> tag, like this:
> 
> 	strbuf_addf(sb, "refs/tags/%s", name);
>         return check_refname_format(sb->buf, 0);

Hmm. But that is because "git tag" always makes one type of tag: one in
which the "tag" field is the same as the refname in which we store it.
So the name must be a valid refname there to meet the ref storage
requirement, and therefore the tag name must, too.

But is that something we necessarily need or want to enforce? Is it OK
for me to have refs/tags/foo pointing to a tag object that is not
related to "foo" (either semantically or syntactically)?

I dunno. I cannot think of a reason you would want to do such a thing,
but this seems like outlawing it because git does not generate it, not
because it is necessarily a problematic thing to be doing.

-Peff