Merge branch 'sp/maint-describe-tiebreak-with-tagger-date'

* sp/maint-describe-tiebreak-with-tagger-date:
  describe: Break annotated tag ties by tagger date
  tag.c: Parse tagger date (if present)
  tag.c: Refactor parse_tag_buffer to be saner to program
  tag.h: Remove unused signature field
  tag.c: Correct indentation

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)

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^

tag.c

@@ -36,43 +36,50 @@ 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)
 {
-	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;
-        item->object.parsed = 1;
+	if (item->object.parsed)
+		return 0;
+	item->object.parsed = 1;
 
 	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 +94,20 @@ 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;
+
+	if (!prefixcmp(bufptr, "tagger "))
+		item->date = parse_tag_date(bufptr, tail);
+	else
+		item->date = 0;
+
 	return 0;
 }
 

tag.h

@@ -9,7 +9,7 @@ struct tag {
 	struct object object;
 	struct object *tagged;
 	char *tag;
-	char *signature; /* not actually implemented */
+	unsigned long date;
 };
 
 extern struct tag *lookup_tag(const unsigned char *sha1);