"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
limit commit notification history traversal
To:
gameoftrees@openbsd.org
Date:
Sun, 17 Aug 2025 20:20:34 +0200

Download raw body.

Thread
  • Stefan Sperling:

    limit commit notification history traversal

I am getting tired of the long history traversal in commit emails
when we send branches to the repository other than main.

This change limits notification history traversal to commits which
are unique to the branch being created or changed.

As a bonus, this will show all commits added on a newly created branch,
rather than just showing the tip commit.

ok?


make commit notifications only show the part of history unique to the branch

When a new branch is created, show all commits added on the branch relative
to the HEAD branch, instead of showing just the tip commit.

When branch history is changed with got send -f or git push -f, limit
history traversal to a common ancestor with the HEAD branch, instead
of walking the entire history until a root commit is found.

Stacked branches will still show some extra history until they hit HEAD.
This can be improved later.

M  gotd/repo_write.c                   |   44+  3-
M  regress/gotd/email_notification.sh  |  120+  5-

2 files changed, 164 insertions(+), 8 deletions(-)

commit - 2d67dc3aa72fb921c2b7ddcf2af6814acf2784c2
commit + ac9f17303e79c90b53aebba4b32db82e1f0a2470
blob - 29c0f8bde535b60c18b781ecf842b2684f50cc7c
blob + 92ca7ea423d643e5a5511e6f27d2067386929373
--- gotd/repo_write.c
+++ gotd/repo_write.c
@@ -1943,17 +1943,53 @@ print_commits(struct got_object_id *root_id, struct go
     struct got_repository *repo, int fd)
 {
 	const struct got_error *err;
-	struct got_commit_graph *graph;
+	struct got_commit_graph *graph = NULL;
 	struct got_object_id_queue reversed_commits;
 	struct got_object_qid *qid;
 	struct got_commit_object *commit = NULL;
 	struct got_pathlist_head changed_paths;
 	int ncommits = 0;
 	const int shortlog_threshold = 50;
+	struct got_object_id *yca_id = NULL;
+	struct got_reference *head_ref = NULL;
+	struct got_object_id *head_id = NULL;
 
 	STAILQ_INIT(&reversed_commits);
 	RB_INIT(&changed_paths);
 
+	if (end_id && got_object_id_cmp(root_id, end_id) != 0) {
+		err = got_commit_graph_find_youngest_common_ancestor(
+		    &yca_id, root_id, end_id, 1, 0, repo_write.repo,
+		    check_cancelled, NULL);
+		if (err)
+			return err;
+		if (yca_id)
+			end_id = yca_id;
+	}
+
+	if (end_id == NULL) {
+		err = got_ref_open(&head_ref, repo_write.repo, GOT_REF_HEAD, 0);
+		if (err) {
+			if (err->code != GOT_ERR_NOT_REF)
+				goto done;
+		} else {
+			err = got_ref_resolve(&head_id, repo_write.repo,
+			    head_ref);
+			if (err)
+				goto done;
+			if (got_object_id_cmp(root_id, head_id) != 0) {
+				err =
+				got_commit_graph_find_youngest_common_ancestor(
+				    &yca_id, root_id, head_id, 1, 0,
+				    repo_write.repo, check_cancelled, NULL);
+				if (err)
+					goto done;
+				if (yca_id)
+					end_id = yca_id;
+			}
+		}
+	}
+
 	/* XXX first-parent only for now */
 	err = got_commit_graph_open(&graph, "/", 1);
 	if (err)
@@ -2018,11 +2054,16 @@ print_commits(struct got_object_id *root_id, struct go
 		got_pathlist_free(&changed_paths, GOT_PATHLIST_FREE_ALL);
 	}
 done:
+	free(yca_id);
+	free(head_id);
+	if (head_ref)
+		got_ref_close(head_ref);
 	if (commit)
 		got_object_commit_close(commit);
 	got_object_id_queue_free(&reversed_commits);
 	got_pathlist_free(&changed_paths, GOT_PATHLIST_FREE_ALL);
-	got_commit_graph_close(graph);
+	if (graph)
+		got_commit_graph_close(graph);
 	return err;
 }
 
