Merge branch 'js/name-rev-use-oldest-ref' into maint
authorJunio C Hamano <gitster@pobox.com>
Tue, 31 May 2016 21:08:26 +0000 (14:08 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 31 May 2016 21:08:26 +0000 (14:08 -0700)
"git describe --contains" often made a hard-to-justify choice of
tag to give name to a given commit, because it tried to come up
with a name with smallest number of hops from a tag, causing an old
commit whose close descendant that is recently tagged were not
described with respect to an old tag but with a newer tag. It did
not help that its computation of "hop" count was further tweaked to
penalize being on a side branch of a merge. The logic has been
updated to favor using the tag with the oldest tagger date, which
is a lot easier to explain to the end users: "We describe a commit
in terms of the (chronologically) oldest tag that contains the
commit."

* js/name-rev-use-oldest-ref:
name-rev: include taggerdate in considering the best name

1  2 
builtin/name-rev.c
t/t9903-bash-prompt.sh
diff --combined builtin/name-rev.c
index 092e03c3cc9b24445ee5ed92cee5b202e04c9ec7,63b65885cbdcf8f79cd62dfe3cf061416d84109a..57be35faf583d5bcb112c9f487b67ae4df8bf368
@@@ -10,6 -10,7 +10,7 @@@
  
  typedef struct rev_name {
        const char *tip_name;
+       unsigned long taggerdate;
        int generation;
        int distance;
  } rev_name;
@@@ -20,7 -21,8 +21,8 @@@ static long cutoff = LONG_MAX
  #define MERGE_TRAVERSAL_WEIGHT 65535
  
  static void name_rev(struct commit *commit,
-               const char *tip_name, int generation, int distance,
+               const char *tip_name, unsigned long taggerdate,
+               int generation, int distance,
                int deref)
  {
        struct rev_name *name = (struct rev_name *)commit->util;
                name = xmalloc(sizeof(rev_name));
                commit->util = name;
                goto copy_data;
-       } else if (name->distance > distance) {
+       } else if (name->taggerdate > taggerdate ||
+                       (name->taggerdate == taggerdate &&
+                        name->distance > distance)) {
  copy_data:
                name->tip_name = tip_name;
+               name->taggerdate = taggerdate;
                name->generation = generation;
                name->distance = distance;
        } else
                        parents;
                        parents = parents->next, parent_number++) {
                if (parent_number > 1) {
 -                      int len = strlen(tip_name);
 -                      char *new_name = xmalloc(len +
 -                              1 + decimal_length(generation) +  /* ~<n> */
 -                              1 + 2 +                           /* ^NN */
 -                              1);
 -
 -                      if (len > 2 && !strcmp(tip_name + len - 2, "^0"))
 -                              len -= 2;
 +                      size_t len;
 +                      char *new_name;
 +
 +                      strip_suffix(tip_name, "^0", &len);
                        if (generation > 0)
 -                              sprintf(new_name, "%.*s~%d^%d", len, tip_name,
 -                                              generation, parent_number);
 +                              new_name = xstrfmt("%.*s~%d^%d", (int)len, tip_name,
 +                                                 generation, parent_number);
                        else
 -                              sprintf(new_name, "%.*s^%d", len, tip_name,
 -                                              parent_number);
 +                              new_name = xstrfmt("%.*s^%d", (int)len, tip_name,
 +                                                 parent_number);
  
-                       name_rev(parents->item, new_name, 0,
+                       name_rev(parents->item, new_name, taggerdate, 0,
                                distance + MERGE_TRAVERSAL_WEIGHT, 0);
                } else {
-                       name_rev(parents->item, tip_name, generation + 1,
-                               distance + 1, 0);
+                       name_rev(parents->item, tip_name, taggerdate,
+                               generation + 1, distance + 1, 0);
                }
        }
  }
@@@ -134,12 -143,13 +139,13 @@@ static int tipcmp(const void *a_, cons
        return hashcmp(a->sha1, b->sha1);
  }
  
 -static int name_ref(const char *path, const unsigned char *sha1, int flags, void *cb_data)
 +static int name_ref(const char *path, const struct object_id *oid, int flags, void *cb_data)
  {
 -      struct object *o = parse_object(sha1);
 +      struct object *o = parse_object(oid->hash);
        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;
  
        if (data->tags_only && !starts_with(path, "refs/tags/"))
                return 0;
                }
        }
  
 -      add_to_tip_table(sha1, path, can_abbreviate_output);
 +      add_to_tip_table(oid->hash, 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->sha1);
 +              o = parse_object(t->tagged->oid.hash);
                deref = 1;
+               taggerdate = t->date;
        }
        if (o && o->type == OBJ_COMMIT) {
                struct commit *commit = (struct commit *)o;
  
                path = name_ref_abbrev(path, can_abbreviate_output);
-               name_rev(commit, xstrdup(path), 0, 0, deref);
+               name_rev(commit, xstrdup(path), taggerdate, 0, 0, deref);
        }
        return 0;
  }
