Junio C Hamano <gitster@xxxxxxxxx> 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;
}