Merge branch 'jc/name-rev-exact-ref'

Corrects the longstanding sloppiness in the implementation of
name-rev that conflated "we take commit-ish" and "differences
between tags and commits do not matter".

* jc/name-rev-exact-ref:
describe: fix --contains when a tag is given as input
name-rev: differentiate between tags and commits they point at
describe: use argv-array
name-rev: allow converting the exact object name at the tip of a ref
name-ref: factor out name shortening logic from name_ref()

diff --git a/builtin/describe.c b/builtin/describe.c
index 4e675c3d0d7b1daeb7f12ef425514a11a92a5c3f..7d73722f59b10b5fe302498bb83c9ab3313b89b0 100644 (file)
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -7,6 +7,7 @@
 #include "parse-options.h"
 #include "diff.h"
 #include "hash.h"
+#include "argv-array.h"
 
 #define SEEN           (1u<<0)
 #define MAX_TAGS       (FLAG_BITS - 1)
@@ -442,24 +443,24 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
                die(_("--long is incompatible with --abbrev=0"));
 
        if (contains) {
-               const char **args = xmalloc((7 + argc) * sizeof(char *));
-               int i = 0;
-               args[i++] = "name-rev";
-               args[i++] = "--name-only";
-               args[i++] = "--no-undefined";
+               struct argv_array args;
+
+               argv_array_init(&args);
+               argv_array_pushl(&args, "name-rev",
+                                "--peel-tag", "--name-only", "--no-undefined",
+                                NULL);
                if (always)
-                       args[i++] = "--always";
+                       argv_array_push(&args, "--always");
                if (!all) {
-                       args[i++] = "--tags";
-                       if (pattern) {
-                               char *s = xmalloc(strlen("--refs=refs/tags/") + strlen(pattern) + 1);
-                               sprintf(s, "--refs=refs/tags/%s", pattern);
-                               args[i++] = s;
-                       }
+                       argv_array_push(&args, "--tags");
+                       if (pattern)
+                               argv_array_pushf(&args, "--refs=refs/tags/%s", pattern);
+               }
+               while (*argv) {
+                       argv_array_push(&args, *argv);
+                       argv++;
                }
-               memcpy(args + i, argv, argc * sizeof(char *));
-               args[i + argc] = NULL;
-               return cmd_name_rev(i + argc, args, prefix);
+               return cmd_name_rev(args.argc, args.argv, prefix);
        }
 
        init_hash(&names);

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 87d485496fd5556cbd0f24793ed7e48f11e9b292..0aaa19e4abeb462b2f1d05d5e03aa29bee810db8 100644 (file)
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -4,6 +4,7 @@
 #include "tag.h"
 #include "refs.h"
 #include "parse-options.h"
+#include "sha1-lookup.h"
 
 #define CUTOFF_DATE_SLOP 86400 /* one day */
 
@@ -96,12 +97,51 @@ static int subpath_matches(const char *path, const char *filter)
        return -1;
 }
 
+static const char *name_ref_abbrev(const char *refname, int shorten_unambiguous)
+{
+       if (shorten_unambiguous)
+               refname = shorten_unambiguous_ref(refname, 0);
+       else if (!prefixcmp(refname, "refs/heads/"))
+               refname = refname + 11;
+       else if (!prefixcmp(refname, "refs/"))
+               refname = refname + 5;
+       return refname;
+}
+
 struct name_ref_data {
        int tags_only;
        int name_only;
        const char *ref_filter;
 };
 
+static struct tip_table {
+       struct tip_table_entry {
+               unsigned char sha1[20];
+               const char *refname;
+       } *table;
+       int nr;
+       int alloc;
+       int sorted;
+} tip_table;
+
+static void add_to_tip_table(const unsigned char *sha1, 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);
+       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);
+}
+
 static int name_ref(const char *path, const unsigned char *sha1, int flags, void *cb_data)
 {
        struct object *o = parse_object(sha1);
@@ -124,6 +164,8 @@ static int name_ref(const char *path, const unsigned char *sha1, int flags, void
                }
        }
 
