git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Jonathan Tan" <jonathantanmy@google.com>,
	"Josh Steadmon" <steadmon@google.com>,
	"Bruno Albuquerque" <bga@google.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 5/5] serve: add support for a git_config() callback
Date: Wed, 16 Jun 2021 16:16:21 +0200	[thread overview]
Message-ID: <patch-5.5-c8625025125-20210616T141332Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-0.5-00000000000-20210616T141332Z-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)
- 59e1205d167 (ls-refs: report unborn targets of symrefs, 2021-02-05)
- 6b5b6e422ee (serve: advertise session ID in v2 capabilities, 2020-11-11)

This code can be simplified by simply adding support to serve.c itself
for reading config at the right time before the capability is
called. In particular the ensure_config_read() function being gone
makes this whole thing much simpler.

A behavior change here is that we're eagerly doing the configuration
for all our capabilities regardless of which one we end up serving,
whereas before we'd defer it until the point where we were processing
the specific command.

We could try to be more lazy here and only read the config if we find
out that we're serving "ls-refs" or "session-id", but it's just a fast
git_config() callback, I think in this case simplicity trumps a
premature optimization.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 ls-refs.c | 55 ++++++++++++++++++-------------------------------------
 ls-refs.h |  1 +
 serve.c   | 34 +++++++++++++++++++++++++++++++++-
 3 files changed, 52 insertions(+), 38 deletions(-)

diff --git a/ls-refs.c b/ls-refs.c
index 88f6c3f60d8..a8a3e90bd71 100644
--- a/ls-refs.c
+++ b/ls-refs.c
@@ -7,38 +7,9 @@
 #include "pkt-line.h"
 #include "config.h"
 
-static int config_read;
-static int advertise_unborn;
-static int allow_unborn;
-
-static void ensure_config_read(void)
-{
-	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;
-			allow_unborn = 1;
-		} else if (!strcmp(str, "allow")) {
-			allow_unborn = 1;
-		} else if (!strcmp(str, "ignore")) {
-			/* do nothing */
-		} else {
-			die(_("invalid value '%s' for lsrefs.unborn"), str);
-		}
-	}
-	config_read = 1;
-}
+/* "unborn" is on by default if there's no lsrefs.unborn config */
+static int advertise_unborn = 1;
+static int allow_unborn = 1;
 
 /*
  * Check if one of the prefixes is a prefix of the ref.
@@ -128,8 +99,22 @@ static void send_possibly_unborn_head(struct ls_refs_data *data)
 	strbuf_release(&namespaced);
 }
 
-static int ls_refs_config(const char *var, const char *value, void *data)
+int ls_refs_configure(const char *var, const char *value, void *data)
 {
+	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(value, "ignore")) {
+			advertise_unborn = 0;
+			allow_unborn = 0;
+		} else {
+			die(_("invalid value '%s' for lsrefs.unborn"), value);
+		}
+	}
+
 	/*
 	 * We only serve fetches over v2 for now, so respect only "uploadpack"
 	 * config. This may need to eventually be expanded to "receive", but we
@@ -146,9 +131,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) {
 		const char *arg = request->line;
 		const char *out;
@@ -179,7 +161,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..afa6dfeb259 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_configure(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 49ea9fc8fd5..62cb8aeacdb 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_configure(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)
@@ -50,6 +57,13 @@ struct protocol_capability {
 	 */
 	const char *name;
 
