Merge branch 'js/name-rev-use-oldest-ref' into maint

gitster · gitster · commit 9ee8f9409cab · 2016-05-31T14:08:26.000-07:00
"git describe --contains" often made a hard-to-justify choice of
tag to give name to a given commit, because it tried to come up
with a name with smallest number of hops from a tag, causing an old
commit whose close descendant that is recently tagged were not
described with respect to an old tag but with a newer tag.  It did
not help that its computation of "hop" count was further tweaked to
penalize being on a side branch of a merge.  The logic has been
updated to favor using the tag with the oldest tagger date, which
is a lot easier to explain to the end users: "We describe a commit
in terms of the (chronologically) oldest tag that contains the
commit."

* js/name-rev-use-oldest-ref:
  name-rev: include taggerdate in considering the best name
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
@@ -10,6 +10,7 @@
 
 typedef struct rev_name {
 	const char *tip_name;
+	unsigned long taggerdate;
 	int generation;
 	int distance;
 } rev_name;
@@ -20,7 +21,8 @@ static long cutoff = LONG_MAX;
 #define MERGE_TRAVERSAL_WEIGHT 65535
 
 static void name_rev(struct commit *commit,
-		const char *tip_name, int generation, int distance,
+		const char *tip_name, unsigned long taggerdate,
+		int generation, int distance,
 		int deref)
 {
 	struct rev_name *name = (struct rev_name *)commit->util;
@@ -43,9 +45,12 @@ static void name_rev(struct commit *commit,
 		name = xmalloc(sizeof(rev_name));
 		commit->util = name;
 		goto copy_data;
-	} else if (name->distance > distance) {
+	} else if (name->taggerdate > taggerdate ||
+			(name->taggerdate == taggerdate &&
+			 name->distance > distance)) {
 copy_data:
 		name->tip_name = tip_name;
+		name->taggerdate = taggerdate;
 		name->generation = generation;
 		name->distance = distance;
 	} else
@@ -66,11 +71,11 @@ static void name_rev(struct commit *commit,
 				new_name = xstrfmt("%.*s^%d", (int)len, tip_name,
 						   parent_number);
 
-			name_rev(parents->item, new_name, 0,
+			name_rev(parents->item, new_name, taggerdate, 0,
 				distance + MERGE_TRAVERSAL_WEIGHT, 0);
 		} else {
-			name_rev(parents->item, tip_name, generation + 1,
-				distance + 1, 0);
+			name_rev(parents->item, tip_name, taggerdate,
+				generation + 1, distance + 1, 0);
 		}
 	}
 }
@@ -140,6 +145,7 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
 	struct name_ref_data *data = cb_data;
 	int can_abbreviate_output = data->tags_only && data->name_only;
 	int deref = 0;
+	unsigned long taggerdate = ULONG_MAX;
 
 	if (data->tags_only && !starts_with(path, "refs/tags/"))
 		return 0;
@@ -164,12 +170,13 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
 			break; /* broken repository */
 		o = parse_object(t->tagged->oid.hash);
 		deref = 1;
+		taggerdate = t->date;
 	}
 	if (o && o->type == OBJ_COMMIT) {
 		struct commit *commit = (struct commit *)o;
 
 		path = name_ref_abbrev(path, can_abbreviate_output);
-		name_rev(commit, xstrdup(path), 0, 0, deref);
+		name_rev(commit, xstrdup(path), taggerdate, 0, 0, deref);
 	}
 	return 0;
 }
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
@@ -107,7 +107,7 @@ test_expect_success 'prompt - describe detached head - contains' '
 '
 
 test_expect_success 'prompt - describe detached head - branch' '
-	printf " ((b1~1))" >expected &&
+	printf " ((tags/t2~1))" >expected &&
 	git checkout b1^ &&
 	test_when_finished "git checkout master" &&
 	(

Original file line number	Diff line number	Diff line change
`@@ -107,7 +107,7 @@ test_expect_success 'prompt - describe detached head - contains' '`
`107`	`107`	`'`
`108`	`108`
`109`	`109`	`test_expect_success 'prompt - describe detached head - branch' '`
`110`		`- printf " ((b1~1))" >expected &&`
	`110`	`+ printf " ((tags/t2~1))" >expected &&`
`111`	`111`	`git checkout b1^ &&`
`112`	`112`	`test_when_finished "git checkout master" &&`
`113`	`113`	`(`