On Tue, Sep 14 2021, Jeff King wrote:
> -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@xxxxxxxxx>
---
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
