Re: [RFD PATCH 3/3] name-rev: Allow lightweight tags and branch refs

[Junio C Hamano]

Junio C Hamano <gits...@pobox.com> writes:

> This could have been an invocation of "name-rev" without "--tags",
> where _any_ tip of ref can be used to name a revision, and in such a
> case, retaining commit->date may still be valuable, but it is
> probably wrong to use it for nothing more than tie-breaking.
> ...
> I think you can do this change _ONLY_ when we are operating under
> the "--tags" option.  That would place unannotated tags at a better
> location in the timestamp order than the current code does, without
> introducing issues with refs that are actively moving.

I'll leave "only do so under --tags" as an exercise to the reader,
but if we want to use the timestamp for tie-breaking between names
based on two branches, a patch to do so may look like this one.  

I am presenting it as a single ball of wax, but in the real history
I have (which I am not publishing, as I haven't decided if this is a
good direction to go in the first place), this is a two-patch
series, the first one that factors out is_better_name() without
doing anything else, followed by a commit that adds "from_tag" and
passes it throughout the callpath to allow is_better_name() to use
it to:

 - always favor names based on a tag;

 - between two names based on tags, favor name based on an older tag
   and tiebreak with distance;

 - between two names based on non-tags, favor name with shorter hops
   and tiebreak with age.

 builtin/name-rev.c | 43 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index cd89d48b65..ba30c9ca23 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -13,6 +13,7 @@ typedef struct rev_name {
        unsigned long taggerdate;
        int generation;
        int distance;
+       int from_tag;
 } rev_name;
 
 static long cutoff = LONG_MAX;
@@ -20,9 +21,31 @@ static long cutoff = LONG_MAX;
 /* How many generations are maximally preferred over _one_ merge traversal? */
 #define MERGE_TRAVERSAL_WEIGHT 65535
 
+static int is_better_name(struct rev_name *name,
+                         const char *tip_name,
+                         unsigned long taggerdate,
+                         int generation,
+                         int distance,
+                         int from_tag)
+{
+       if (name->from_tag > from_tag)
+               return 0;
+       else if (name->from_tag < from_tag)
+               return 1;
+
+       if (from_tag)
+               return (name->taggerdate > taggerdate ||
+                       (name->taggerdate == taggerdate &&
+                        name->distance > distance));
+       else
+               return (name->distance > distance ||
+                       (name->distance == distance &&
+                        name->taggerdate > taggerdate));
+}
+
 static void name_rev(struct commit *commit,
                const char *tip_name, unsigned long taggerdate,
-               int generation, int distance,
+               int generation, int distance, int from_tag,
                int deref)
 {
        struct rev_name *name = (struct rev_name *)commit->util;
@@ -45,14 +68,14 @@ static void name_rev(struct commit *commit,
                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;
+               name->from_tag = from_tag;
        } else
                return;
 
@@ -72,10 +95,12 @@ static void name_rev(struct commit *commit,
                                                   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);
                }
        }
 }
@@ -174,9 +199,13 @@ static int name_ref(const char *path, const struct 
object_id *oid, int flags, vo
        }
        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;
 }