name-rev: favor describing with tags and use committer date to tiebreakjc/name-rev

"git name-rev" assigns a phony "far in the future" taggerdate to the
tips of refs that are not pointing at tag objects, and picks the
name based on the ref with the oldest taggerdate.

This makes it almost impossible for unannotated tags and branches to
be counted as viable bases for names, which is problematic when the
command is run with the "--tags" option.  If an unannotated tag that
points at an ancient commit and an annotated tag that points at a
much newer commit both reach the commit that is being named, the
name based on the older unannotated tag would generally be a better
name, based on an older tag with shorter hops.

Update the "taggerdate" field of the rev-name structure, which keeps
track of the age of the tip of ref, to have the committer date if
the object at the tip of ref is a commit, not a tag, so that we can
optionally take it into account when doing "is this name better?"
comparison logic.

When "name-rev" is run without the "--tags" option, the general
expectation is still to name the commit based on a tag if possible,
but use non-tag refs as fallback, and tiebreak among these non-tag
refs by favoring names with shorter hops from the tip.  The use of a
phony "far in the future" taggerdate in the original code is an
effective way to ensure that this expectation is held: non-tag tips
get the same "far in the future" taggerdate, giving precedence to
tags with real taggerdates, and among non-tag tips, names with
shorter hops are preferred over longer hops, without taking the
"taggerdate" into account.

As we are taking over the "taggerdate" field to store the committer
date for tips with commits:

 (1) Keep the original logic when comparing names based on two refs
     both of which are from refs/tags/.  This allows both annotated
     and unannotated tags to be counted with precedence over other
     refs.

 (2) Favor a name based on a ref in refs/tags/ hierarchy over a ref
     outside the hierarchy.

 (3) Between two names based on a ref both outside refs/tags/, give
     precedence to a name with shorter hops and use "taggerdate"
     only to tie-break.

An update to t4202 in this patch is a natural consequence of the
above.  The test creates a commit on the "side" branch and points at
it with the "side-2" tag that is unannotated.  The original code
couldn't decide which one to favor at all, and gave a name based on
the branch, simply because refs/heads/side sorts earlier than
refs/tags/side-2.

Because the updated logic is taught to favor refs in the refs/tags/
hierarchy, the the test is updated to expect to see "tags/side-2"
used as the name.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

2 files changed, 49 insertions, 10 deletions

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index f64c71d9bc..eac0180c62 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;
@@ -24,16 +25,47 @@ static int is_better_name(struct rev_name *name,
 			  const char *tip_name,
 			  unsigned long taggerdate,
 			  int generation,
-			  int distance)
+			  int distance,
+			  int from_tag)
 {
-	return (name->taggerdate > taggerdate ||
-		(name->taggerdate == taggerdate &&
-		 name->distance > distance));
+	/*
+	 * 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));
+
+#define COMPARE(attribute, smaller_is_better)	 \
+	if (name->attribute > attribute) \
+		return smaller_is_better; \
+	if (name->attribute < attribute) \
+		return !smaller_is_better
+
+	/*
+	 * We know that at least one of them is a non-tag at this point.
+	 * favor a tag over a non-tag.
+	 */
+	COMPARE(from_tag, 0);
+
+	/*
+	 * We are now looking at two non-tags.  Tiebreak to favor
+	 * shorter hops.
+	 */
+	COMPARE(distance, 1);
+
+	/* ... or tiebreak to favor older date */
+	COMPARE(taggerdate, 1);
+
+	/* keep the current one if we cannot decide */
+	return 0;
+#undef COMPARE
 }
 
 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;
@@ -57,12 +89,13 @@ static void name_rev(struct commit *commit,
 		commit->util = name;
 		goto copy_data;
 	} else if (is_better_name(name, tip_name, taggerdate,
-				  generation, distance)) {
+				  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;
 
@@ -82,10 +115,12 @@ copy_data:
 						   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);
 		}
 	}
 }
@@ -184,9 +219,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;
 }
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 48b55bfd27..038911f230 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -398,7 +398,7 @@ cat > expect <<\EOF
 | |
 | |     Merge branch 'side'
 | |
-| * commit side
+| * commit tags/side-2
 | | Author: A U Thor <author@example.com>
 | |
 | |     side-2