Re: [PATCH v3 04/11] bundle-uri client: add boolean transfer.bundleURI setting

[Victoria Dye <vdye@github.com>]

Ævar Arnfjörð Bjarmason via GitGitGadget wrote:
> From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=
>  <avarab@gmail.com>
> 
> The yet-to-be introduced client support for bundle-uri will always
> fall back on a full clone, but we'd still like to be able to ignore a
> server's bundle-uri advertisement entirely.
> 
> The new transfer.bundleURI config option defaults to 'false', but a user
> can set it to 'true' to enable checking for bundle URIs from the origin
> Git server using protocol v2.
> 
> To enable this setting by default in the correct tests, add a
> GIT_TEST_BUNDLE_URI environment variable.

It wasn't immediately clear to me from reading this patch, but it looks like
'GIT_TEST_BUNDLE_URI' is mainly used to allow 'test-tool bundle-uri
ls-remote' to issue the bundle URI command (since it can't use a '-c
transfer.bundleURI=true' command line option) in patch 7 [1].

If that's the only use for 'GIT_TEST_BUNDLE_URI', could you avoid the
environment variable altogether by setting 'transfer.bundleURI=true' with
'test_config' before the 'test-tool' call (and 'test_unconfig' after, if
needed)? Alternatively, if you do want to be able to test the bundle URI
protocol wholesale across all tests (e.g., in the 'linux-TEST-vars' CI job),
then I think the environment variable makes sense.

[1] https://lore.kernel.org/git/acc5a8f57f903342c47802115f8e3de9e9d588dc.1670262639.git.gitgitgadget@gmail.com/

> diff --git a/t/lib-bundle-uri-protocol.sh b/t/lib-bundle-uri-protocol.sh
> index d44c6e10f9e..77bfd4f0119 100644
> --- a/t/lib-bundle-uri-protocol.sh
> +++ b/t/lib-bundle-uri-protocol.sh
> @@ -85,9 +85,10 @@ test_expect_success "connect with $BUNDLE_URI_PROTOCOL:// using protocol v2: hav
>  '
>  
>  test_expect_success "clone with $BUNDLE_URI_PROTOCOL:// using protocol v2: request bundle-uris" '
> -	test_when_finished "rm -rf log cloned" &&
> +	test_when_finished "rm -rf log cloned cloned2" &&
>  
>  	GIT_TRACE_PACKET="$PWD/log" \
> +	GIT_TEST_BUNDLE_URI=0 \
>  	git \
>  		-c protocol.version=2 \
>  		clone "$BUNDLE_URI_REPO_URI" cloned \
> @@ -99,6 +100,22 @@ test_expect_success "clone with $BUNDLE_URI_PROTOCOL:// using protocol v2: reque
>  	# Server advertised bundle-uri capability
>  	grep "< bundle-uri" log &&
>  
> +	# Client did not issue bundle-uri command
> +	! grep "> command=bundle-uri" log &&
> +
> +	GIT_TRACE_PACKET="$PWD/log" \
> +	git \
> +		-c transfer.bundleURI=true \
> +		-c protocol.version=2 \
> +		clone "$BUNDLE_URI_REPO_URI" cloned2 \
> +		>actual 2>err &&

If 'GIT_TEST_BUNDLE_URI' is set to '1' in a more global scope (by a CI job
or user running the tests), then the '-c transfer.bundleURI' config isn't
actually what's enabling the behavior. To make this more directly comparable
to the case earlier in this test, could you add 'GIT_TEST_BUNDLE_URI=0' here
as well?