git tag -a HEAD leads to ambiguous branch name

Jeff King· Nov 27, 2024, 14:27 UTC · re: git@jowil.de · lore

Re: git tag -a HEAD leads to ambiguous branch name

On Wed, Nov 27, 2024 at 01:14:23AM +0100, git@jowil.de wrote:

> today I encountered a warning using  git version 2.39.0.windows.2:
> /warning: refname 'HEAD' is ambiguous./
> It turned out, having written
> /git tag -a HEAD -m "Some message."/
> was the culprit.
> As
> /git branch HEAD/
> yields the following error
> /fatal: 'HEAD' is not a valid branch name/
> I wanted to aks, whether a similar check should  be applied to tag names.

-Peff

Junio C Hamano· Nov 27, 2024, 23:30 UTC · re: Jeff King · lore

Re: git tag -a HEAD leads to ambiguous branch name

Jeff King <peff@peff.net> writes:

> Yes, I think that's reasonable. We won't ever completely forbid "HEAD"
> as a branch or tag name for historical reasons. But when porcelain like
> git-branch or git-tag sees it, it is almost certainly a typo or
> forgotten argument.

A quick band-aid would be

 builtin/tag.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git c/builtin/tag.c w/builtin/tag.c
index 93d10d5915..c91aba5e26 100644
--- c/builtin/tag.c
+++ w/builtin/tag.c
@@ -449,7 +449,7 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
 
 static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
 {
-	if (name[0] == '-')
+	if (name[0] == '-' || !strcmp(name, "HEAD"))
 		return -1;
 
 	strbuf_reset(sb);

but this (together with what "git branch" uses) is unsatisfactory
for at least two reasons.

 - This helper function and object-name.c:strbuf_check_branch_ref()
   should not be named with "strbuf_" prefix.  

 - The right place to do these checks is check_refname_format(), but
   it would retroactively forbid porcelains from using HEAD as the
   name of a branch or a tag (which is technically allowed, but it
   is dubious anybody wants to do so because it is so cumbersome to
   use).

There is a dubious test in t5604 that expects you can create such a
tag.  t5604.22 wants to create one:

test_expect_success 'clone with reference from a tagged repository' '
	(
		cd A && git tag -a -m tagged HEAD
	) &&
	git clone --reference=A A I
'

and t5604.24 wants to use it.

test_expect_success 'fetch with incomplete alternates' '
	git init K &&
	echo "$base_dir/A/.git/objects" >K/.git/objects/info/alternates &&
	(
		cd K &&
		git remote add J "file://$base_dir/J" &&
		GIT_TRACE_PACKET=$U.K git fetch J
	) &&
	main_object=$(cd A && git for-each-ref --format="%(objectname)" refs/heads/main) &&
	test -s "$U.K" &&
	! grep " want $main_object" "$U.K" &&
	tag_object=$(cd A && git for-each-ref --format="%(objectname)" refs/tags/HEAD) &&
	! grep " want $tag_object" "$U.K"
'

If we did this check at check_refname_format() level, it probably
will affect cloning from an existing repository with such a branch
or a tag.

Jeff King· Dec 1, 2024, 22:44 UTC · re: Junio C Hamano · lore

Re: git tag -a HEAD leads to ambiguous branch name

On Thu, Nov 28, 2024 at 08:30:10AM +0900, Junio C Hamano wrote:

> diff --git c/builtin/tag.c w/builtin/tag.c
> index 93d10d5915..c91aba5e26 100644
> --- c/builtin/tag.c
> +++ w/builtin/tag.c
> @@ -449,7 +449,7 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
>  
>  static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
>  {
> -	if (name[0] == '-')
> +	if (name[0] == '-' || !strcmp(name, "HEAD"))
>  		return -1;
>  
>  	strbuf_reset(sb);
> 
> but this (together with what "git branch" uses) is unsatisfactory
> for at least two reasons.
> 
>  - This helper function and object-name.c:strbuf_check_branch_ref()
>    should not be named with "strbuf_" prefix. 

Agreed here.

>  - The right place to do these checks is check_refname_format(), but
>    it would retroactively forbid porcelains from using HEAD as the
>    name of a branch or a tag (which is technically allowed, but it
>    is dubious anybody wants to do so because it is so cumbersome to
>    use).

I'm OK with revisiting that decision, but:

  1. I think it's orthogonal to adding a similar UI-level check for
     git-tag.

  2. We'd probably need some deprecation period or breaking-changes
     moment to do the switch.

> There is a dubious test in t5604 that expects you can create such a
> tag.  t5604.22 wants to create one:
> 
> test_expect_success 'clone with reference from a tagged repository' '
> 	(
> 		cd A && git tag -a -m tagged HEAD
> 	) &&
> 	git clone --reference=A A I
> '
> 
> and t5604.24 wants to use it.

-Peff

Junio C Hamano· Dec 2, 2024, 04:38 UTC · re: Jeff King · lore

Re: git tag -a HEAD leads to ambiguous branch name

Jeff King <peff@peff.net> writes:

> But I don't know about this. I thought when we outlawed "git branch
> HEAD" that it was quite intentional _not_ to forbid these names at the
> lowest level in order to retain backwards compatibility. They are just
> about preventing common UI mistakes.

Yeah, I think this is a sensible way to think about it.

>> There is a dubious test in t5604 that expects you can create such a
>> tag.  t5604.22 wants to create one:
>> 
>> test_expect_success 'clone with reference from a tagged repository' '
>> 	(
>> 		cd A && git tag -a -m tagged HEAD
>> 	) &&
>> 	git clone --reference=A A I
>> '
>> 
>> and t5604.24 wants to use it.
>
> Weird. Comes from 09116a1c31 (refs: loosen over-strict "format" check,
> 2011-11-16). I don't see any reason it could not just be "foo".