Re: [PATCH 3/4] promisor-remote: allow a server to advertise extra fields

From: Christian Couder <christian.couder@gmail.com>
Date: Apr 29, 2025, 15:12 UTC
Message-ID: <CAP8UFD0UzX1-WsLzo0R6mND_dTpF6hPgD97Wvg4h3fqXO5-Bmg@mail.gmail.com>
In-Reply-To: <aAdr2Q9jthNV699i@pks.im>

On Tue, Apr 22, 2025 at 12:13 PM Patrick Steinhardt <ps@pks.im> wrote:

Show 5 quoted lines

> I agree that we should properly specify which fields are accepted and
> what their respective format as well as semantics would be. Otherwise,
> without such a definition, hosting sites may eventually end up with
> slightly incompatible semantics. I also don't expect that there should
> be all that many fields.

Yeah, I have done that in the next version where only "partialCloneFilter" and "token" are accepted and are documented.

> This raises a question though: what would happen if a field was
> advertised that the client doesn't understand? Should the client simply
> ignore such a field?

Yes, for backward compatibility we need them to ignore fields they don't understand.

> Should they bail out?

No, this would prevent clients that are a bit old but could work with a server to just not be able to work anymore as soon as the server tries new features.

Show 6 quoted lines

> I think we need to also think
> about this edge case and specify client-side behaviour. I think in the
> end, both ways would be rather limiting:
>
>   - If we simply ignored all unknown fields our hands might be bound if
>     we ever had to introduce changes that aren't backwards compatible.

Yeah, but that still leaves backward compatible changes as OK.

>   - If we always bail out on an unknown field our hands would be bound
>     equally, as we cannot ever introduce a new field.

Yeah, that's worse.

> Which raises the question whether we need to be able to dynamically
> figure out fields. This could be in the form of capability negotiation
> or protocol versions.

There is no real negotiation. The server advertises some remotes and associated fields, and the client just accepts some of these remotes or not. We would need a separate protocol capability if we wanted a real capability negotiation.

> But in any case, I think we need to have something
> ready so that we can change behaviour depending on which features are
> supported by a client.

If the client ignores fields it doesn't know, a server can provide both some fields for old clients and new fields for new clients. Then the client itself will be able to use what looks best for itself.