@@ -2135,7 +2176,7 @@ static const struct got_error *
 notify_created_ref(const char *refname, struct got_object_id *id,
     struct gotd_imsgev *iev, int fd)
 {
-	const struct got_error *err;
+	const struct got_error *err = NULL;
 	int obj_type;
 
 	err = got_object_get_type(&obj_type, repo_write.repo, id);
blob - b006f86ca11e0d1c8226fdee8bd96e7d6dfee32f
blob + 227b9295acd94b7780e5a55e82a4e86a08d5e3e0
--- regress/gotd/email_notification.sh
+++ regress/gotd/email_notification.sh
@@ -286,10 +286,15 @@ test_branch_created() {
 	(cd $testroot/wt && got branch newbranch > /dev/null)
 
 	echo "change alpha on branch" > $testroot/wt/alpha
-	(cd $testroot/wt && got commit -m 'newbranch' > /dev/null)
+	(cd $testroot/wt && got commit -m 'change alpha on newbranch' \
+		> /dev/null)
 	local commit_id=`git_show_branch_head $testroot/repo-clone newbranch`
 	local author_time=`git_show_author_time $testroot/repo-clone $commit_id`
 
+	echo "change alpha again" > $testroot/wt/alpha
+	(cd $testroot/wt && got commit -m 'change alpha again' > /dev/null)
+	local commit_id2=`git_show_branch_head $testroot/repo-clone newbranch`
+	local author_time2=`git_show_author_time $testroot/repo-clone $commit_id`
 	(printf "220\r\n250\r\n250\r\n250\r\n354\r\n250\r\n221\r\n" \
 		| timeout 5 nc -l "$GOTD_TEST_SMTP_PORT" > $testroot/stdout) &
 
@@ -305,6 +310,7 @@ test_branch_created() {
 
 	wait %1 # wait for nc -l
 
+	short_commit_id2=`trim_obj_id 12 $commit_id2`
 	short_commit_id=`trim_obj_id 12 $commit_id`
 	HOSTNAME=`hostname`
 	printf "HELO localhost\r\n" > $testroot/stdout.expected
@@ -315,19 +321,29 @@ test_branch_created() {
 	printf "From: ${GOTD_USER}@${HOSTNAME}\r\n" >> $testroot/stdout.expected
 	printf "To: ${GOTD_DEVUSER}\r\n" >> $testroot/stdout.expected
 	printf "Subject: $GOTD_TEST_REPO_NAME: " >> $testroot/stdout.expected
-	printf "${GOTD_DEVUSER} created refs/heads/newbranch: $short_commit_id\r\n" \
+	printf "${GOTD_DEVUSER} created refs/heads/newbranch: $short_commit_id2\r\n" \
 		>> $testroot/stdout.expected
 	printf "\r\n" >> $testroot/stdout.expected
 	printf "commit $commit_id\n" >> $testroot/stdout.expected
 	printf "from: $GOT_AUTHOR\n" >> $testroot/stdout.expected
-	d=`date -u -r $author_time +"%a %b %e %X %Y UTC"`
+	d=`date -u -r $author_time2 +"%a %b %e %X %Y UTC"`
 	printf "date: $d\n" >> $testroot/stdout.expected
-	printf "messagelen: 11\n" >> $testroot/stdout.expected
+	printf "messagelen: 27\n" >> $testroot/stdout.expected
 	printf " \n" >> $testroot/stdout.expected
-	printf " newbranch\n \n" >> $testroot/stdout.expected
+	printf " change alpha on newbranch\n \n" >> $testroot/stdout.expected
 	printf " M  alpha  |  1+  1-\n\n"  >> $testroot/stdout.expected
 	printf "1 file changed, 1 insertion(+), 1 deletion(-)\n\n" \
 		>> $testroot/stdout.expected
+	printf "commit $commit_id2\n" >> $testroot/stdout.expected
+	printf "from: $GOT_AUTHOR\n" >> $testroot/stdout.expected
+	d=`date -u -r $author_time2 +"%a %b %e %X %Y UTC"`
+	printf "date: $d\n" >> $testroot/stdout.expected
+	printf "messagelen: 20\n" >> $testroot/stdout.expected
+	printf " \n" >> $testroot/stdout.expected
+	printf " change alpha again\n \n" >> $testroot/stdout.expected
+	printf " M  alpha  |  1+  1-\n\n"  >> $testroot/stdout.expected
+	printf "1 file changed, 1 insertion(+), 1 deletion(-)\n\n" \
+		>> $testroot/stdout.expected
 	printf "\r\n" >> $testroot/stdout.expected
 	printf ".\r\n" >> $testroot/stdout.expected
 	printf "QUIT\r\n" >> $testroot/stdout.expected
@@ -344,6 +360,104 @@ test_branch_created() {
 	test_done "$testroot" "$ret"
 }
 
+test_branch_recreated() {
+	local testroot=`test_init branch_recreated 1`
+
+	got clone -a -q ${GOTD_TEST_REPO_URL} $testroot/repo-clone
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got clone failed unexpectedly" >&2
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	got branch -r $testroot/repo-clone -d newbranch > /dev/null
+
+	got checkout -q $testroot/repo-clone $testroot/wt >/dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got checkout failed unexpectedly" >&2
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	(cd $testroot/wt && got branch newbranch > /dev/null)
+
+	echo "change beta on branch" > $testroot/wt/beta
+	(cd $testroot/wt && got commit -m 'change beta on newbranch' \
+		> /dev/null)
+	local commit_id=`git_show_branch_head $testroot/repo-clone newbranch`
+	local author_time=`git_show_author_time $testroot/repo-clone $commit_id`
+
+	echo "change beta again" > $testroot/wt/beta
+	(cd $testroot/wt && got commit -m 'change beta again' > /dev/null)
+	local commit_id2=`git_show_branch_head $testroot/repo-clone newbranch`
+	local author_time2=`git_show_author_time $testroot/repo-clone $commit_id`
+	(printf "220\r\n250\r\n250\r\n250\r\n354\r\n250\r\n221\r\n" \
+		| timeout 5 nc -l "$GOTD_TEST_SMTP_PORT" > $testroot/stdout) &
+
+	sleep 1 # server starts up
+
+	got send -b newbranch -f -q -r $testroot/repo-clone
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got send failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	wait %1 # wait for nc -l
+
+	short_commit_id2=`trim_obj_id 12 $commit_id2`
+	short_commit_id=`trim_obj_id 12 $commit_id`
+	HOSTNAME=`hostname`
+	printf "HELO localhost\r\n" > $testroot/stdout.expected
+	printf "MAIL FROM:<${GOTD_USER}@${HOSTNAME}>\r\n" \
+		>> $testroot/stdout.expected
+	printf "RCPT TO:<${GOTD_DEVUSER}>\r\n" >> $testroot/stdout.expected
+	printf "DATA\r\n" >> $testroot/stdout.expected
+	printf "From: ${GOTD_USER}@${HOSTNAME}\r\n" >> $testroot/stdout.expected
+	printf "To: ${GOTD_DEVUSER}\r\n" >> $testroot/stdout.expected
+	printf "Subject: $GOTD_TEST_REPO_NAME: " >> $testroot/stdout.expected
+	printf "${GOTD_DEVUSER} changed refs/heads/newbranch: $short_commit_id2\r\n" \
+		>> $testroot/stdout.expected
+	printf "\r\n" >> $testroot/stdout.expected
+	printf "commit $commit_id\n" >> $testroot/stdout.expected
+	printf "from: $GOT_AUTHOR\n" >> $testroot/stdout.expected
+	d=`date -u -r $author_time2 +"%a %b %e %X %Y UTC"`
+	printf "date: $d\n" >> $testroot/stdout.expected
+	printf "messagelen: 26\n" >> $testroot/stdout.expected
+	printf " \n" >> $testroot/stdout.expected
+	printf " change beta on newbranch\n \n" >> $testroot/stdout.expected
+	printf " M  beta  |  1+  1-\n\n"  >> $testroot/stdout.expected
+	printf "1 file changed, 1 insertion(+), 1 deletion(-)\n\n" \
+		>> $testroot/stdout.expected
+	printf "commit $commit_id2\n" >> $testroot/stdout.expected
+	printf "from: $GOT_AUTHOR\n" >> $testroot/stdout.expected
+	d=`date -u -r $author_time2 +"%a %b %e %X %Y UTC"`
+	printf "date: $d\n" >> $testroot/stdout.expected
+	printf "messagelen: 19\n" >> $testroot/stdout.expected
+	printf " \n" >> $testroot/stdout.expected
+	printf " change beta again\n \n" >> $testroot/stdout.expected
+	printf " M  beta  |  1+  1-\n\n"  >> $testroot/stdout.expected
+	printf "1 file changed, 1 insertion(+), 1 deletion(-)\n\n" \
+		>> $testroot/stdout.expected
+	printf "\r\n" >> $testroot/stdout.expected
+	printf ".\r\n" >> $testroot/stdout.expected
+	printf "QUIT\r\n" >> $testroot/stdout.expected
+
+	grep -v ^Date $testroot/stdout > $testroot/stdout.filtered
+	cmp -s $testroot/stdout.expected $testroot/stdout.filtered
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout.filtered
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	test_done "$testroot" "$ret"
+}
+
 test_branch_removed() {
 	local testroot=`test_init branch_removed 1`
 
@@ -744,6 +858,7 @@ run_test test_file_changed
 run_test test_many_commits_not_summarized
 run_test test_many_commits_summarized
 run_test test_branch_created
+run_test test_branch_recreated
 run_test test_branch_removed
 run_test test_tag_created
 run_test test_tag_changed