On Tue, May 27, 2025 at 9:51 AM Patrick Steinhardt <ps@xxxxxx> wrote: > > On Mon, May 19, 2025 at 04:11:18PM +0200, Christian Couder wrote: > > I am not sure I understand what you mean. This promisor.sendFields > > config variable is for the server side which advertises remotes. The > > server advertises its remotes (if it wants to) before receiving > > information from the client, so it cannot know what the client > > accepts. > > In the current form you need to reflow this whole paragraph every time a > new field is supported, and it's easy to miss the exact supported > fields. So my idea was to maybe move the supported fields into a > bulleted list. E.g.: > > promisor.sendFields:: > A comma or space separated list of additional remote related > fields that a server will send while advertising its promisor > remotes using the "promisor-remote" capability, see > linkgit:gitprotocol-v2[5]. The following fields are supported: > + > * "partialCloneFilter": contains the partial clone filter used for > the remote. > * "token": contains the authentication token for the remote. Yeah, the doc uses a bulleted list in v4. > > > Furthermore, should we maybe refactor this to match the restrictive > > > design where valid fields are explicitly specified? In other words, > > > should we have separate config keys for each of the accepted fields now? > > > > Maybe I don't understand what you mean with "accepted fields". > > I think I had a misunderstanding on my side. I didn't get that this is > only configuring field _names_ that we'll end up sending to the remote > side. So I thought that the user is expected to configure name-value > pairs here that are then sent to the client, not only the name. > > I guess this is mostly because the config documentation talks about > "fields", but that term is used elsewhere to indicate a name-value pair. I see. I have tried to use "field names" instead of "fields" everywhere it makes sense in the v4. > > > Does > > > it mean that this promisor remote should only be used in case we do have > > > the exact same filter passed to git-clone(1)? > > > > It's up to the client to decide, but yeah it will likely work better > > if the same filter is used. It should still work if a different filter > > is used though. In case the promisor remote doesn't have an object, > > there should be a fallback to ask the main server for that object. > > > > Also the filter mechanism already exists for a long time and this > > series doesn't change how it works. It's already possible to have > > different repos using the same promisor remote with different filters. > > So documentation about what happens when they do that should not be > > specific to this patch series. > > That's fair enough, but spelling this out somewhere and drawing the > bigger picture helps the reviewer understand the vision that you've got > here. Ok, I have added the following to the cover letter of the v4 about this: "Note that the filter mechanism already exists for a long time and this series doesn't change how it works. For example, it has already been possible for a long time to have different repos using the same promisor remote with different filters. See the existing partial clone documentation (like "Documentation/technical/partial-clone.adoc") for more information on partial clone."
