git tag -a HEAD leads to ambiguous branch name

git tag -a HEAD leads to ambiguous branch name

[git]

Good evening,
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.
Best regards
Johannes Wilde

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

[Jeff King]

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.

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.

-Peff

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

[Junio C Hamano]

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.

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

[Jeff King]

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).

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.

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.

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".

-Peff

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

[Junio C Hamano]

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".

Hmph, yeah, reverting the code change of 09116a1c31 and then using
'foo' instead of 'HEAD' fails the test exactly the same way.