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 <spearce@spearce.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-describe.c b/builtin-describe.c
index 71be2a9364748668996696f6c74057dba43315b5..43caff2ffe185df6ab224b93b4fd9ce3d82de2d0 100644 (file)
--- 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 065deadc29eb3838f391ce758c4b188249dc87f9..876d1ab7430c2054ddc69bd3712ff769de52e774 100755 (executable)
--- 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^