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

From: Taylor Blau <me@ttaylorr.com>
Date: Jul 31, 2024, 15:40 UTC
Message-ID: <Zqpa8/aLpgtzoBH2@nand.local>
In-Reply-To: <20240731134014.2299361-4-christian.couder@gmail.com>

On Wed, Jul 31, 2024 at 03:40:13PM +0200, Christian Couder wrote:

Show 5 quoted lines

> 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