From: Tian Yuchen <cat@malon.dev>
Date: Sat, 11 Apr 2026 18:49:05 GMT
Subject: Re: [GSoC PATCH v5 5/6] t7703: test for promisor file content after geometric repack
Message-ID: <a20d7433-2297-4ad9-a68e-302443f1f941@malon.dev>
In-Reply-To: <ab307e68feb9f84898dba748ef1f4598b07fc0e2.1775861047.git.lorenzo.pegorari2002@gmail.com>

On 4/11/26 06:56, LorenzoPegorari wrote:
> Add test that checks if the content of ".promisor" files are correctly
> copied inside the ".promisor" files created by a geometric repack.
> 
> Signed-off-by: LorenzoPegorari <lorenzo.pegorari2002@gmail.com>
> ---
>   t/t7703-repack-geometric.sh | 33 +++++++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
> 
> diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh
> index 04d5d8fc33..a8e3e6ae3f 100755
> --- a/t/t7703-repack-geometric.sh
> +++ b/t/t7703-repack-geometric.sh
> @@ -541,4 +541,37 @@ test_expect_success 'geometric repack works with promisor packs' '
>   	)
>   '
>   
> +test_expect_success 'check .promisor file content after geometric repack' '
> +	test_when_finished rm -rf prom_test &&
> +	git init prom_test &&
> +	path=prom_test/.git/objects/pack &&
> +
> +	(
> +		# Create 2 packs with 3 objs each, and manually create .promisor files
> +		test_commit_bulk -C prom_test --start=1 1 &&  # 3 objects

---

> +		prom1=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&

This approach seems a bit fragile.

- Perhaps you’ve heard the saying which goes like "never parse the 
output of ls". In a nutshell, the output of this command is not 
standardised;

- *.pack? This may produce multiple lines of output, which I don’t think 
is what we want.

- $path instead of "$path", which cannot correctly handle spacing in 
directory names;

- sed s command matches the "first" string it meets. We can’t guarantee 
that the '.pack' part won’t appear in users’ path names, can we?

(Fun fact: There are approximately 8,000 people in the United States 
with the surname 'Pack'. Source: 2010 Census ; - )


> +		oid1=$(git -C prom_test rev-parse HEAD) &&
> +		echo "$oid1 ref1" >"$prom1" &&
> +		test_commit_bulk -C prom_test --start=2 1 &&  # 3 objects
> +		prom2=$(ls $path/*.pack | sed "s/\.pack/.promisor/; \|$prom1|d") &&
> +		oid2=$(git -C prom_test rev-parse HEAD) &&
> +		echo "$oid2 ref2" >"$prom2" &&
> +
> +		# Create 1 pack with 12 objs, and manually create .promisor file
> +		test_commit_bulk -C prom_test --start=3 4 &&  # 12 objects
> +		prom3=$(ls $path/*.pack | sed "s/\.pack/.promisor/; \|$prom1|d; \|$prom2|d") &&
> +		oid3=$(git -C prom_test rev-parse HEAD) &&
> +		echo "$oid3 ref3" >"$prom3" &&
> +
> +		# Geometric repack, and check if correct
> +		git -C prom_test repack --geometric 2 -d &&
> +		prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/; \|$prom3|d") &&
> +		# $prom should have repacked only the first 2 small packs, so it should only
> +		# contain the following: "$oid1 ref1 <time>" & "$oid2 ref2 <time>"
> +		test_grep "$oid1 ref1 " "$prom" &&
> +		test_grep "$oid2 ref2 " "$prom" &&
> +		test_grep ! "$oid3 ref3" "$prom"
> +	)
> +'
> +
>   test_done

Thanks, yuchen