@matheusbsbs / matheus-bagliano / commits / ff165f00c1
Fork 0

describe: force long format for a name based on a mislocated tag

An annotated tag has two names: where it sits in the refs/tags hierarchy and the tagname recorded in the "tag" field in the object itself. They usually should match. Since 212945d4 ("Teach git-describe to verify annotated tag names before output", 2008-02-28), a commit described using an annotated tag bases its name on the tagname from the object. While this was a deliberate design decision to make it easier to converse about tags with others, even if the tags happen to be fetched to a different name than it was given by its creator, it had one downside. The output from "git describe", at least in the modern Git, should be usable as an object name to name the exact commit given to the "git describe" command. Using the tagname, when two names differ, breaks this property, when describing a commit that is directly pointed at by such a tag. An annotated tag Bob made as "v1.0" may sit at "refs/tags/v1.0-bob" in the ref hierarchy, and output from "git describe v1.0-bob^0" would say "v1.0", but there may not be any tag at "refs/tags/v1.0" locally or there may be another tag that points at a different object. Note that this won't be a problem if a commit being described is not directly pointed at by such a mislocated tag. In the example in the previous paragraph, describing a commit whose parent is v1.0-bob would result in "v1.0" (i.e. the tagname taken from the tag object) followed by "-1-gXXXXX" where XXXXX is the abbreviated object name, and a string that ends with "-g" followed by a hexadecimal string is an object name for the object whose name begins with hexadecimal string (as long as it is unique), so it does not matter if the leading part is "v1.0" or "v1.0-bob". Show the name in the long format, i.e. with "-0-gXXXXX" suffix, when the name we give is based on a mislocated annotated tag to ensure that the output can be used as the object name for the object originally given to the command to fix the issue. While at it, remove an overly cautious dead code to protect against an annotated tag object without the tagname. Such a tag is filtered out much earlier in the codeflow, and will not reach this part of the code. Helped-by: Matheus Tavares <matheus.bernardino@usp.br> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Feb 20, 2020 at 09:34 UTC ff165f00c1065d94c0bcab8708364e6dc5655f5d
2 files changed +28 -7
builtin/describe.c
+9 -6
@@ -54,6 +54,7 @@ struct commit_name {
54 struct tag *tag;
55 unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
56 unsigned name_checked:1;
57 + unsigned misnamed:1;
58 struct object_id oid;
59 char *path;
60 };
@@ -132,6 +133,7 @@ static void add_to_known_names(const char *path,
133 e->tag = tag;
134 e->prio = prio;
135 e->name_checked = 0;
136 + e->misnamed = 0;
137 oidcpy(&e->oid, oid);
138 free(e->path);
139 e->path = xstrdup(path);
@@ -275,10 +277,11 @@ static void append_name(struct commit_name *n, struct strbuf *dst)
277 die(_("annotated tag %s not available"), n->path);
278 }
279 if (n->tag && !n->name_checked) {
278 - if (!n->tag->tag)
279 - die(_("annotated tag %s has no embedded name"), n->path);
280 - if (strcmp(n->tag->tag, all ? n->path + 5 : n->path))
281 - warning(_("tag '%s' is really '%s' here"), n->tag->tag, n->path);
280 + if (strcmp(n->tag->tag, all ? n->path + 5 : n->path)) {
281 + warning(_("tag '%s' is externally known as '%s'"),
282 + n->path, n->tag->tag);
283 + n->misnamed = 1;
284 + }
285 n->name_checked = 1;
286 }
287
@@ -314,7 +317,7 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)
317 * Exact match to an existing ref.
318 */
319 append_name(n, dst);
317 - if (longformat)
320 + if (n->misnamed || longformat)
321 append_suffix(0, n->tag ? get_tagged_oid(n->tag) : oid, dst);
322 if (suffix)
323 strbuf_addstr(dst, suffix);
@@ -449,7 +452,7 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)
452 }
453
454 append_name(all_matches[0].name, dst);
452 - if (abbrev)
455 + if (all_matches[0].name->misnamed || abbrev)
456 append_suffix(all_matches[0].depth, &cmit->object.oid, dst);
457 if (suffix)
458 strbuf_addstr(dst, suffix);
t/t6120-describe.sh
+19 -1
@@ -130,12 +130,30 @@ test_expect_success 'rename tag A to Q locally' '
130 mv .git/refs/tags/A .git/refs/tags/Q
131 '
132 cat - >err.expect <<EOF
133 -warning: tag 'A' is really 'Q' here
133 +warning: tag 'Q' is externally known as 'A'
134 EOF
135 check_describe A-* HEAD
136 test_expect_success 'warning was displayed for Q' '
137 test_i18ncmp err.expect err.actual
138 '
139 +test_expect_success 'misnamed annotated tag forces long output' '
140 + description=$(git describe --no-long Q^0) &&
141 + expr "$description" : "A-0-g[0-9a-f]*$" &&
142 + git rev-parse --verify "$description" >actual &&
143 + git rev-parse --verify Q^0 >expect &&
144 + test_cmp expect actual
145 +'
146 +
147 +test_expect_success 'abbrev=0 will not break misplaced tag (1)' '
148 + description=$(git describe --abbrev=0 Q^0) &&
149 + expr "$description" : "A-0-g[0-9a-f]*$"
150 +'
151 +
152 +test_expect_success 'abbrev=0 will not break misplaced tag (2)' '
153 + description=$(git describe --abbrev=0 c^0) &&
154 + expr "$description" : "A-1-g[0-9a-f]*$"
155 +'
156 +
157 test_expect_success 'rename tag Q back to A' '
158 mv .git/refs/tags/Q .git/refs/tags/A
159 '