mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <>
To: Jeff King <>
Cc: Matheus Tavares <>,,
Subject: Re: [PATCH] describe: output tag's ref instead of embedded name
Date: Tue, 18 Feb 2020 11:31:35 -0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <> (Jeff King's message of "Sun, 16 Feb 2020 01:51:01 -0500")

Jeff King <> 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

 t/ | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/t/ b/t/
index 960fd99bb1..7544278782 100755
--- a/t/
+++ b/t/
@@ -6,7 +6,7 @@ test_description='test describe
        /                   /    /
  o----o----o----o----o----.    /
-       \        A    c        /
+       \   U    A    c        /
                    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
+	test_i18ncmp err.expect err.actual
 test_expect_success 'pack tag refs' 'git pack-refs'
 check_describe A-* HEAD

  reply	other threads:[~2020-02-18 19:31 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-05 14:13 git-describe --tags warning: 'X' is really 'Y' here Roland Hieber
2020-02-05 17:15 ` Matheus Tavares Bernardino
2020-02-14  6:53   ` Jeff King
2020-02-14 16:57     ` Junio C Hamano
2020-02-15 21:34       ` [PATCH] describe: output tag's ref instead of embedded name Matheus Tavares
2020-02-16  6:51         ` Jeff King
2020-02-18 19:31           ` Junio C Hamano [this message]
2020-02-18 19:54             ` Jeff King
2020-02-18 23:05               ` Junio C Hamano
2020-02-18 23:28                 ` Junio C Hamano
2020-02-19  1:57                   ` Jeff King
2020-02-19  3:22                     ` Junio C Hamano
2020-02-19  3:56                       ` Jeff King
2020-02-19 11:14                         ` Junio C Hamano
2020-02-20 11:25                           ` Jeff King
2020-02-20 17:34                             ` Junio C Hamano
2020-02-20 22:19                               ` Matheus Tavares Bernardino
2020-02-20 22:59                                 ` Junio C Hamano
2020-02-21  1:33                                   ` Matheus Tavares
2020-02-21  2:05                                     ` Junio C Hamano
2020-02-21  6:00                                       ` Jeff King
2020-02-21  5:58                               ` Jeff King
2020-02-19 10:08                       ` Roland Hieber

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).