From: Stefan Sperling <stsp@stsp.name>
Subject: Re: limit commit notification history traversal
To: gameoftrees@openbsd.org
Date: Sat, 6 Sep 2025 09:20:01 +0200

On Sun, Aug 17, 2025 at 08:20:34PM +0200, Stefan Sperling wrote:
> 
> 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?

Ping.

> 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
> 
>