git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Matheus Tavares <matheus.bernardino@usp.br>
To: git@vger.kernel.org
Cc: peff@peff.net, gitster@pobox.com, rhi@pengutronix.de
Subject: [PATCH] describe: output tag's ref instead of embedded name
Date: Sat, 15 Feb 2020 18:34:13 -0300	[thread overview]
Message-ID: <fcf19a46b80322c5579142efe4ec681a4dcbdd28.1581802264.git.matheus.bernardino@usp.br> (raw)
In-Reply-To: <xmqqd0ahp0na.fsf@gitster-ct.c.googlers.com>

If a tag describes a commit, we currently output not the tag's ref but
its embedded name. This means that when the tag is locally stored under
a different name, the output given cannot be used to access the tag in
any way. A warning is also emitted in this case, but the message is not
very enlightening:

$ git tag -am "testing tag body" testing-tag
$ mv .git/refs/tags/testing-tag .git/refs/tags/testing-tag-with-new-name
$ git describe --tags --abbrev=0
warning: tag 'testing-tag' is really 'testing-tag-with-new-name' here
testing-tag

Let's make git-describe output the tag's local name instead and
rephrase the warning to reflect the situation a little better.

Since the embedded name will no longer be needed for correct output, we
can convert the die() call in append_name() when a tag doesn't have the
embedded name to a warning(). Also, this function will now have two
disconnected responsibilities: verifying if the tag's embedded name
matches the ref and actually appending the ref to a given buffer (which
does not depend on the parsed tag object itself). Thus, to increase
intelligibility, let's make the function specialize in the former and do
the latter outside it.

Suggested-by: Jeff King <peff@peff.net>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
---
 builtin/describe.c  | 23 ++++++++---------------
 t/t6120-describe.sh |  4 ++--
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index b6df81d8d0..6a11604d2e 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -267,7 +267,7 @@ static unsigned long finish_depth_computation(
 	return seen_commits;
 }
 
-static void append_name(struct commit_name *n, struct strbuf *dst)
+static void verify_tag_embedded_name(struct commit_name *n)
 {
 	if (n->prio == 2 && !n->tag) {
 		n->tag = lookup_tag(the_repository, &n->oid);
@@ -276,19 +276,11 @@ static void append_name(struct commit_name *n, struct strbuf *dst)
 	}
 	if (n->tag && !n->name_checked) {
 		if (!n->tag->tag)
-			die(_("annotated tag %s has no embedded name"), n->path);
-		if (strcmp(n->tag->tag, all ? n->path + 5 : n->path))
-			warning(_("tag '%s' is really '%s' here"), n->tag->tag, n->path);
+			warning(_("annotated tag %s has no embedded name"), n->path);
+		else if (strcmp(n->tag->tag, all ? n->path + 5 : n->path))
+			warning(_("tag '%s' is externally known as '%s'"), n->path, n->tag->tag);
 		n->name_checked = 1;
 	}
-
-	if (n->tag) {
-		if (all)
-			strbuf_addstr(dst, "tags/");
-		strbuf_addstr(dst, n->tag->tag);
-	} else {
-		strbuf_addstr(dst, n->path);
-	}
 }
 
 static void append_suffix(int depth, const struct object_id *oid, struct strbuf *dst)
@@ -313,7 +305,8 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)
 		/*
 		 * Exact match to an existing ref.
 		 */
-		append_name(n, dst);
+		verify_tag_embedded_name(n);
+		strbuf_addstr(dst, n->path);
 		if (longformat)
 			append_suffix(0, n->tag ? get_tagged_oid(n->tag) : oid, dst);
 		if (suffix)
@@ -447,8 +440,8 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)
 				oid_to_hex(&gave_up_on->object.oid));
 		}
 	}
-
-	append_name(all_matches[0].name, dst);
+	verify_tag_embedded_name(all_matches[0].name);
+	strbuf_addstr(dst, all_matches[0].name->path);
 	if (abbrev)
 		append_suffix(all_matches[0].depth, &cmit->object.oid, dst);
 	if (suffix)
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 09c50f3f04..d34c091e0b 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -129,9 +129,9 @@ test_expect_success 'rename tag A to Q locally' '
 	mv .git/refs/tags/A .git/refs/tags/Q
 '
 cat - >err.expect <<EOF
-warning: tag 'A' is really 'Q' here
+warning: tag 'Q' is externally known as 'A'
 EOF
-check_describe A-* HEAD
+check_describe Q-* HEAD
 test_expect_success 'warning was displayed for Q' '
 	test_i18ncmp err.expect err.actual
 '
-- 
2.25.0


  reply	other threads:[~2020-02-15 21:34 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       ` Matheus Tavares [this message]
2020-02-16  6:51         ` [PATCH] describe: output tag's ref instead of embedded name Jeff King
2020-02-18 19:31           ` Junio C Hamano
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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=fcf19a46b80322c5579142efe4ec681a4dcbdd28.1581802264.git.matheus.bernardino@usp.br \
    --to=matheus.bernardino@usp.br \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=rhi@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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

	https://80x24.org/mirrors/git.git

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