+       add_to_tip_table(sha1, path, can_abbreviate_output);
+
        while (o && o->type == OBJ_TAG) {
                struct tag *t = (struct tag *) o;
                if (!t->tagged)
@@ -134,18 +176,38 @@ static int name_ref(const char *path, const unsigned char *sha1, int flags, void
        if (o && o->type == OBJ_COMMIT) {
                struct commit *commit = (struct commit *)o;
 
-               if (can_abbreviate_output)
-                       path = shorten_unambiguous_ref(path, 0);
-               else if (!prefixcmp(path, "refs/heads/"))
-                       path = path + 11;
-               else if (!prefixcmp(path, "refs/"))
-                       path = path + 5;
-
+               path = name_ref_abbrev(path, can_abbreviate_output);
                name_rev(commit, xstrdup(path), 0, 0, 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;
+}
+
+static const char *get_exact_ref_match(const struct object *o)
+{
+       int found;
+
+       if (!tip_table.table || !tip_table.nr)
+               return NULL;
+
+       if (!tip_table.sorted) {
+               qsort(tip_table.table, tip_table.nr, sizeof(*tip_table.table),
+                     tipcmp);
+               tip_table.sorted = 1;
+       }
+
+       found = sha1_pos(o->sha1, tip_table.table, tip_table.nr,
+                        nth_tip_table_ent);
+       if (0 <= found)
+               return tip_table.table[found].refname;
+       return NULL;
+}
+
 /* returns a static buffer */
 static const char *get_rev_name(const struct object *o)
 {
@@ -154,7 +216,7 @@ static const char *get_rev_name(const struct object *o)
        struct commit *c;
 
        if (o->type != OBJ_COMMIT)
-               return NULL;
+               return get_exact_ref_match(o);
        c = (struct commit *) o;
        n = c->util;
        if (!n)
@@ -245,7 +307,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)
 int cmd_name_rev(int argc, const char **argv, const char *prefix)
 {
        struct object_array revs = OBJECT_ARRAY_INIT;
-       int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0;
+       int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
        struct name_ref_data data = { 0, 0, NULL };
        struct option opts[] = {
                OPT_BOOLEAN(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
@@ -258,6 +320,12 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
                OPT_BOOLEAN(0, "undefined", &allow_undefined, N_("allow to print `undefined` names")),
                OPT_BOOLEAN(0, "always",     &always,
                           N_("show abbreviated commit object as fallback")),
+               {
+                       /* A Hidden OPT_BOOL */
+                       OPTION_SET_INT, 0, "peel-tag", &peel_tag, NULL,
+                       N_("dereference tags in the input (internal use)"),
+                       PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1,
+               },
                OPT_END(),
        };
 
@@ -272,7 +340,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 
        for (; argc; argc--, argv++) {
                unsigned char sha1[20];
-               struct object *o;
+               struct object *object;
                struct commit *commit;
 
                if (get_sha1(*argv, sha1)) {
@@ -281,17 +349,34 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
                        continue;
                }
 
-               o = deref_tag(parse_object(sha1), *argv, 0);
-               if (!o || o->type != OBJ_COMMIT) {
-                       fprintf(stderr, "Could not get commit for %s. Skipping.\n",
+               commit = NULL;
+               object = parse_object(sha1);
+               if (object) {
+                       struct object *peeled = deref_tag(object, *argv, 0);
+                       if (peeled && peeled->type == OBJ_COMMIT)
+                               commit = (struct commit *)peeled;
+               }
+
+               if (!object) {
+                       fprintf(stderr, "Could not get object for %s. Skipping.\n",
                                        *argv);
                        continue;
                }
 
-               commit = (struct commit *)o;
-               if (cutoff > commit->date)
-                       cutoff = commit->date;
-               add_object_array((struct object *)commit, *argv, &revs);
+               if (commit) {
+                       if (cutoff > commit->date)
+                               cutoff = commit->date;
+               }
+
+               if (peel_tag) {
+                       if (!commit) {
+                               fprintf(stderr, "Could not get commit for %s. Skipping.\n",
+                                       *argv);
+                               continue;
+                       }
+                       object = (struct object *)commit;
+               }
+               add_object_array(object, *argv, &revs);
        }
 
        if (cutoff)

diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index a25729f2a7bb4c4d4f755ef5f855261ee1d71645..c0e5b2a6275df2d92172327b2d629cc73720423e 100755 (executable)
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -174,4 +174,28 @@ check_describe "test2-lightweight-*" --tags --match="test2-*"
 
 check_describe "test2-lightweight-*" --long --tags --match="test2-*" HEAD^
 
+test_expect_success 'name-rev with exact tags' '
+       echo A >expect &&
+       tag_object=$(git rev-parse refs/tags/A) &&
+       git name-rev --tags --name-only $tag_object >actual &&
+       test_cmp expect actual &&
+
+       echo "A^0" >expect &&
+       tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+       git name-rev --tags --name-only $tagged_commit >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success 'describe --contains with the exact tags' '
+       echo "A^0" >expect &&
+       tag_object=$(git rev-parse refs/tags/A) &&
+       git describe --contains $tag_object >actual &&
+       test_cmp expect actual &&
+
+       echo "A^0" >expect &&
+       tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+       git describe --contains $tagged_commit >actual &&
+       test_cmp expect actual
+'
+
 test_done