From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 19 Feb 2026 15:35:52 GMT
Subject: Re: [PATCH v7 6/6] refs: add GIT_REFERENCE_BACKEND to specify reference backend
Message-ID: <aZct2M3sbQSduK2q@pks.im>
In-Reply-To: <20260219-kn-alternate-ref-dir-v7-6-16f27860dbdf@gmail.com>

On Thu, Feb 19, 2026 at 10:38:25AM +0100, Karthik Nayak wrote:
> diff --git a/t/t1423-ref-backend.sh b/t/t1423-ref-backend.sh
> index 9912433b8c..d69aea3f7f 100755
> --- a/t/t1423-ref-backend.sh
> +++ b/t/t1423-ref-backend.sh
> @@ -30,44 +39,86 @@ run_with_uri() {
>  #   <repo> is the relative path to the repo to run the command in.
>  #   <backend> is the original ref storage of the repo.
>  #   <uri> is the new URI to be set for the ref storage.
> +#   <via> if 'config', set the backend via the 'extensions.refStorage' config.
> +#         if 'env', set the backend via the 'GIT_REFERENCE_BACKEND' env.
>  #   <err_msg> (optional) if set, check if 'git-refs(1)' failed with the provided msg.
>  test_refs_backend() {
>  	repo=$1 &&
>  	backend=$2 &&
>  	uri=$3 &&
> -	err_msg=$4 &&
> +	via=$4 &&
> +	err_msg=$5 &&
> +
>  
> -	git -C "$repo" config set core.repositoryformatversion 1 &&
>  	if test -n "$err_msg";
>  	then
> -		git -C "$repo" config set extensions.refStorage "$uri" &&
> -		test_must_fail git -C "$repo" refs list 2>err &&
> -		test_grep "$err_msg" err
> +		if test "$via" = "env"
> +		then
> +			test_env GIT_REFERENCE_BACKEND="$uri" test_must_fail git -C "$repo" refs list 2>err
> +		elif test "$via" = "config"
> +		then
> +			git -C "$repo" config set extensions.refStorage "$uri" &&
> +			test_must_fail git -C "$repo" refs list 2>err &&
> +			test_grep "$err_msg" err
> +		fi
>  	else
>  		git -C "$repo" refs list >expect &&
> -		run_with_uri "$repo" "$backend" "$uri" "refs list" >actual &&
> +		run_with_uri "$repo" "$backend" "$uri" "refs list" "$via">actual &&
>  		test_cmp expect actual
>  	fi
>  }
>  
> -test_expect_success 'URI is invalid' '
> +# Verify that the expected files are present in the gitdir and the refsdir.
> +# Usage: verify_files_exist <gitdir> <refdir>
> +#   <gitdir> is the path for the gitdir.
> +#   <refdir> is the path for the refdir.
> +verify_files_exist() {
> +	gitdir=$1 &&
> +	refdir=$2 &&
> +
> +	# verify that the stubs were added to the $GITDIR.
> +	cat $gitdir/refs/heads >actual &&
> +	echo "repository uses alternate refs storage" >expect &&
> +	test_cmp expect actual &&

Tiny nit, not worth addressing on its own: we could simply `test_cmp
expect "$gitdir/refs/heads", without the need to copy that file first.

> +	cat $gitdir/HEAD >actual &&
> +	echo "ref: refs/heads/.invalid" >expect &&
> +	test_cmp expect actual

Same here, no need to copy the file around.

Other than that I'm happy with this patch series now, thanks!

Patrick