Re: [PATCH v2 03/11] serve: add "receive" method for v2 capabilities table
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Sep 15, 2021, 00:31 UTC
- Message-ID
- <87sfy6mzzg.fsf@evledraar.gmail.com>
- In-Reply-To
- <YUE1fGZc1FuuyUNH@coredump.intra.peff.net>
On Tue, Sep 14 2021, Jeff King wrote:
Show 15 quoted lines
> -static int is_valid_capability(const char *key)
> +static int receive_client_capability(const char *key)
> {
> const char *value;
> const struct protocol_capability *c = get_capability(key, &value);
>
> - return c && c->advertise(the_repository, NULL);
> + if (!c || !c->advertise(the_repository, NULL))
> + return 0;
> +
> + if (c->receive)
> + c->receive(the_repository, value);
> + return 1;
> }
> I haven't actually run this yet (and need to zZzZ soon), but AFAICT at the end of this series you leave the existing advertise semantics of advertise() be (which is fine). I have this unsubmitted patch locally which you may or may not want to work into this.
I considered splitting up the advertise() method as well, i.e. we could have a new "is_advertised" boolean callback, and then a "capability_string" or whatever. "server-option" and "object-info" never add anything, so they could leave that as NULL.
But it's probably not worth it, just food for thought...
-- >8 -- serve: document that "advertise" is called in two modes
If we're being called with a non-NULL argument we're sending out the advertisement line, but if it's NULL we're actually in the middle of a request.
So you can use the check for NULL to emit your own "die" on "return 0", like "wtf, I said I don't support command xyz, why are you calling it?", as opposed to the default "invalid command '%s'".
Maybe nobody cares, and we can't assume that we're going from an advertisement to a command for the same request anyway (can we?). I.e. are we serving multiple clients?
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- serve.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/serve.c b/serve.c index aa8209f147e..b187ce26911 100644 --- a/serve.c +++ b/serve.c @@ -55,6 +55,12 @@ struct protocol_capability { * Optionally a value can be specified by adding it to 'value'. * If a value is added to 'value', the server will advertise this * capability as "<name>=<value>" instead of "<name>". + * + * When called with a NULL value we're past the advertisement + * itself, and are checking during a request whether we + * support this command. This can be checked and used used to + * e.g. emit a die("we support this, but don't have it + * enabled!") error. */ int (*advertise)(struct repository *r, struct strbuf *value);
-- 2.33.0.1013.ge8323766266