+	/*
+	 * A git_config() callback. If defined it'll be dispatched too
+	 * sometime before the other functions are called, giving the
+	 * capability a chance to configure itself.
+	 */
+	int (*configure)(const char *var, const char *value, void *data);
+
 	/*
 	 * Function queried to see if a capability should be advertised.
 	 * Optionally a value can be specified by adding it to 'value'.
@@ -79,6 +93,7 @@ static struct protocol_capability capabilities[] = {
 	},
 	{
 		.name = "ls-refs",
+		.configure = ls_refs_configure,
 		.advertise = ls_refs_advertise,
 		.command = ls_refs,
 	},
@@ -97,6 +112,7 @@ static struct protocol_capability capabilities[] = {
 	},
 	{
 		.name = "session-id",
+		.configure = session_id_configure,
 		.advertise = session_id_advertise,
 	},
 	{
@@ -106,6 +122,22 @@ static struct protocol_capability capabilities[] = {
 	},
 };
 
+static int git_serve_config(const char *var, const char *value, void *data)
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(capabilities); i++) {
+		struct protocol_capability *c = &capabilities[i];
+		config_fn_t fn = c->configure;
+		int ret;
+		if (!fn)
+			continue;
+		ret = fn(var, value, data);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
 static void advertise_capabilities(void)
 {
 	struct strbuf capability = STRBUF_INIT;
@@ -303,7 +335,7 @@ 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);
+	git_config(git_serve_config, NULL);
 
 	if (options->advertise_capabilities || !options->stateless_rpc) {
 		/* serve by default supports v2 */
