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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: tog: regress to cover work tree entries + simple fix
To:
Mark Jamsek <mark@jamsek.com>
Cc:
gameoftrees@openbsd.org
Date:
Thu, 19 Dec 2024 02:35:52 +0100

Download raw body.

Thread
On Wed, Dec 11, 2024 at 05:06:45PM +1100, Mark Jamsek wrote:
> Below are two diffs: the first is a simple fix for an ignored
> pthread_cond_signal(3) error and a missed pthread_cond_destroy(3) call
> when tog is run by the test harness; the second adds a test to tog's log
> regress to cover the new work tree diff feature.
> 
> The second needs the pending diffs in the recent thread started by Lucas
> (Make missing worktree author non-fatal in tog) to be applied first.

ok  (meanwhile, the pending diffs this depends on have been committed)
 
> -----------------------------------------------
> commit 6fbe6f479f13c3059ef7fc8b645757a75adddb8e
> from: Mark Jamsek <mark@jamsek.dev>
> date: Wed Dec 11 04:58:00 2024 UTC
>  
>  tog: fix err deadstore and destroy regress pthread condition
>  
>  When tog is run by the test harness, free the log_loaded pthread
>  condition variable. And don't ignore a log_loaded pthread_cond_signal(3)
>  error.
>  
>  M  tog/tog.c  |  12+  1-
> 
> 1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff 974d5f1186af30a7c2c769ed8b33bf19a1eaf901 6fbe6f479f13c3059ef7fc8b645757a75adddb8e
> commit - 974d5f1186af30a7c2c769ed8b33bf19a1eaf901
> commit + 6fbe6f479f13c3059ef7fc8b645757a75adddb8e
> blob - a09591e35e09e13fb23f7c13166a95b75ab02ffc
> blob + fb50d0d367f8c852aed59a901ac3a658197a647e
> --- tog/tog.c
> +++ tog/tog.c
> @@ -4061,10 +4061,14 @@ log_thread(void *arg)
>  				if (tog_io.wait_for_ui) {
>  					errcode = pthread_cond_signal(
>  					    &a->log_loaded);
> -					if (errcode && err == NULL)
> +					if (errcode) {
>  						err = got_error_set_errno(
>  						    errcode,
>  						    "pthread_cond_signal");
> +						pthread_mutex_unlock(
> +						    &tog_mutex);
> +						goto done;
> +					}
>  				}
>  
>  				errcode = pthread_cond_wait(&a->need_commits,
> @@ -4186,6 +4190,13 @@ close_log_view(struct tog_view *view)
>  	if (errcode && err == NULL)
>  		err = got_error_set_errno(errcode, "pthread_cond_destroy");
>  
> +	if (using_mock_io) {
> +		errcode = pthread_cond_destroy(&s->thread_args.log_loaded);
> +		if (errcode && err == NULL)
> +			err = got_error_set_errno(errcode,
> +			    "pthread_cond_destroy");
> +	}
> +
>  	free_commits(&s->limit_commits);
>  	free_commits(&s->real_commits);
>  	free_colors(&s->colors);
> 
> -----------------------------------------------
> commit 45b12d17fa516fdd411ecdea1d13fa79340a223a (main)
> from: Mark Jamsek <mark@jamsek.dev>
> date: Wed Dec 11 05:42:42 2024 UTC
>  
>  tog: expand log regress to cover work tree entries
>  
>  M  regress/tog/log.sh  |  131+  0-
> 
> 1 file changed, 131 insertions(+), 0 deletions(-)
> 
> diff 6fbe6f479f13c3059ef7fc8b645757a75adddb8e 45b12d17fa516fdd411ecdea1d13fa79340a223a
> commit - 6fbe6f479f13c3059ef7fc8b645757a75adddb8e
> commit + 45b12d17fa516fdd411ecdea1d13fa79340a223a
> blob - dc5a64d8bdc14067f6bc59ebcbdbeedaf610d748
> blob + 5afc587650afd7ac221d87769ed311be1bdde14f
> --- regress/tog/log.sh
> +++ regress/tog/log.sh
> @@ -902,6 +902,136 @@ test_log_mark_keymap()
>  	test_done "$testroot" $ret
>  }
>  
> +test_log_worktree_entries()
> +{
> +	test_init log_worktree_entries 96 4
> +	local repo="$testroot/repo"
> +
> +	got checkout "$repo" "$testroot/wt" > /dev/null
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		echo "got checkout failed unexpectedly"
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	# test dirty work tree (no staged changes)
> +	cd "$testroot/wt"
> +	echo "'alpha" >> alpha
> +
> +	local id=$(git_show_head "$repo")
> +	local id10=$(trim_obj_id 10 $id)
> +	local author_time=$(git_show_author_time "$repo")
> +	local ymd=$(date -u -r "$author_time" +"%F")
> +
> +	cat <<-EOF >$TOG_TEST_SCRIPT
> +	WAIT_FOR_UI	wait for log thread to fetch wt state
> +	WAIT_FOR_UI	trigger redraw to reveal work tree entries
> +	SCREENDUMP
> +	EOF
> +
> +	cat <<-EOF >$testroot/view.expected
> +	diff $testroot/wt (work tree changes) [0/1] master
> +	$ymd flan_hacker  work tree changes based on [$id10]
> +	$ymd flan_hacker *[master] adding the test tree
> +
> +	EOF
> +
> +	tog log
> +	cmp -s "$testroot/view.expected" "$testroot/view"
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		diff -u "$testroot/view.expected" "$testroot/view"
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	# test dirty work tree with staged changes too
> +	echo "'beta" >> beta
> +	got stage beta > /dev/null
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		echo "got stage failed unexpectedly"
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	cat <<-EOF >$TOG_TEST_SCRIPT
> +	WAIT_FOR_UI	wait for log thread to fetch wt state
> +	j		select staged entry
> +	SCREENDUMP
> +	EOF
> +
> +	cat <<-EOF >$testroot/view.expected
> +	diff -s $testroot/wt (staged work tree changes) [0/1] master
> +	$ymd flan_hacker  work tree changes based on [$id10]
> +	$ymd flan_hacker  staged work tree changes based on [$id10]
> +	$ymd flan_hacker *[master] adding the test tree
> +	EOF
> +
> +	tog log
> +	cmp -s "$testroot/view.expected" "$testroot/view"
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		diff -u "$testroot/view.expected" "$testroot/view"
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	# test with only staged changes
> +	got revert alpha > /dev/null
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		echo "got revert failed unexpectedly"
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	cat <<-EOF >$TOG_TEST_SCRIPT
> +	WAIT_FOR_UI	wait for log thread to fetch wt state
> +	WAIT_FOR_UI	trigger redraw to reveal work tree entries
> +	SCREENDUMP
> +	EOF
> +
> +	cat <<-EOF >$testroot/view.expected
> +	diff -s $testroot/wt (staged work tree changes) [0/1] master
> +	$ymd flan_hacker  staged work tree changes based on [$id10]
> +	$ymd flan_hacker *[master] adding the test tree
> +
> +	EOF
> +
> +	tog log
> +	cmp -s "$testroot/view.expected" "$testroot/view"
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		diff -u "$testroot/view.expected" "$testroot/view"
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	# test with no author set
> +	echo "'alpha" >> alpha
> +
> +	cat <<-EOF >$testroot/view.expected
> +	diff $testroot/wt (work tree changes) [0/1] master
> +	$ymd              work tree changes based on [$id10]
> +	$ymd              staged work tree changes based on [$id10]
> +	$ymd flan_hacker *[master] adding the test tree
> +	EOF
> +
> +	unset GOT_AUTHOR
> +	tog log
> +	cmp -s "$testroot/view.expected" "$testroot/view"
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		diff -u "$testroot/view.expected" "$testroot/view"
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	test_done "$testroot" 0
> +}
> +
>  test_parseargs "$@"
>  run_test test_log_hsplit_diff
>  run_test test_log_vsplit_diff
> @@ -915,3 +1045,4 @@ run_test test_log_show_base_commit
>  run_test test_log_limit_view
>  run_test test_log_search
>  run_test test_log_mark_keymap
> +run_test test_log_worktree_entries
> 
> 
> 
> 
> -- 
> Mark Jamsek <https://bsdbox.org>
> GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68
> 
>