git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] fmt-merge-msg: show those involved in a merged series

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 5, 2012, 21:34 UTC
Message-ID
<7vmx7uzq8h.fsf_-_@alter.siamese.dyndns.org>
In-Reply-To
<CA+55aFw-sS_p7JXNXbSbpiwh9_bZhSrTtC3is4NtLa_n9Hzk5A@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Of course, since you do it in branches that you maintain, to you
> committer information is pointless. But I pull from submaintainers,
> and then it really is the committer part that is way more relevant.
Fair enough.

I do not think a simple population count on the committer field is a good heuristics. For example, I suspect that you would want to see a lot more weight given to names of committers near the tip of the history you are pulling from, iow, if your lieutenant pulled 100 commit series by a leaf contributor, you still want to see the name of the lieutenant, not the leaf contributor, even the latter may have far more commits.

This patch punts the deep thinking part and just picks the tip committer and counts only merges in the history you are pulling in. It may not be a very good heuristics, but the code structure should be a good place to start.

Changes to the test vector in t6200 illustrate how the output should look like.

-- >8 -- As we already walk the history of the branch that gets merged to come up with a short log, let's label it with names of the primary authors, so that the user who summarizes the merge can easily give credit to them in the log message.

Also infer the names of "lieutents" to help integrators at higher level of the food-chain to give credit to them, by counting:

 * The committer of the 'tip' commit that is merged
 * The committer of merge commits that are merged

Often the first one gives the owner of the history being pulled, but his last pull from his sublieutenants may have been a fast-forward, in which case the first one would not be. The latter rule will count the integrator of the history, so together it might be a reasonable heuristics.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/fmt-merge-msg.c         |  101 +++++++++++++++++++++++++++++++++++++--
 t/t6200-fmt-merge-msg.sh        |   32 +++++++++++--
 t/t7600-merge.sh                |    1 +
 t/t7604-merge-custom-message.sh |    1 +
 4 files changed, 128 insertions(+), 7 deletions(-)
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index c81a7fe..b2465b4 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -27,6 +27,8 @@ int fmt_merge_msg_config(const char *key, const char *value, void *cb)
 			merge_log_config = DEFAULT_MERGE_LOG_LEN;
 	} else if (!strcmp(key, "merge.branchdesc")) {
 		use_branch_desc = git_config_bool(key, value);
+	} else {
+		return git_default_config(key, value, cb);
 	}
 	return 0;
 }
@@ -180,6 +182,89 @@ static void add_branch_desc(struct strbuf *out, const char *name)
 	strbuf_release(&desc);
 }
 