-- 
2.32.0.576.g59759b6ca7d


  parent reply	other threads:[~2021-06-16 14:16 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-16 14:16 [PATCH 0/5] serve: add "configure" callback Ævar Arnfjörð Bjarmason
2021-06-16 14:16 ` [PATCH 1/5] serve: mark has_capability() as static Ævar Arnfjörð Bjarmason
2021-06-16 16:28   ` Eric Sunshine
2021-06-17  0:45     ` Junio C Hamano
2021-06-16 14:16 ` [PATCH 2/5] transport: rename "fetch" in transport_vtable to "fetch_refs" Ævar Arnfjörð Bjarmason
2021-06-16 14:16 ` [PATCH 3/5] transport: use designated initializers Ævar Arnfjörð Bjarmason
2021-06-16 14:16 ` [PATCH 4/5] serve: " Ævar Arnfjörð Bjarmason
2021-06-16 14:16 ` Ævar Arnfjörð Bjarmason [this message]
2021-06-16 16:22   ` [PATCH 5/5] serve: add support for a git_config() callback Jeff King
2021-06-16 16:23 ` [PATCH 0/5] serve: add "configure" callback Jeff King
2021-06-17  0:49   ` Junio C Hamano
2021-06-28 19:19 ` [PATCH v2 0/8] serve: add "startup_config" callback Ævar Arnfjörð Bjarmason
2021-06-28 19:19   ` [PATCH v2 1/8] serve: mark has_capability() as static Ævar Arnfjörð Bjarmason
2021-06-28 19:19   ` [PATCH v2 2/8] transport: rename "fetch" in transport_vtable to "fetch_refs" Ævar Arnfjörð Bjarmason
2021-06-28 19:19   ` [PATCH v2 3/8] transport: use designated initializers Ævar Arnfjörð Bjarmason
2021-06-28 19:19   ` [PATCH v2 4/8] serve: " Ævar Arnfjörð Bjarmason
2021-06-28 19:19   ` [PATCH v2 5/8] serve.c: add call_{advertise,command}() indirection Ævar Arnfjörð Bjarmason
2021-06-28 19:19   ` [PATCH v2 6/8] serve.c: add trace2 regions for advertise & command Ævar Arnfjörð Bjarmason
2021-07-01 16:30     ` Jeff King
2021-07-02 12:54       ` Ævar Arnfjörð Bjarmason
2021-07-05 12:24     ` Han-Wen Nienhuys
2021-06-28 19:19   ` [PATCH v2 7/8] serve: add support for a "startup" git_config() callback Ævar Arnfjörð Bjarmason
2021-07-01 16:43     ` Jeff King
2021-07-01 16:47       ` Jeff King
2021-07-02 12:55       ` Ævar Arnfjörð Bjarmason
2021-07-02 21:13         ` Jeff King
2021-07-05 12:23     ` Han-Wen Nienhuys
2021-07-05 12:34     ` Han-Wen Nienhuys
2021-06-28 19:19   ` [PATCH v2 8/8] upload-pack.c: convert to new serve.c "startup" config cb Ævar Arnfjörð Bjarmason
2021-07-05 14:00     ` Han-Wen Nienhuys
2021-07-21 23:40   ` [PATCH v3 00/12] serve.[ch]: general API cleanup Ævar Arnfjörð Bjarmason
2021-07-21 23:40     ` [PATCH v3 01/12] serve: mark has_capability() as static Ævar Arnfjörð Bjarmason
2021-07-21 23:40     ` [PATCH v3 02/12] transport: rename "fetch" in transport_vtable to "fetch_refs" Ævar Arnfjörð Bjarmason
2021-07-21 23:40     ` [PATCH v3 03/12] transport: use designated initializers Ævar Arnfjörð Bjarmason
2021-07-21 23:40     ` [PATCH v3 04/12] serve: " Ævar Arnfjörð Bjarmason
2021-07-21 23:40     ` [PATCH v3 05/12] serve.c: add call_{advertise,command}() indirection Ævar Arnfjörð Bjarmason
2021-07-21 23:40     ` [PATCH v3 06/12] serve: add support for a "startup" git_config() callback Ævar Arnfjörð Bjarmason
2021-07-21 23:40     ` [PATCH v3 07/12] serve.c: move version line to advertise_capabilities() Ævar Arnfjörð Bjarmason
2021-07-21 23:40     ` [PATCH v3 08/12] serve.[ch]: remove "serve_options", split up --advertise-refs code Ævar Arnfjörð Bjarmason
2021-07-21 23:40     ` [PATCH v3 09/12] {upload,receive}-pack tests: add --advertise-refs tests Ævar Arnfjörð Bjarmason
2021-08-03  6:00       ` [PATCH] fixup! " Carlo Marcelo Arenas Belón
2021-07-21 23:40     ` [PATCH v3 10/12] upload-pack: document and rename --advertise-refs Ævar Arnfjörð Bjarmason
2021-07-21 23:40     ` [PATCH v3 11/12] upload-pack.c: convert to new serve.c "startup" config cb Ævar Arnfjörð Bjarmason
2021-07-21 23:40     ` [PATCH v3 12/12] serve.[ch]: don't pass "struct strvec *keys" to commands Ævar Arnfjörð Bjarmason
2021-08-02 21:07     ` [PATCH v3 00/12] serve.[ch]: general API cleanup Josh Steadmon
2021-08-05  1:25     ` [PATCH v4 00/10] serve.[ch]: general API cleanup + --advertise-refs cleanup Ævar Arnfjörð Bjarmason
2021-08-05  1:25       ` [PATCH v4 01/10] serve: mark has_capability() as static Ævar Arnfjörð Bjarmason
2021-08-05  1:25       ` [PATCH v4 02/10] transport: rename "fetch" in transport_vtable to "fetch_refs" Ævar Arnfjörð Bjarmason
2021-08-05  1:25       ` [PATCH v4 03/10] transport: use designated initializers Ævar Arnfjörð Bjarmason
2021-08-05  1:25       ` [PATCH v4 04/10] serve: " Ævar Arnfjörð Bjarmason
2021-08-05  1:25       ` [PATCH v4 05/10] serve.[ch]: don't pass "struct strvec *keys" to commands Ævar Arnfjörð Bjarmason
2021-08-05  1:25       ` [PATCH v4 06/10] serve: move transfer.advertiseSID check into session_id_advertise() Ævar Arnfjörð Bjarmason
2021-08-05  1:25       ` [PATCH v4 07/10] serve.c: move version line to advertise_capabilities() Ævar Arnfjörð Bjarmason
2021-08-05  1:25       ` [PATCH v4 08/10] {upload,receive}-pack tests: add --advertise-refs tests Ævar Arnfjörð Bjarmason
2021-08-05  1:25       ` [PATCH v4 09/10] serve.[ch]: remove "serve_options", split up --advertise-refs code Ævar Arnfjörð Bjarmason
2021-08-24 16:52         ` Derrick Stolee
2021-08-05  1:25       ` [PATCH v4 10/10] upload-pack: document and rename --advertise-refs Ævar Arnfjörð Bjarmason
2021-08-24 16:55       ` [PATCH v4 00/10] serve.[ch]: general API cleanup + --advertise-refs cleanup Derrick Stolee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-5.5-c8625025125-20210616T141332Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=bga@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    --cc=peff@peff.net \
    --cc=steadmon@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).