Merge branch 'jc/name-rev-lw-tag'
authorJunio C Hamano <gitster@pobox.com>
Tue, 30 May 2017 02:16:39 +0000 (11:16 +0900)
committerJunio C Hamano <gitster@pobox.com>
Tue, 30 May 2017 02:16:40 +0000 (11:16 +0900)
"git describe --contains" penalized light-weight tags so much that
they were almost never considered. Instead, give them about the
same chance to be considered as an annotated tag that is the same
age as the underlying commit would.

* jc/name-rev-lw-tag:
name-rev: favor describing with tags and use committer date to tiebreak
name-rev: refactor logic to see if a new candidate is a better name

1  2 
builtin/name-rev.c
t/t4202-log.sh
diff --combined builtin/name-rev.c
index b7afffef73483ffda51dae87a8ae86f2677cef84,bf7ed015aebc07d2dd410745786fec269c28091f..663e26ebbdc49071d1fb6e2b289cd8f51841c688
  
  typedef struct rev_name {
        const char *tip_name;
 -      unsigned long taggerdate;
 +      timestamp_t taggerdate;
        int generation;
        int distance;
+       int from_tag;
  } rev_name;
  
  static long cutoff = LONG_MAX;
  /* How many generations are maximally preferred over _one_ merge traversal? */
  #define MERGE_TRAVERSAL_WEIGHT 65535
  
 -                        unsigned long taggerdate,
+ static int is_better_name(struct rev_name *name,
+                         const char *tip_name,
++                        timestamp_t taggerdate,
+                         int generation,
+                         int distance,
+                         int from_tag)
+ {
+       /*
+        * When comparing names based on tags, prefer names
+        * based on the older tag, even if it is farther away.
+        */
+       if (from_tag && name->from_tag)
+               return (name->taggerdate > taggerdate ||
+                       (name->taggerdate == taggerdate &&
+                        name->distance > distance));
+       /*
+        * We know that at least one of them is a non-tag at this point.
+        * favor a tag over a non-tag.
+        */
+       if (name->from_tag != from_tag)
+               return from_tag;
+       /*
+        * We are now looking at two non-tags.  Tiebreak to favor
+        * shorter hops.
+        */
+       if (name->distance != distance)
+               return name->distance > distance;
+       /* ... or tiebreak to favor older date */
+       if (name->taggerdate != taggerdate)
+               return name->taggerdate > taggerdate;
+       /* keep the current one if we cannot decide */
+       return 0;
+ }
  static void name_rev(struct commit *commit,
 -              const char *tip_name, unsigned long taggerdate,
 +              const char *tip_name, timestamp_t taggerdate,
-               int generation, int distance,
+               int generation, int distance, int from_tag,
                int deref)
  {
        struct rev_name *name = (struct rev_name *)commit->util;
        struct commit_list *parents;
        int parent_number = 1;
 +      char *to_free = NULL;
  
        parse_commit(commit);
  
@@@ -36,7 -74,7 +75,7 @@@
                return;
  
        if (deref) {
 -              tip_name = xstrfmt("%s^0", tip_name);
 +              tip_name = to_free = xstrfmt("%s^0", tip_name);
  
                if (generation)
                        die("generation: %d, but deref?", generation);
                name = xmalloc(sizeof(rev_name));
                commit->util = name;
                goto copy_data;
-       } else if (name->taggerdate > taggerdate ||
-                       (name->taggerdate == taggerdate &&
-                        name->distance > distance)) {
+       } else if (is_better_name(name, tip_name, taggerdate,
+                                 generation, distance, from_tag)) {
  copy_data:
                name->tip_name = tip_name;
                name->taggerdate = taggerdate;
                name->generation = generation;
                name->distance = distance;
 -      } else
+               name->from_tag = from_tag;
 +      } else {
 +              free(to_free);
                return;
 +      }
  
        for (parents = commit->parents;
                        parents;
                                                   parent_number);
  
                        name_rev(parents->item, new_name, taggerdate, 0,
-                               distance + MERGE_TRAVERSAL_WEIGHT, 0);
+                                distance + MERGE_TRAVERSAL_WEIGHT,
+                                from_tag, 0);
                } else {
                        name_rev(parents->item, tip_name, taggerdate,
-                               generation + 1, distance + 1, 0);
+                                generation + 1, distance + 1,
+                                from_tag, 0);
                }
        }
  }
