[PATCH v2 7/8] serve: add support for a "startup" git_config() callback
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jun 28, 2021, 19:19 UTC
- Message-ID
- <patch-7.8-0a4fb01ae38-20210628T191634Z-avarab@gmail.com>
- In-Reply-To
- <cover-0.8-00000000000-20210628T191634Z-avarab@gmail.com>
Since the introduction of serve.c we've added git_config() callbacks and other config reading for capabilities in the following commits:
- e20b4192a37 (upload-pack: support hidden refs with protocol v2, 2018-12-18) - 08450ef7918 (upload-pack: clear filter_options for each v2 fetch command, 2020-05-08) - 6b5b6e422ee (serve: advertise session ID in v2 capabilities, 2020-11-11) - 59e1205d167 (ls-refs: report unborn targets of symrefs, 2021-02-05)
Of these 08450ef7918 fixed code that needed to read config on a per-request basis, whereas most of the config reading just wants to check if we've enabled one semi-static config variable or other. We'd like to re-read that value eventually, but from request-to-request it's OK if we retain the old one, and it isn't impacted by other request data.
So let's support this common pattern as a "startup_config" callback, making use of our recently added "call_{advertise,command}()" functions. This allows us to simplify e.g. the "ensure_config_read()" function added in 59e1205d167 (ls-refs: report unborn targets of symrefs, 2021-02-05).
We could read all the config for all the protocol capabilities, but let's do it one callback at a time in anticipation that some won't be called at all, and that some might be more expensive than others in the future.
I'm not migrating over the code in the upload_pack_v2 function in upload-pack.c yet, that case is more complex since it deals with both v1 and v2. It will be dealt with in a code a subsequent commit.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- ls-refs.c | 42 ++++++++++++++---------------------------- ls-refs.h | 1 + serve.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 30 deletions(-)
diff --git a/ls-refs.c b/ls-refs.c index 88f6c3f60d8..02fbcfd9bde 100644 --- a/ls-refs.c +++ b/ls-refs.c @@ -7,37 +7,26 @@ #include "pkt-line.h" #include "config.h" -static int config_read; -static int advertise_unborn; -static int allow_unborn; +/* "unborn" is on by default if there's no lsrefs.unborn config */ +static int advertise_unborn = 1; +static int allow_unborn = 1; -static void ensure_config_read(void) +int ls_refs_startup_config(const char *var, const char *value, void *data) { - const char *str = NULL; - - if (config_read) - return; - - if (repo_config_get_string_tmp(the_repository, "lsrefs.unborn", &str)) { - /* - * If there is no such config, advertise and allow it by - * default. - */ - advertise_unborn = 1; - allow_unborn = 1; - } else { - if (!strcmp(str, "advertise")) { - advertise_unborn = 1; + if (!strcmp(var, "lsrefs.unborn")) { + if (!strcmp(value, "advertise")) { + /* Allowed and advertised by default */ + } else if (!strcmp(value, "allow")) { + advertise_unborn = 0; allow_unborn = 1; - } else if (!strcmp(str, "allow")) { - allow_unborn = 1; - } else if (!strcmp(str, "ignore")) { - /* do nothing */ + } else if (!strcmp(value, "ignore")) { + advertise_unborn = 0; + allow_unborn = 0; } else { - die(_("invalid value '%s' for lsrefs.unborn"), str); + die(_("invalid value '%s' for lsrefs.unborn"), value); } } - config_read = 1; + return 0; } /* @@ -145,8 +134,6 @@ int ls_refs(struct repository *r, struct strvec *keys, memset(&data, 0, sizeof(data)); strvec_init(&data.prefixes); - - ensure_config_read(); git_config(ls_refs_config, NULL); while (packet_reader_read(request) == PACKET_READ_NORMAL) { @@ -179,7 +166,6 @@ int ls_refs(struct repository *r, struct strvec *keys, int ls_refs_advertise(struct repository *r, struct strbuf *value) { if (value) { - ensure_config_read(); if (advertise_unborn) strbuf_addstr(value, "unborn"); } diff --git a/ls-refs.h b/ls-refs.h index a99e4be0bdd..5fcab0d1dca 100644 --- a/ls-refs.h +++ b/ls-refs.h @@ -6,6 +6,7 @@ struct strvec; struct packet_reader; int ls_refs(struct repository *r, struct strvec *keys, struct packet_reader *request); +int ls_refs_startup_config(const char *var, const char *value, void *data); int ls_refs_advertise(struct repository *r, struct strbuf *value); #endif /* LS_REFS_H */ diff --git a/serve.c b/serve.c index 6dbd05248b9..cce96dd5a81 100644 --- a/serve.c +++ b/serve.c @@ -33,6 +33,13 @@ static int object_format_advertise(struct repository *r, return 1; } +static int session_id_startup_config(const char *var, const char *value, void *data) +{ + if (!strcmp(var, "transfer.advertisesid")) + advertise_sid = git_config_bool(var, value); + return 0; +} + static int session_id_advertise(struct repository *r, struct strbuf *value) { if (!advertise_sid) @@ -54,6 +61,24 @@ struct protocol_capability { */ const char *name; + /* + * A git_config() callback that'll be called only once for the + * lifetime of the process, possibly over many different + * requests. Used for reading config that's expected to be + * static. + * + * The "command" or "advertise" callbacks themselves are + * expected to read config that needs to be more current than + * that, or which is dependent on request data. + */ + int (*startup_config)(const char *var, const char *value, void *data); + + /* + * A boolean to check if we've called our "startup_config" + * callback. + */ + int have_startup_config; + /* * Function queried to see if a capability should be advertised. * Optionally a value can be specified by adding it to 'value'. @@ -81,6 +106,7 @@ static struct protocol_capability capabilities[] = { }, { .name = "ls-refs", + .startup_config = ls_refs_startup_config, .advertise = ls_refs_advertise, .command = ls_refs, }, @@ -99,6 +125,7 @@ static struct protocol_capability capabilities[] = { }, { .name = "session-id", + .startup_config = session_id_startup_config, .advertise = session_id_advertise, }, { @@ -108,6 +135,15 @@ static struct protocol_capability capabilities[] = { }, }; +static void read_startup_config(struct protocol_capability *command) +{ + if (!command->startup_config) + return; + if (command->have_startup_config++) + return; + git_config(command->startup_config, NULL); +} + static int call_advertise(struct protocol_capability *command, struct repository *r, struct strbuf *value) { @@ -115,6 +151,8 @@ static int call_advertise(struct protocol_capability *command, struct strbuf sb = STRBUF_INIT; const char *msg; + read_startup_config(command); + strbuf_addf(&sb, "advertise/%s", command->name); trace2_region_enter("serve", sb.buf, r); ret = command->advertise(r, value); @@ -132,6 +170,8 @@ static int call_command(struct protocol_capability *command, int ret; struct strbuf sb = STRBUF_INIT; + read_startup_config(command); + strbuf_addf(&sb, "command/%s", command->name); trace2_region_enter("serve", sb.buf, r); ret = command->command(r, keys, request); @@ -338,8 +378,6 @@ static int process_request(void) /* Main serve loop for protocol version 2 */ void serve(struct serve_options *options) { - git_config_get_bool("transfer.advertisesid", &advertise_sid); - if (options->advertise_capabilities || !options->stateless_rpc) { /* serve by default supports v2 */ packet_write_fmt(1, "version 2\n");
-- 2.32.0.611.gd4a17395dfa