commit 5b9c302980f3cf3d8df39dad1c4fb33ec357b731
from: Mark Jamsek <mark@jamsek.dev>
via: Thomas Adam <thomas@xteddy.org>
date: Sun Feb 16 13:44:27 2025 UTC

regress/cmdline: diffstat test for long, nested paths

Although ostensibly a test to ensure long paths are properly aligned,
the bug prompting this was due to computing the max path length of
an already checked path due to an incorrect RB_MAX() call (fix in the
next commit): get_diffstat() may not receive paths in lexicographic
order, in which case the bad RB_MAX() call returns a path that has
already been length checked, which is not the most recently added path.
So we would sometimes end up with a max_path_len that was not actually
the longest path, which manifested in misaligned path separators in the
diffstat and was reported by stsp on IRC.

ok stsp@

commit - 984dc60f4f97b89e1e9fe13698f2e234bffeef31
commit + 5b9c302980f3cf3d8df39dad1c4fb33ec357b731
blob - 9a199b109499e4e13271e33d51016fdef2dda3b7
blob + bf8d3b70fb368cfdf5eb3373e5833c14371b84f5
--- regress/cmdline/common.sh
+++ regress/cmdline/common.sh
@@ -235,9 +235,10 @@ get_blob_id()
 	repo="$1"
 	tree_path="$2"
 	filename="$3"
+	commit="${4:+-c $4}"
 
-	got tree -r $repo -i $tree_path | grep "[0-9a-f] ${filename}$" | \
-		cut -d' ' -f 1
+	got tree -r $repo $commit -i $tree_path | \
+		grep "[0-9a-f] ${filename}$" | cut -d' ' -f 1
 }
 
 test_init()
blob - 1c2391cef1211e8f745596a22c4944873f497779
blob + 9451899ec3ccb72c13c0347b302d8c1726fb7079
--- regress/cmdline/diff.sh
+++ regress/cmdline/diff.sh
@@ -1812,6 +1812,135 @@ EOF
 	test_done "$testroot" "$ret"
 }
 
+test_diff_long_path_diffstat() {
+	local testroot=$(test_init diff_long_path_diffstat)
+	local id_root=$(git_show_head "$testroot/repo")
+	local blobid_beta=$(get_blob_id "$testroot/repo" "" beta)
+	local blobid_alpha=$(get_blob_id "$testroot/repo" "" alpha)
+	local blobid_zeta=$(get_blob_id "$testroot/repo" "epsilon" zeta)
+	local blobid_delta=$(get_blob_id "$testroot/repo" "gamma" delta)
+	local wspadding="                                "
+	local longpathdir=c123456789-0123456789/d123456789
+	local longerpathdir=$longpathdir/$longpathdir/$longpathdir
+	local longerpath=$longerpathdir/longerpath
+	local longpath=$longpathdir/longpath
+
+	got checkout "$testroot/repo" "$testroot/wt" > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got checkout failed unexpectedly" >&2
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	(cd "$testroot/wt" && got branch longpath >/dev/null)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got branch failed unexpectedly" >&2
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	mkdir -p "$testroot/wt/$longerpathdir"
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "failed to make directory $testroot/wt/$longerpathdir" >&2
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	echo "xbeta" > $testroot/wt/beta
+	echo "xalpha" > $testroot/wt/alpha
+	echo "xdelta" > $testroot/wt/gamma/delta
+	echo "xzeta" > $testroot/wt/epsilon/zeta
+	echo "longpath" > $testroot/wt/$longpath
+	echo "longerpath" > $testroot/wt/$longerpath
+
+	(cd "$testroot/wt" && got add "$longpath" "$longerpath" >/dev/null)
+	if [ $ret -ne 0 ]; then
+		echo "got add failed unexpectedly" >&2
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	(cd "$testroot/wt" && got commit -m "longpath" >/dev/null)
+	if [ $ret -ne 0 ]; then
+		echo "got commit failed unexpectedly" >&2
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	local blobid_longpath=$(get_blob_id "$testroot/repo" \
+	    "$longpathdir" longpath longpath)
+	local blobid_longerpath=$(get_blob_id "$testroot/repo" \
+	    "$longerpathdir" longerpath longpath)
+
+	cat <<-EOF >$testroot/stdout.expected
+	diffstat refs/heads/master refs/heads/longpath
+	 M  alpha      $wspadding $wspadding $wspadding  |  1+  1-
+	 M  beta       $wspadding $wspadding $wspadding  |  1+  1-
+	 A  $longerpath  |  1+  0-
+	 A  $longpath  $wspadding $wspadding   |  1+  0-
+	 M  epsilon/zeta $wspadding$wspadding$wspadding  |  1+  1-
+	 M  gamma/delta  $wspadding$wspadding$wspadding  |  1+  1-
+
+	6 files changed, 6 insertions(+), 4 deletions(-)
+
+	diff refs/heads/master refs/heads/longpath
+	commit - $id_root
+	commit + $(cd "$testroot/wt" && got br -l | fgrep '*' | cut -d' ' -f3)
+	blob - $blobid_alpha
+	blob + $(get_blob_id "$testroot/repo" "" alpha longpath)
+	--- alpha
+	+++ alpha
+	@@ -1 +1 @@
+	-alpha
+	+xalpha
+	blob - $blobid_beta
+	blob + $(get_blob_id "$testroot/repo" "" beta longpath)
+	--- beta
+	+++ beta
+	@@ -1 +1 @@
+	-beta
+	+xbeta
+	blob - $blobid_zeta
+	blob + $(get_blob_id "$testroot/repo" "epsilon" zeta longpath)
+	--- epsilon/zeta
+	+++ epsilon/zeta
+	@@ -1 +1 @@
+	-zeta
+	+xzeta
+	blob - /dev/null
+	blob + $blobid_longerpath (mode 644)
+	--- /dev/null
+	+++ $longerpath
+	@@ -0,0 +1 @@
+	+longerpath
+	blob - /dev/null
+	blob + $blobid_longpath (mode 644)
+	--- /dev/null
+	+++ $longpath
+	@@ -0,0 +1 @@
+	+longpath
+	blob - $blobid_delta
+	blob + $(get_blob_id "$testroot/repo" "gamma" delta longpath)
+	--- gamma/delta
+	+++ gamma/delta
+	@@ -1 +1 @@
+	-delta
+	+xdelta
+	EOF
+
+	(cd "$testroot/wt" && got diff -d master longpath > "$testroot/stdout")
+
+	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_diff_file_to_dir() {
 	local testroot=`test_init diff_file_to_dir`
 	local commit_id0=`git_show_head $testroot/repo`
@@ -2404,6 +2533,7 @@ run_test test_diff_crlf
 run_test test_diff_worktree_newfile_xbit
 run_test test_diff_commit_diffstat
 run_test test_diff_worktree_diffstat
+run_test test_diff_long_path_diffstat
 run_test test_diff_file_to_dir
 run_test test_diff_dir_to_file
 run_test test_diff_path_in_root_commit