@@@ -117,7 -155,7 +158,7 @@@ struct name_ref_data 
  
  static struct tip_table {
        struct tip_table_entry {
 -              unsigned char sha1[20];
 +              struct object_id oid;
                const char *refname;
        } *table;
        int nr;
        int sorted;
  } tip_table;
  
 -static void add_to_tip_table(const unsigned char *sha1, const char *refname,
 +static void add_to_tip_table(const struct object_id *oid, const char *refname,
                             int shorten_unambiguous)
  {
        refname = name_ref_abbrev(refname, shorten_unambiguous);
  
        ALLOC_GROW(tip_table.table, tip_table.nr + 1, tip_table.alloc);
 -      hashcpy(tip_table.table[tip_table.nr].sha1, sha1);
 +      oidcpy(&tip_table.table[tip_table.nr].oid, oid);
        tip_table.table[tip_table.nr].refname = xstrdup(refname);
        tip_table.nr++;
        tip_table.sorted = 0;
  static int tipcmp(const void *a_, const void *b_)
  {
        const struct tip_table_entry *a = a_, *b = b_;
 -      return hashcmp(a->sha1, b->sha1);
 +      return oidcmp(&a->oid, &b->oid);
  }
  
  static int name_ref(const char *path, const struct object_id *oid, int flags, void *cb_data)
  {
 -      struct object *o = parse_object(oid->hash);
 +      struct object *o = parse_object(oid);
        struct name_ref_data *data = cb_data;
        int can_abbreviate_output = data->tags_only && data->name_only;
        int deref = 0;
 -      unsigned long taggerdate = ULONG_MAX;
 +      timestamp_t taggerdate = TIME_MAX;
  
        if (data->tags_only && !starts_with(path, "refs/tags/"))
                return 0;
                        return 0;
        }
  
 -      add_to_tip_table(oid->hash, path, can_abbreviate_output);
 +      add_to_tip_table(oid, path, can_abbreviate_output);
  
        while (o && o->type == OBJ_TAG) {
                struct tag *t = (struct tag *) o;
                if (!t->tagged)
                        break; /* broken repository */
 -              o = parse_object(t->tagged->oid.hash);
 +              o = parse_object(&t->tagged->oid);
                deref = 1;
                taggerdate = t->date;
        }
        if (o && o->type == OBJ_COMMIT) {
                struct commit *commit = (struct commit *)o;
+               int from_tag = starts_with(path, "refs/tags/");
  
+               if (taggerdate == ULONG_MAX)
+                       taggerdate = ((struct commit *)o)->date;
                path = name_ref_abbrev(path, can_abbreviate_output);
-               name_rev(commit, xstrdup(path), taggerdate, 0, 0, deref);
+               name_rev(commit, xstrdup(path), taggerdate, 0, 0,
+                        from_tag, deref);
        }
        return 0;
  }
  static const unsigned char *nth_tip_table_ent(size_t ix, void *table_)
  {
        struct tip_table_entry *table = table_;
 -      return table[ix].sha1;
 +      return table[ix].oid.hash;
  }
  
  static const char *get_exact_ref_match(const struct object *o)
        return NULL;
  }
  
 -/* returns a static buffer */
 -static const char *get_rev_name(const struct object *o)
 +/* may return a constant string or use "buf" as scratch space */
 +static const char *get_rev_name(const struct object *o, struct strbuf *buf)
  {
 -      static char buffer[1024];
        struct rev_name *n;
        struct commit *c;
  
                int len = strlen(n->tip_name);
                if (len > 2 && !strcmp(n->tip_name + len - 2, "^0"))
                        len -= 2;
 -              snprintf(buffer, sizeof(buffer), "%.*s~%d", len, n->tip_name,
 -                              n->generation);
 -
 -              return buffer;
 +              strbuf_reset(buf);
 +              strbuf_addf(buf, "%.*s~%d", len, n->tip_name, n->generation);
 +              return buf->buf;
        }
  }
  
