From 2e0052a5eb336dc48856bae0283a23198346b5ac Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 12 Apr 2010 16:25:25 -0700 Subject: [PATCH 1/5] tag.c: Correct indentation These lines were incorrectly indented with spaces, violating our coding style. Its annoying to read with 4 position tab stops, so fix the indentation to be correct. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano --- tag.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tag.c b/tag.c index 4470d2bf78..52d71bb1bc 100644 --- a/tag.c +++ b/tag.c @@ -44,9 +44,9 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size) char type[20]; const char *start = data; - if (item->object.parsed) - return 0; - item->object.parsed = 1; + if (item->object.parsed) + return 0; + item->object.parsed = 1; if (size < 64) return -1; From 628511a5883fa809e86b34ebc147ac62eb214458 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 12 Apr 2010 16:25:26 -0700 Subject: [PATCH 2/5] tag.h: Remove unused signature field Its documented as unused. So lets just drop it from the structure since we haven't ever used it. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano --- tag.h | 1 - 1 file changed, 1 deletion(-) diff --git a/tag.h b/tag.h index 7a0cb0070d..c437890f79 100644 --- a/tag.h +++ b/tag.h @@ -9,7 +9,6 @@ struct tag { struct object object; struct object *tagged; char *tag; - char *signature; /* not actually implemented */ }; extern struct tag *lookup_tag(const unsigned char *sha1); From 28de5b6b400fcdbd2d0665bc0cec7d7e4b813b43 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 12 Apr 2010 16:25:27 -0700 Subject: [PATCH 3/5] tag.c: Refactor parse_tag_buffer to be saner to program This code was horribly ugly to follow. The structure of the headers in an annotated tag object must follow a prescribed order, and most of these are required. Simplify the entire parsing logic by going through the headers in the order they are supposed to appear in, acting on each header as its identified in the buffer. This change has the same behavior as the older version, its just easier to read and maintain. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano --- tag.c | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/tag.c b/tag.c index 52d71bb1bc..ceb86557c5 100644 --- a/tag.c +++ b/tag.c @@ -38,11 +38,11 @@ struct tag *lookup_tag(const unsigned char *sha1) int parse_tag_buffer(struct tag *item, void *data, unsigned long size) { - int typelen, taglen; unsigned char sha1[20]; - const char *type_line, *tag_line, *sig_line; char type[20]; - const char *start = data; + const char *bufptr = data; + const char *tail = bufptr + size; + const char *nl; if (item->object.parsed) return 0; @@ -50,29 +50,19 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size) if (size < 64) return -1; - if (memcmp("object ", data, 7) || get_sha1_hex((char *) data + 7, sha1)) + if (memcmp("object ", bufptr, 7) || get_sha1_hex(bufptr + 7, sha1) || bufptr[47] != '\n') return -1; + bufptr += 48; /* "object " + sha1 + "\n" */ - type_line = (char *) data + 48; - if (memcmp("\ntype ", type_line-1, 6)) + if (prefixcmp(bufptr, "type ")) return -1; - - tag_line = memchr(type_line, '\n', size - (type_line - start)); - if (!tag_line || memcmp("tag ", ++tag_line, 4)) + bufptr += 5; + nl = memchr(bufptr, '\n', tail - bufptr); + if (!nl || sizeof(type) <= (nl - bufptr)) return -1; - - sig_line = memchr(tag_line, '\n', size - (tag_line - start)); - if (!sig_line) - return -1; - sig_line++; - - typelen = tag_line - type_line - strlen("type \n"); - if (typelen >= 20) - return -1; - memcpy(type, type_line + 5, typelen); - type[typelen] = '\0'; - taglen = sig_line - tag_line - strlen("tag \n"); - item->tag = xmemdupz(tag_line + 4, taglen); + strncpy(type, bufptr, nl - bufptr); + type[nl - bufptr] = '\0'; + bufptr = nl + 1; if (!strcmp(type, blob_type)) { item->tagged = &lookup_blob(sha1)->object; @@ -87,6 +77,15 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size) item->tagged = NULL; } + if (prefixcmp(bufptr, "tag ")) + return -1; + bufptr += 4; + nl = memchr(bufptr, '\n', tail - bufptr); + if (!nl) + return -1; + item->tag = xmemdupz(bufptr, nl - bufptr); + bufptr = nl + 1; + return 0; } From e451d06bf39df35d1106ad9bde5e38505533d805 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 12 Apr 2010 16:25:28 -0700 Subject: [PATCH 4/5] tag.c: Parse tagger date (if present) Just like with committer dates, we parse the tagger date into the struct tag so its available for further downstream processing. However since the tagger header was not introduced until Git 0.99.1 we must consider it optional. For tags missing this header we use the default date of 0. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano --- tag.c | 22 ++++++++++++++++++++++ tag.h | 1 + 2 files changed, 23 insertions(+) diff --git a/tag.c b/tag.c index ceb86557c5..85607c219e 100644 --- a/tag.c +++ b/tag.c @@ -36,6 +36,23 @@ struct tag *lookup_tag(const unsigned char *sha1) return (struct tag *) obj; } +static unsigned long parse_tag_date(const char *buf, const char *tail) +{ + const char *dateptr; + + while (buf < tail && *buf++ != '>') + /* nada */; + if (buf >= tail) + return 0; + dateptr = buf; + while (buf < tail && *buf++ != '\n') + /* nada */; + if (buf >= tail) + return 0; + /* dateptr < buf && buf[-1] == '\n', so strtoul will stop at buf-1 */ + return strtoul(dateptr, NULL, 10); +} + int parse_tag_buffer(struct tag *item, void *data, unsigned long size) { unsigned char sha1[20]; @@ -86,6 +103,11 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size) item->tag = xmemdupz(bufptr, nl - bufptr); bufptr = nl + 1; + if (!prefixcmp(bufptr, "tagger ")) + item->date = parse_tag_date(bufptr, tail); + else + item->date = 0; + return 0; } diff --git a/tag.h b/tag.h index c437890f79..47662724a6 100644 --- a/tag.h +++ b/tag.h @@ -9,6 +9,7 @@ struct tag { struct object object; struct object *tagged; char *tag; + unsigned long date; }; extern struct tag *lookup_tag(const unsigned char *sha1); From 03e8b541b38d95d1cd7b287963c82617a0469f80 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 12 Apr 2010 16:25:29 -0700 Subject: [PATCH 5/5] describe: Break annotated tag ties by tagger date If more than one annotated tag points at the same commit, use the tag whose tagger field has a more recent date stamp. This resolves non-deterministic cases where the maintainer has done: $ git tag -a -m "2.1-rc1" v2.1-rc1 deadbeef $ git tag -a -m "2.1" v2.1 deadbeef If the tag is an older-style annotated tag with no tagger date, we assume a date stamp at the UNIX epoch. This will cause us to prefer an annotated tag that has a valid date. We could also try to consider the tag object chain, favoring a tag that "includes" another one: $ git tag -a -m "2.1-rc0" v2.1-rc1 deadbeef $ git tag -a -m "2.1" v2.1 v2.1-rc1 However traversing the tag's object chain looking for inclusion is much more complicated. Its already very likely that even in these cases the v2.1 tag will have a more recent tagger date than v2.1-rc1, so with this change describe should still resolve this by selecting the more recent v2.1. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano --- builtin-describe.c | 49 +++++++++++++++++++++++++++++++++++++++++---- t/t6120-describe.sh | 8 +++++--- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/builtin-describe.c b/builtin-describe.c index 71be2a9364..43caff2ffe 100644 --- a/builtin-describe.c +++ b/builtin-describe.c @@ -35,7 +35,8 @@ static const char *diff_index_args[] = { struct commit_name { struct tag *tag; - int prio; /* annotated tag = 2, tag = 1, head = 0 */ + unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */ + unsigned name_checked:1; unsigned char sha1[20]; char path[FLEX_ARRAY]; /* more */ }; @@ -43,18 +44,53 @@ static const char *prio_names[] = { "head", "lightweight", "annotated", }; +static int replace_name(struct commit_name *e, + int prio, + const unsigned char *sha1, + struct tag **tag) +{ + if (!e || e->prio < prio) + return 1; + + if (e->prio == 2 && prio == 2) { + /* Multiple annotated tags point to the same commit. + * Select one to keep based upon their tagger date. + */ + struct tag *t; + + if (!e->tag) { + t = lookup_tag(e->sha1); + if (!t || parse_tag(t)) + return 1; + e->tag = t; + } + + t = lookup_tag(sha1); + if (!t || parse_tag(t)) + return 0; + *tag = t; + + if (e->tag->date < t->date) + return 1; + } + + return 0; +} + static void add_to_known_names(const char *path, struct commit *commit, int prio, const unsigned char *sha1) { struct commit_name *e = commit->util; - if (!e || e->prio < prio) { + struct tag *tag = NULL; + if (replace_name(e, prio, sha1, &tag)) { size_t len = strlen(path)+1; free(e); e = xmalloc(sizeof(struct commit_name) + len); - e->tag = NULL; + e->tag = tag; e->prio = prio; + e->name_checked = 0; hashcpy(e->sha1, sha1); memcpy(e->path, path, len); commit->util = e; @@ -165,10 +201,15 @@ static void display_name(struct commit_name *n) { if (n->prio == 2 && !n->tag) { n->tag = lookup_tag(n->sha1); - if (!n->tag || parse_tag(n->tag) || !n->tag->tag) + if (!n->tag || parse_tag(n->tag)) die("annotated tag %s not available", n->path); + } + 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); + n->name_checked = 1; } if (n->tag) diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh index 065deadc29..876d1ab743 100755 --- a/t/t6120-describe.sh +++ b/t/t6120-describe.sh @@ -8,7 +8,7 @@ test_description='test describe o----o----o----o----o----. / \ A c / .------------o---o---o - D e + D,R e ' . ./test-lib.sh @@ -68,6 +68,8 @@ test_expect_success setup ' echo D >another && git add another && git commit -m D && test_tick && git tag -a -m D D && + test_tick && + git tag -a -m R R && test_tick && echo DD >another && git commit -a -m another && @@ -89,10 +91,10 @@ test_expect_success setup ' check_describe A-* HEAD check_describe A-* HEAD^ -check_describe D-* HEAD^^ +check_describe R-* HEAD^^ check_describe A-* HEAD^^2 check_describe B HEAD^^2^ -check_describe D-* HEAD^^^ +check_describe R-* HEAD^^^ check_describe c-* --tags HEAD check_describe c-* --tags HEAD^