Re: [PATCH 3/4] Add 'promisor-remote' capability to protocol v2

[Taylor Blau <me@ttaylorr.com>]

On Wed, Jul 31, 2024 at 03:40:13PM +0200, Christian Couder wrote:
> By default, or if "promisor.advertise" is set to 'false', a server S will
> advertise only the "promisor-remote" capability without passing any
> argument through this capability. This means that S supports the new
> capability but doesn't wish any client C to directly access any promisor
> remote X S might use.

Even if the server supports this new capability, is there a reason to
advertise it to the client if the server knows ahead of time that it has
no promisor remotes to advertise?

I am not sure what action the client would take if it knows the server
supports this capability, but does not actually have any promisor
remotes to advertise. I would suggest that setting promisor.advertise to
false indeed prevents advertising it as a capability in the first place.

Selfishly, it prevents some issues that I have when rolling out new Git
versions within GitHub's infrastructure, since our push proxy layer
picks a single replica to replay the capabilities from, but obviously
replays the client's response to all replicas. So if only some replicas
understand the new 'promisor-remote' capability, we can run into issues.

I'm not sure if the client even bothers to send back promisor-remote if
the server did not send any such remotes to begin with, but between that
and what I wrote in the second paragraph here, I don't see a reason to
advertise the capability when promisor.advertise is false.

Thanks,
Taylor