Re: [PATCH 01/10] promisor-remote: try accepted remotes before others in get_direct()

[Patrick Steinhardt <ps@pks.im>]

On Thu, Apr 02, 2026 at 09:06:04AM +0200, Christian Couder wrote:
> diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc
> index f985cb4c47..4fcb1a7bda 100644
> --- a/Documentation/gitprotocol-v2.adoc
> +++ b/Documentation/gitprotocol-v2.adoc
> @@ -848,6 +848,10 @@ advertised, it can reply with "promisor-remote=<pr-names>" where
>  where `pr-name` is the urlencoded name of a promisor remote the server
>  advertised and the client accepts.
>  
> +The promisor remotes that the client accepted will be tried before the
> +other configured promisor remotes when the client attempts to fetch
> +missing objects.
> +
>  Note that, everywhere in this document, the ';' and ',' characters
>  MUST be encoded if they appear in `pr-name` or `field-value`.

Makes sense, thanks for adding this blurb.

> diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
> index 357822c01a..bf0eed9f10 100755
> --- a/t/t5710-promisor-remote-capability.sh
> +++ b/t/t5710-promisor-remote-capability.sh
> @@ -166,6 +166,75 @@ test_expect_success "init + fetch with promisor.advertise set to 'true'" '
>  	check_missing_objects server 1 "$oid"
>  '
>  
> +test_expect_success "clone with two promisors but only one advertised" '
> +	git -C server config promisor.advertise true &&
> +	test_when_finished "rm -rf client unused_lop" &&
> +
> +	# Create a promisor that will be configured but not be used
> +	git init --bare unused_lop &&
> +
> +	# Clone from server to create a client
> +	GIT_TRACE="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \
> +		-c remote.unused_lop.promisor=true \
> +		-c remote.unused_lop.fetch="+refs/heads/*:refs/remotes/unused_lop/*" \
> +		-c remote.unused_lop.url="file://$(pwd)/unused_lop" \
> +		-c remote.lop.promisor=true \
> +		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
> +		-c remote.lop.url="file://$(pwd)/lop" \
> +		-c promisor.acceptfromserver=All \
> +		--no-local --filter="blob:limit=5k" server client &&
> +
> +	# Check that "unused_lop" appears before "lop" in the config
> +	printf "remote.%s.promisor true\n" "unused_lop" "lop" "origin" >expect &&
> +	git -C client config get --all --show-names --regexp "^remote\..*\.promisor$" >actual &&
> +	test_cmp expect actual &&
> +
> +	# Check that "lop" was tried
> +	test_grep " fetch lop " trace &&
> +	# Check that "unused_lop" was not contacted
> +	# This means "lop", the accepted promisor, was tried first
> +	test_grep ! " fetch unused_lop " trace &&
> +
> +	# Check that the largest object is still missing on the server
> +	check_missing_objects server 1 "$oid"
> +'
> +
> +test_expect_success "init + fetch two promisors but only one advertised" '
> +	git -C server config promisor.advertise true &&
> +	test_when_finished "rm -rf client unused_lop" &&
> +
> +	# Create a promisor that will be configured but not be used
> +	git init --bare unused_lop &&
> +
> +	mkdir client &&
> +	git -C client init &&

Tiniest nit, not worth rerolling over: this could just be `git init client`.

Patrick