On Mon, Jan 29, 2024 at 03:07:43PM +0100, Stefan Sperling wrote:
>On Sun, Jan 28, 2024 at 06:34:17PM +0000, James Cook wrote:
>> I've managed to trigger the error
>>
>> 	got: //d/g: no such entry found in tree
>>
>> by running got log -b d/g or tog log -b d/g in a specially crafted repo.
>
>Thanks! This is an edge case I had overlooked.
>Below is a fix which I tested manually. Does it work for you?
>
>The patch repeats an if-condition in a slightly awkward way because the
>diff becomes easier to read, avoiding indentation of a lot of lines by
>one more level. Essentially we need to guard the STAILQ_FOREACH loop
>with if (merged_id != NULL).
>
>> I have a script (at bottom) to reproduce and am happy to turn that into a
>> patch for regress/cmdline.
>
>Yes, please do turn it into a regression test if you have time.

Thanks, your patch fixes the problem for me, including in the original 
private repo where I discovered it.

Comparing the output of "got log -b path" with "git log path", I 
notice got likes to show some merge commits that git skips over, 
but that doesn't seem like a big problem.

Below is a patch adding a test case to log.sh based on my repro 
script. I named the test ..._corner_case because it's pretty specific. 
I followed the pattern I see of using git commands instead of got 
commands to set up the repo.

-- 
James


diff fc9b745fd3b1a01f7e89f269600db36ad5222b3e 88456a98463ce990d4f4df3c98274b43fd148eda
commit - fc9b745fd3b1a01f7e89f269600db36ad5222b3e
commit + 88456a98463ce990d4f4df3c98274b43fd148eda
blob - 4721d314c761fef6ff7f1dfe3ff55d52e6d39216
blob + 2382dfe2d37e40510f2213eafd7c5177407f3df6
--- regress/cmdline/log.sh
+++ regress/cmdline/log.sh
@@ -769,6 +769,60 @@ test_log_changed_paths() {
  	test_done "$testroot" "$ret"
  }
  
+test_log_merge_commit_corner_case() {
+	local testroot=`test_init log_merge_commit_corner_case 1`
+
+        # Create the following commit graph (most recent commit shown first):
+        #
+        #   o  create dir/beta
+        #   |
+        #   o  merge (does not touch dir)
+        #  / \
+        # o   o  changes which don't touch the directory "dir"
+        #  \ /
+        #   o  initial commit, which includes directory "dir" but not dir/beta
+
+
+	mkdir $testroot/repo/dir
+	touch $testroot/repo/dir/alpha
+	git -C $testroot/repo add dir/alpha
+	git_commit $testroot/repo -m "initial commit"
+
+	git -C $testroot/repo checkout -q -b aux
+	touch $testroot/repo/gamma
+	git -C $testroot/repo add gamma
+	git_commit $testroot/repo -m "change on aux"
+
+	git -C $testroot/repo checkout -q master
+	touch $testroot/repo/delta
+	git -C $testroot/repo add delta
+	git_commit $testroot/repo -m "change on master"
+
+	git -C $testroot/repo merge -q -m "merge" aux
+
+	touch $testroot/repo/dir/beta
+	git -C $testroot/repo add dir/beta
+	git_commit $testroot/repo -m "add beta"
+
+	head_commit=`git_show_head $testroot/repo`
+
+	got log -r $testroot/repo -b dir/beta | grep ^commit > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "log command failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	echo "commit $head_commit (master)" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" "$ret"
+}
+
  test_log_submodule() {
  	local testroot=`test_init log_submodule`
  
@@ -1201,6 +1255,7 @@ run_test test_log_end_at_commit
  run_test test_log_reverse_display
  run_test test_log_in_worktree_different_repo
  run_test test_log_changed_paths
+run_test test_log_merge_commit_corner_case
  run_test test_log_submodule
  run_test test_log_diffstat
  run_test test_log_commit_keywords