+static void record_person(int which, struct string_list *people,
+			  struct commit *commit)
+{
+	char name_buf[MAX_GITNAME], *name, *name_end;
+	struct string_list_item *elem;
+	const char *field = (which == 'a') ? "\nauthor " : "\ncommitter ";
+
+	name = strstr(commit->buffer, field);
+	if (!name)
+		return;
+	name += strlen(field);
+	name_end = strchrnul(name, '<');
+	if (*name_end)
+		name_end--;
+	while (isspace(*name_end) && name <= name_end)
+		name_end--;
+	if (name_end < name || name + MAX_GITNAME <= name_end)
+		return;
+	memcpy(name_buf, name, name_end - name + 1);
+	name_buf[name_end - name + 1] = '\0';
+
+	elem = string_list_lookup(people, name_buf);
+	if (!elem) {
+		elem = string_list_insert(people, name_buf);
+		elem->util = (void *) 0;
+	}
+	elem->util = (void*)(((intptr_t)elem->util) + 1);
+}
+
+#define util_as_int(elem) ((intptr_t)((elem)->util))
+
+static int cmp_string_list_util_as_int(const void *a_, const void *b_)
+{
+	const struct string_list_item *a = a_, *b = b_;
+	return util_as_int(b) - util_as_int(a);
+}
+
+static void add_people_count(struct strbuf *out, struct string_list *people)
+{
+	if (people->nr == 1)
+		strbuf_addf(out, "%s", people->items[0].string);
+	else if (people->nr == 2)
+		strbuf_addf(out, "%s (%d) and %s (%d)",
+			    people->items[0].string,
+			    (int)util_as_int(&people->items[0]),
+			    people->items[1].string,
+			    (int)util_as_int(&people->items[1]));
+	else if (people->nr)
+		strbuf_addf(out, "%s (%d) and others",
+			    people->items[0].string,
+			    (int)util_as_int(&people->items[0]));
+}
+
+static int committer_is_me(const char *name)
+{
+	int namelen = strlen(name);
+	const char *me = git_committer_info(IDENT_NO_DATE);
+	return (me && !memcmp(me, name, namelen) &&
+		!memcmp(me + namelen, " <", 2));
+}
+
+static void add_people_info(struct strbuf *out,
+			    struct string_list *authors,
+			    struct string_list *committers)
+{
+	if (authors->nr)
+		qsort(authors->items,
+		      authors->nr, sizeof(authors->items[0]),
+		      cmp_string_list_util_as_int);
+	if (committers->nr)
+		qsort(committers->items,
+		      committers->nr, sizeof(committers->items[0]),
+		      cmp_string_list_util_as_int);
+
+	strbuf_addstr(out, "\nBy ");
+	add_people_count(out, authors);
+	if (committers->nr == 1 &&
+	    committer_is_me(committers->items->string))
+		return;
+	strbuf_addstr(out, "\nvia ");
+	add_people_count(out, committers);
+}
+
 static void shortlog(const char *name,
 		     struct origin_data *origin_data,
 		     struct commit *head,
@@ -190,6 +275,8 @@ static void shortlog(const char *name,
 	struct commit *commit;
 	struct object *branch;
 	struct string_list subjects = STRING_LIST_INIT_DUP;
+	struct string_list authors = STRING_LIST_INIT_DUP;
+	struct string_list committers = STRING_LIST_INIT_DUP;
 	int flags = UNINTERESTING | TREESAME | SEEN | SHOWN | ADDED;
 	struct strbuf sb = STRBUF_INIT;
 	const unsigned char *sha1 = origin_data->sha1;
@@ -199,7 +286,6 @@ static void shortlog(const char *name,
 		return;
 
 	setup_revisions(0, NULL, rev, NULL);
-	rev->ignore_merges = 1;
 	add_pending_object(rev, branch, name);
 	add_pending_object(rev, &head->object, "^HEAD");
 	head->object.flags |= UNINTERESTING;
@@ -208,10 +294,15 @@ static void shortlog(const char *name,
 	while ((commit = get_revision(rev)) != NULL) {
 		struct pretty_print_context ctx = {0};
 
-		/* ignore merges */
-		if (commit->parents && commit->parents->next)
+		if (commit->parents && commit->parents->next) {
+			/* do not list a merge but count committer */
+			record_person('c', &committers, commit);
 			continue;
-
+		}
+		if (!count)
+			/* the 'tip' committer */
+			record_person('c', &committers, commit);
+		record_person('a', &authors, commit);
 		count++;
 		if (subjects.nr > limit)
 			continue;
@@ -226,6 +317,7 @@ static void shortlog(const char *name,
 			string_list_append(&subjects, strbuf_detach(&sb, NULL));
 	}
 
+	add_people_info(out, &authors, &committers);
 	if (count > limit)
 		strbuf_addf(out, "\n* %s: (%d commits)\n", name, count);
 	else
@@ -246,6 +338,7 @@ static void shortlog(const char *name,
 	rev->commits = NULL;
 	rev->pending.nr = 0;
 
+	string_list_clear(&authors, 0);
 	string_list_clear(&subjects, 0);
 }
 
diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh
index 9a16806..b90015d 100755
--- a/t/t6200-fmt-merge-msg.sh
+++ b/t/t6200-fmt-merge-msg.sh
@@ -35,15 +35,18 @@ test_expect_success setup '
 
 	echo "l3" >two &&
 	test_tick &&
-	git commit -a -m "Left #3" &&
+	GIT_COMMITTER_NAME="Another Committer" \
+	GIT_AUTHOR_NAME="Another Author" git commit -a -m "Left #3" &&
 
 	echo "l4" >two &&
 	test_tick &&
-	git commit -a -m "Left #4" &&
+	GIT_COMMITTER_NAME="Another Committer" \
+	GIT_AUTHOR_NAME="Another Author" git commit -a -m "Left #4" &&
 
 	echo "l5" >two &&
 	test_tick &&
-	git commit -a -m "Left #5" &&
+	GIT_COMMITTER_NAME="Another Committer" \
+	GIT_AUTHOR_NAME="Another Author" git commit -a -m "Left #5" &&
 	git tag tag-l5 &&
 
 	git checkout right &&
@@ -99,6 +102,8 @@ test_expect_success '[merge] summary/log configuration' '
 	cat >expected <<-EOF &&
 	Merge branch ${apos}left${apos}
 
+	By Another Author (3) and A U Thor (2)
+	via Another Committer
 	* left:
 	  Left #5
 	  Left #4
@@ -144,6 +149,8 @@ test_expect_success 'merge.log=3 limits shortlog length' '
 	cat >expected <<-EOF &&
 	Merge branch ${apos}left${apos}
 
+	By Another Author (3) and A U Thor (2)
+	via Another Committer
 	* left: (5 commits)
 	  Left #5
 	  Left #4
@@ -159,6 +166,8 @@ test_expect_success 'merge.log=5 shows all 5 commits' '
 	cat >expected <<-EOF &&
 	Merge branch ${apos}left${apos}
 
+	By Another Author (3) and A U Thor (2)
+	via Another Committer
 	* left:
 	  Left #5
 	  Left #4
@@ -181,6 +190,8 @@ test_expect_success '--log=3 limits shortlog length' '
 	cat >expected <<-EOF &&
 	Merge branch ${apos}left${apos}
 
+	By Another Author (3) and A U Thor (2)
+	via Another Committer
 	* left: (5 commits)
 	  Left #5
 	  Left #4
@@ -196,6 +207,8 @@ test_expect_success '--log=5 shows all 5 commits' '
 	cat >expected <<-EOF &&
 	Merge branch ${apos}left${apos}
 
+	By Another Author (3) and A U Thor (2)
+	via Another Committer
 	* left:
 	  Left #5
 	  Left #4
@@ -225,6 +238,8 @@ test_expect_success 'fmt-merge-msg -m' '
 	cat >expected.log <<-EOF &&
 	Sync with left
 
+	By Another Author (3) and A U Thor (2)
+	via Another Committer
 	* ${apos}left${apos} of $(pwd):
 	  Left #5
 	  Left #4
@@ -256,6 +271,8 @@ test_expect_success 'setup: expected shortlog for two branches' '
 	cat >expected <<-EOF
 	Merge branches ${apos}left${apos} and ${apos}right${apos}
 
+	By Another Author (3) and A U Thor (2)
+	via Another Committer
 	* left:
 	  Left #5
 	  Left #4
@@ -263,6 +280,7 @@ test_expect_success 'setup: expected shortlog for two branches' '
 	  Common #2
 	  Common #1
 
+	By A U Thor
 	* right:
 	  Right #5
 	  Right #4
@@ -353,6 +371,7 @@ test_expect_success 'merge-msg tag' '
 	cat >expected <<-EOF &&
 	Merge tag ${apos}tag-r3${apos}
 
+	By A U Thor
 	* tag ${apos}tag-r3${apos}:
 	  Right #3
 	  Common #2
@@ -374,11 +393,14 @@ test_expect_success 'merge-msg two tags' '
 	cat >expected <<-EOF &&
 	Merge tags ${apos}tag-r3${apos} and ${apos}tag-l5${apos}
 
+	By A U Thor
 	* tag ${apos}tag-r3${apos}:
 	  Right #3
 	  Common #2
 	  Common #1
 
+	By Another Author (3) and A U Thor (2)
+	via Another Committer
 	* tag ${apos}tag-l5${apos}:
 	  Left #5
 	  Left #4
@@ -402,11 +424,14 @@ test_expect_success 'merge-msg tag and branch' '
 	cat >expected <<-EOF &&
 	Merge branch ${apos}left${apos}, tag ${apos}tag-r3${apos}
 
+	By A U Thor
 	* tag ${apos}tag-r3${apos}:
 	  Right #3
 	  Common #2
 	  Common #1
 
+	By Another Author (3) and A U Thor (2)
+	via Another Committer
 	* left:
 	  Left #5
 	  Left #4
@@ -431,6 +456,7 @@ test_expect_success 'merge-msg lots of commits' '
 		cat <<-EOF &&
 		Merge branch ${apos}long${apos}
 
+		By A U Thor
 		* long: (35 commits)
 		EOF
 
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 9e27bbf..e09b932 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -58,6 +58,7 @@ create_merge_msgs () {
 	} >squash.1-5-9 &&
 	echo >msg.nolog &&
 	{
+		echo "By A U Thor" &&
 		echo "* tag 'c3':" &&
 		echo "  commit 3" &&
 		echo
diff --git a/t/t7604-merge-custom-message.sh b/t/t7604-merge-custom-message.sh
index 89619cf..b84cea3 100755
--- a/t/t7604-merge-custom-message.sh
+++ b/t/t7604-merge-custom-message.sh
@@ -11,6 +11,7 @@ create_merge_msgs() {
 
 	cp exp.subject exp.log &&
 	echo >>exp.log "" &&
+	echo >>exp.log "By A U Thor" &&
 	echo >>exp.log "* tag 'c2':" &&
 	echo >>exp.log "  c2"
 }

-- 
1.7.9.2.413.ge58a8e
Previous: Linus TorvaldsNext: Linus Torvalds
Message 5 of 36 in “A possible fmt-merge-msg update?”
  1. Junio C HamanoMar 5, 2012
  2. Linus TorvaldsMar 5, 2012
  3. Junio C HamanoMar 5, 2012
  4. Linus TorvaldsMar 5, 2012
  5. fmt-merge-msg: show those involved in a merged seriesJunio C Hamano, Mar 5, 2012
  6. Linus TorvaldsMar 5, 2012
  7. Junio C HamanoMar 5, 2012
  8. René ScharfeMar 7, 2012
  9. Junio C HamanoMar 7, 2012
  10. René ScharfeMar 8, 2012
  11. Junio C HamanoMar 8, 2012
  12. Junio C HamanoMar 8, 2012
  13. Phil HordMar 12, 2012
  14. Jeff KingMar 13, 2012
  15. Junio C HamanoMar 14, 2012
  16. Phil HordMar 14, 2012
  17. Jonathan NiederMar 12, 2012
  18. Junio C HamanoMar 13, 2012
  19. Jonathan NiederMar 13, 2012
  20. Junio C HamanoMar 13, 2012
  21. Johannes SixtMar 13, 2012
  22. Junio C HamanoMar 13, 2012
  23. Johannes SixtMar 14, 2012
  24. Junio C HamanoMar 14, 2012
  25. 1/1 fmt-merge-msg: show those involved in a merged seriesJunio C Hamano, Mar 13, 2012
  26. fmt-merge-msg: add a blank line after people infoJonathan Nieder, May 11, 2012
  27. Junio C HamanoMay 11, 2012
  28. Linus TorvaldsMay 11, 2012
  29. Junio C HamanoMay 14, 2012
  30. Junio C HamanoMay 15, 2012
  31. Linus TorvaldsMay 16, 2012
  32. Junio C HamanoMay 16, 2012
  33. Jonathan NiederJun 6, 2012
  34. Jonathan NiederJun 6, 2012
  35. Junio C HamanoJun 6, 2012
  36. Jeff KingMar 6, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.