@@@ -193,7 -204,7 +200,7 @@@ static const char *get_exact_ref_match(
                tip_table.sorted = 1;
        }
  
 -      found = sha1_pos(o->sha1, tip_table.table, tip_table.nr,
 +      found = sha1_pos(o->oid.hash, tip_table.table, tip_table.nr,
                         nth_tip_table_ent);
        if (0 <= found)
                return tip_table.table[found].refname;
@@@ -232,19 -243,19 +239,19 @@@ static void show_name(const struct obje
                      int always, int allow_undefined, int name_only)
  {
        const char *name;
 -      const unsigned char *sha1 = obj->sha1;
 +      const struct object_id *oid = &obj->oid;
  
        if (!name_only)
 -              printf("%s ", caller_name ? caller_name : sha1_to_hex(sha1));
 +              printf("%s ", caller_name ? caller_name : oid_to_hex(oid));
        name = get_rev_name(obj);
        if (name)
                printf("%s\n", name);
        else if (allow_undefined)
                printf("undefined\n");
        else if (always)
 -              printf("%s\n", find_unique_abbrev(sha1, DEFAULT_ABBREV));
 +              printf("%s\n", find_unique_abbrev(oid->hash, DEFAULT_ABBREV));
        else
 -              die("cannot describe '%s'", sha1_to_hex(sha1));
 +              die("cannot describe '%s'", oid_to_hex(oid));
  }
  
  static char const * const name_rev_usage[] = {
diff --combined t/t9903-bash-prompt.sh
index ffbfa0efb8712f06cb14adc986ae29c4d1c23bf1,9265976dcfcdfba41c7dc63bd3b4932ada1b2ef4..0db4469c89492ae625307629cb7132178c3b9671
@@@ -67,7 -67,7 +67,7 @@@ repo_with_newline='rep
  with
  newline'
  
 -if mkdir "$repo_with_newline" 2>/dev/null
 +if test_have_prereq !MINGW && mkdir "$repo_with_newline" 2>/dev/null
  then
        test_set_prereq FUNNYNAMES
  else
@@@ -107,7 -107,7 +107,7 @@@ test_expect_success 'prompt - describe 
  '
  
  test_expect_success 'prompt - describe detached head - branch' '
-       printf " ((b1~1))" >expected &&
+       printf " ((tags/t2~1))" >expected &&
        git checkout b1^ &&
        test_when_finished "git checkout master" &&
        (
@@@ -273,36 -273,11 +273,36 @@@ test_expect_success 'prompt - dirty sta
        test_cmp expected "$actual"
  '
  
 -test_expect_success 'prompt - dirty status indicator - before root commit' '
 -      printf " (master #)" >expected &&
 +test_expect_success 'prompt - dirty status indicator - orphan branch - clean' '
 +      printf " (orphan #)" >expected &&
 +      test_when_finished "git checkout master" &&
 +      git checkout --orphan orphan &&
 +      git reset --hard &&
 +      (
 +              GIT_PS1_SHOWDIRTYSTATE=y &&
 +              __git_ps1 >"$actual"
 +      ) &&
 +      test_cmp expected "$actual"
 +'
 +
 +test_expect_success 'prompt - dirty status indicator - orphan branch - dirty index' '
 +      printf " (orphan +)" >expected &&
 +      test_when_finished "git checkout master" &&
 +      git checkout --orphan orphan &&
 +      (
 +              GIT_PS1_SHOWDIRTYSTATE=y &&
 +              __git_ps1 >"$actual"
 +      ) &&
 +      test_cmp expected "$actual"
 +'
 +
 +test_expect_success 'prompt - dirty status indicator - orphan branch - dirty index and worktree' '
 +      printf " (orphan *+)" >expected &&
 +      test_when_finished "git checkout master" &&
 +      git checkout --orphan orphan &&
 +      >file &&
        (
                GIT_PS1_SHOWDIRTYSTATE=y &&
 -              cd otherrepo &&
                __git_ps1 >"$actual"
        ) &&
        test_cmp expected "$actual"
@@@ -418,31 -393,6 +418,31 @@@ test_expect_success 'prompt - untracke
        (
                GIT_PS1_SHOWUNTRACKEDFILES=y &&
                __git_ps1 >"$actual"
 +      ) &&
 +      test_cmp expected "$actual"
 +'
 +
 +test_expect_success 'prompt - untracked files status indicator - empty untracked dir' '
 +      printf " (master)" >expected &&
 +      mkdir otherrepo/untracked-dir &&
 +      test_when_finished "rm -rf otherrepo/untracked-dir" &&
 +      (
 +              GIT_PS1_SHOWUNTRACKEDFILES=y &&
 +              cd otherrepo &&
 +              __git_ps1 >"$actual"
 +      ) &&
 +      test_cmp expected "$actual"
 +'
 +
 +test_expect_success 'prompt - untracked files status indicator - non-empty untracked dir' '
 +      printf " (master %%)" >expected &&
 +      mkdir otherrepo/untracked-dir &&
 +      test_when_finished "rm -rf otherrepo/untracked-dir" &&
 +      >otherrepo/untracked-dir/untracked-file &&
 +      (
 +              GIT_PS1_SHOWUNTRACKEDFILES=y &&
 +              cd otherrepo &&
 +              __git_ps1 >"$actual"
        ) &&
        test_cmp expected "$actual"
  '