name-rev: Fix tag lookup on repository with mixed types of tags

From: Orgad Shaneh <[email protected]>

Commit 7550424804 (name-rev: include taggerdate in considering the best name) introduced a bug in name-rev.

If a repository has both annotated and non-annotated tags, annotated tag will always win, even if it was created decades after the commit.

Consider a repository that always used non-annotated tags, and at some point started using annotated tags - name-rev --tags will return the first annotated tags for all the old commits (in our repository it is followed by ~5067 for one commit, or by ~120^2~21^2~88^2~87 for another...). This is obviously not what the user expects.

There was an attempt to fix this in ef1e74065, but it is not enough.

The taggerdate should only be matched if *both tags* have it.

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

index 9e088ebd11..dc9eaf21fa 100644
@@ -35,7 +35,7 @@ static int is_better_name(struct rev_name *name,
 	 */
 	if (from_tag && name->from_tag)
 		return (name->taggerdate > taggerdate ||
-			(name->taggerdate == taggerdate &&
+			((taggerdate == TIME_MAX || name->taggerdate == taggerdate) &&
 			 name->distance > distance));
 
 	/*

@@ -53,7 +53,7 @@ static int is_better_name(struct rev_name *name,
 		return name->distance > distance;
 
 	/* ... or tiebreak to favor older date */
-	if (name->taggerdate != taggerdate)
+	if (taggerdate != TIME_MAX && name->taggerdate != taggerdate)
 		return name->taggerdate > taggerdate;
 
 	/* keep the current one if we cannot decide */

@@ -90,7 +90,8 @@ static void name_rev(struct commit *commit,
 				  generation, distance, from_tag)) {
 copy_data:
 		name->tip_name = tip_name;
-		name->taggerdate = taggerdate;
+		if (taggerdate != TIME_MAX)
+			name->taggerdate = taggerdate;
 		name->generation = generation;
 		name->distance = distance;
 		name->from_tag = from_tag;

2.14.2.windows.3

Can you update a test to protect the change this patch makes from future breakages? That would also demonstrate how the current code is broken and this patch improves the situation, as I am having a bit of trouble seeing why this change is needed.

Thanks.