@@@ -272,11 -316,10 +317,11 @@@ static void show_name(const struct obje
  {
        const char *name;
        const struct object_id *oid = &obj->oid;
 +      struct strbuf buf = STRBUF_INIT;
  
        if (!name_only)
                printf("%s ", caller_name ? caller_name : oid_to_hex(oid));
 -      name = get_rev_name(obj);
 +      name = get_rev_name(obj, &buf);
        if (name)
                printf("%s\n", name);
        else if (allow_undefined)
                printf("%s\n", find_unique_abbrev(oid->hash, DEFAULT_ABBREV));
        else
                die("cannot describe '%s'", oid_to_hex(oid));
 +      strbuf_release(&buf);
  }
  
  static char const * const name_rev_usage[] = {
  
  static void name_rev_line(char *p, struct name_ref_data *data)
  {
 +      struct strbuf buf = STRBUF_INIT;
        int forty = 0;
        char *p_start;
        for (p_start = p; *p; p++) {
  #define ishex(x) (isdigit((x)) || ((x) >= 'a' && (x) <= 'f'))
                if (!ishex(*p))
                        forty = 0;
 -              else if (++forty == 40 &&
 +              else if (++forty == GIT_SHA1_HEXSZ &&
                         !ishex(*(p+1))) {
 -                      unsigned char sha1[40];
 +                      struct object_id oid;
                        const char *name = NULL;
                        char c = *(p+1);
                        int p_len = p - p_start + 1;
                        forty = 0;
  
                        *(p+1) = 0;
 -                      if (!get_sha1(p - 39, sha1)) {
 +                      if (!get_oid(p - (GIT_SHA1_HEXSZ - 1), &oid)) {
                                struct object *o =
 -                                      lookup_object(sha1);
 +                                      lookup_object(oid.hash);
                                if (o)
 -                                      name = get_rev_name(o);
 +                                      name = get_rev_name(o, &buf);
                        }
                        *(p+1) = c;
  
                                continue;
  
                        if (data->name_only)
 -                              printf("%.*s%s", p_len - 40, p_start, name);
 +                              printf("%.*s%s", p_len - GIT_SHA1_HEXSZ, p_start, name);
                        else
                                printf("%.*s (%s)", p_len, p_start, name);
                        p_start = p + 1;
        /* flush */
        if (p_start != p)
                fwrite(p_start, p - p_start, 1, stdout);
 +
 +      strbuf_release(&buf);
  }
  
  int cmd_name_rev(int argc, const char **argv, const char *prefix)
                cutoff = 0;
  
        for (; argc; argc--, argv++) {
 -              unsigned char sha1[20];
 +              struct object_id oid;
                struct object *object;
                struct commit *commit;
  
 -              if (get_sha1(*argv, sha1)) {
 +              if (get_oid(*argv, &oid)) {
                        fprintf(stderr, "Could not get sha1 for %s. Skipping.\n",
                                        *argv);
                        continue;
                }
  
                commit = NULL;
 -              object = parse_object(sha1);
 +              object = parse_object(&oid);
                if (object) {
                        struct object *peeled = deref_tag(object, *argv, 0);
                        if (peeled && peeled->type == OBJ_COMMIT)
diff --combined t/t4202-log.sh
index 1c7d6729c699dc8c8784d19659551df24530bd05,038911f2304ac056c74af75a991224a9f54fa08e..6499cdf6ff06b6da3185107f76902cc88767c15d
@@@ -4,7 -4,6 +4,7 @@@ test_description='git log
  
  . ./test-lib.sh
  . "$TEST_DIRECTORY/lib-gpg.sh"
 +. "$TEST_DIRECTORY/lib-terminal.sh"
  
  test_expect_success setup '
  
@@@ -399,7 -398,7 +399,7 @@@ cat > expect <<\EO
  | |
  | |     Merge branch 'side'
  | |
- | * commit side
+ | * commit tags/side-2
  | | Author: A U Thor <author@example.com>
  | |
  | |     side-2
@@@ -521,7 -520,7 +521,7 @@@ test_expect_success 'log --graph with m
  '
  
  test_expect_success 'log.decorate configuration' '
 -      git log --oneline >expect.none &&
 +      git log --oneline --no-decorate >expect.none &&
        git log --oneline --decorate >expect.short &&
        git log --oneline --decorate=full >expect.full &&
  
  
  '
  
 +test_expect_success 'log.decorate config parsing' '
 +      git log --oneline --decorate=full >expect.full &&
 +      git log --oneline --decorate=short >expect.short &&
 +
 +      test_config log.decorate full &&
 +      test_config log.mailmap true &&
 +      git log --oneline >actual &&
 +      test_cmp expect.full actual &&
 +      git log --oneline --decorate=short >actual &&
 +      test_cmp expect.short actual
 +'
 +
 +test_expect_success TTY 'log output on a TTY' '
 +      git log --oneline --decorate >expect.short &&
 +
 +      test_terminal git log --oneline >actual &&
 +      test_cmp expect.short actual
 +'
 +
  test_expect_success 'reflog is expected format' '
        git log -g --abbrev-commit --pretty=oneline >expect &&
        git reflog >actual &&