Re: [PATCH] describe: output tag's ref instead of embedded name

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> I think the conversion of the die() to warning() makes sense here. Do we
> want to cover that with a test?

I presume that you are talking about this case.

>  	if (n->tag && !n->name_checked) {
>  		if (!n->tag->tag)
> -			die(_("annotated tag %s has no embedded name"), n->path);
> +			warning(_("annotated tag %s has no embedded name"), n->path);

The attached is my attempt to craft such a test.  It turns out that
it is tricky to trigger this die/warning.  I haven't dug deeply
enough, but I suspect this might be a dead code now.

A few curious points about the attached *test* that does not work.

 * "git tag -a" cannot create a tag without tagname.  We need to
   resort to "git hash-object -t tag".

 * "git hash-object -t tag" has internal consistency check, and
   rejects input that lack the tagname.  We need to resort to the
   --literally option.

 * Instead of "cat U.objname >.git/refs/tags/U", the method that is
   agnostic to the ref backend implementation, such as "git tag U
   $(cat U.objname)" or "git update-ref refs/tags/U $(cat
   U.objname)" should be usable and should be used.  But they
   complain that the object whose name is $(cat U.objname) does
   *NOT* exist.  I suspect we are triggering an error return from
   parse_tag_buffer() and parse_tag(), which tells
   parse_object_buffer() to return NULL, which in turn means there
   is no such object to its caller.

And of course, "check_describe U A^1" does not even see "U" and does
not complain.  I think that happens way before this part of the
code.  add_to_known_names() calls replace_name() that in turn calls
parse_tag() and a malformed annotated tag there won't even become
a candidate to describe the given commit, I think.

So, we might want to revisit this, analyze what happens fully, and
replace it die/warning with a BUG(), if it turns out to be a dead
code.

 t/t6120-describe.sh | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 960fd99bb1..7544278782 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -6,7 +6,7 @@ test_description='test describe
         .--------------o----o----o----x
        /                   /    /
  o----o----o----o----o----.    /
-       \        A    c        /
+       \   U    A    c        /
         .------------o---o---o
                    D,R   e
 '
@@ -140,6 +140,20 @@ test_expect_success 'rename tag Q back to A' '
 	mv .git/refs/tags/Q .git/refs/tags/A
 '
 
+test_expect_success 'warn for annotated tag without tagname' '
+	git cat-file tag A >A.txt &&
+	sed -e "s/object .*/object $(git rev-parse A^1)/" \
+	    -e "/^tag A/d" A.txt |
+	git hash-object -w --literally --stdin -t tag >U.objname &&
+	cat U.objname >.git/refs/tags/U &&
+
+	check_describe U A^1 &&
+	cat >err.expect <<-\EOF &&
+	warning: annotated tag U has no embedded name
+	EOF
+	test_i18ncmp err.expect err.actual
+'
+
 test_expect_success 'pack tag refs' 'git pack-refs'
 check_describe A-* HEAD