serve.[ch]: remove "serve_options", split up --advertise-refs code

The "advertise capabilities" mode of serve.c added in ed10cb952d3 (serve: introduce git-serve, 2018-03-15) is only used by the http-backend.c to call {upload,receive}-pack with the --advertise-refs parameter. See 42526b478e3 (Add stateless RPC options to upload-pack, receive-pack, 2009-10-30). Let's just make cmd_upload_pack() take the two (v2) or three (v2) parameters the the v2/v1 servicing functions need directly, and pass those in via the function signature. The logic of whether daemon mode is implied by the timeout belongs in the v1 function (only used there). Once we split up the "advertise v2 refs" from "serve v2 request" it becomes clear that v2 never cared about those in combination. The only time it mattered was for v1 to emit its ref advertisement, in that case we wanted to emit the smart-http-only "no-done" capability. Since we only do that in the --advertise-refs codepath let's just have it set "do_done" itself in v1's upload_pack() just before send_ref(), at that point --advertise-refs and --stateless-rpc in combination are redundant (the only user is get_info_refs() in http-backend.c), so we can just pass in --advertise-refs only. Since we need to touch all the serve() and advertise_capabilities() codepaths let's rename them to less clever and obvious names, it's been suggested numerous times, the latest of which is [1]'s suggestion for protocol_v2_serve_loop(). Let's go with that. 1. https://lore.kernel.org/git/CAFQ2z_NyGb8rju5CKzmo6KhZXD0Dp21u-BbyCb2aNxLEoSPRJw@mail.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Aug 5, 2021 at 03:25 UTC f234da80197b3115cd3c280e98d3393a884a9327
7 files changed +42 -53
builtin/upload-pack.c
+12 -13
@@ -16,16 +16,17 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
16 {
17 const char *dir;
18 int strict = 0;
19 - struct upload_pack_options opts = { 0 };
20 - struct serve_options serve_opts = SERVE_OPTIONS_INIT;
19 + int advertise_refs = 0;
20 + int stateless_rpc = 0;
21 + int timeout = 0;
22 struct option options[] = {
22 - OPT_BOOL(0, "stateless-rpc", &opts.stateless_rpc,
23 + OPT_BOOL(0, "stateless-rpc", &stateless_rpc,
24 N_("quit after a single request/response exchange")),
24 - OPT_BOOL(0, "advertise-refs", &opts.advertise_refs,
25 + OPT_BOOL(0, "advertise-refs", &advertise_refs,
26 N_("exit immediately after initial ref advertisement")),
27 OPT_BOOL(0, "strict", &strict,
28 N_("do not try <directory>/.git/ if <directory> is no Git directory")),
28 - OPT_INTEGER(0, "timeout", &opts.timeout,
29 + OPT_INTEGER(0, "timeout", &timeout,
30 N_("interrupt transfer after <n> seconds of inactivity")),
31 OPT_END()
32 };
@@ -38,9 +39,6 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
39 if (argc != 1)
40 usage_with_options(upload_pack_usage, options);
41
41 - if (opts.timeout)
42 - opts.daemon_mode = 1;
43 -
42 setup_path();
43
44 dir = argv[0];
@@ -50,21 +48,22 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
48
49 switch (determine_protocol_version_server()) {
50 case protocol_v2:
53 - serve_opts.advertise_capabilities = opts.advertise_refs;
54 - serve_opts.stateless_rpc = opts.stateless_rpc;
55 - serve(&serve_opts);
51 + if (advertise_refs)
52 + protocol_v2_advertise_capabilities();
53 + else
54 + protocol_v2_serve_loop(stateless_rpc);
55 break;
56 case protocol_v1:
57 /*
58 * v1 is just the original protocol with a version string,
59 * so just fall through after writing the version string.
60 */
62 - if (opts.advertise_refs || !opts.stateless_rpc)
61 + if (advertise_refs || !stateless_rpc)
62 packet_write_fmt(1, "version 1\n");
63
64 /* fallthrough */
65 case protocol_v0:
67 - upload_pack(&opts);
66 + upload_pack(advertise_refs, stateless_rpc, timeout);
67 break;
68 case protocol_unknown_version:
69 BUG("unknown protocol version");
http-backend.c
+1 -1
@@ -534,7 +534,7 @@ static void get_info_refs(struct strbuf *hdr, char *arg)
534
535 if (service_name) {
536 const char *argv[] = {NULL /* service name */,
537 - "--stateless-rpc", "--advertise-refs",
537 + "--advertise-refs",
538 ".", NULL};
539 struct rpc_service *svc = select_service(hdr, service_name);
540
serve.c
+5 -13
@@ -106,7 +106,7 @@ static struct protocol_capability capabilities[] = {
106 },
107 };
108
109 -static void advertise_capabilities(void)
109 +void protocol_v2_advertise_capabilities(void)
110 {
111 struct strbuf capability = STRBUF_INIT;
112 struct strbuf value = STRBUF_INIT;
@@ -303,24 +303,16 @@ static int process_request(void)
303 return 0;
304 }
305
306 -/* Main serve loop for protocol version 2 */
307 -void serve(struct serve_options *options)
306 +void protocol_v2_serve_loop(int stateless_rpc)
307 {
309 - if (options->advertise_capabilities || !options->stateless_rpc) {
310 - advertise_capabilities();
311 - /*
312 - * If only the list of capabilities was requested exit
313 - * immediately after advertising capabilities
314 - */
315 - if (options->advertise_capabilities)
316 - return;
317 - }
308 + if (!stateless_rpc)
309 + protocol_v2_advertise_capabilities();
310
311 /*
312 * If stateless-rpc was requested then exit after
313 * a single request/response exchange
314 */
323 - if (options->stateless_rpc) {
315 + if (stateless_rpc) {
316 process_request();
317 } else {
318 for (;;)
serve.h
+2 -6
@@ -1,11 +1,7 @@
1 #ifndef SERVE_H
2 #define SERVE_H
3
4 -struct serve_options {
5 - unsigned advertise_capabilities;
6 - unsigned stateless_rpc;
7 -};
8 -#define SERVE_OPTIONS_INIT { 0 }
9 -void serve(struct serve_options *options);
4 +void protocol_v2_advertise_capabilities(void);
5 +void protocol_v2_serve_loop(int stateless_rpc);
6
7 #endif /* SERVE_H */
t/helper/test-serve-v2.c
+9 -5
@@ -10,12 +10,12 @@ static char const * const serve_usage[] = {
10
11 int cmd__serve_v2(int argc, const char **argv)
12 {
13 - struct serve_options opts = SERVE_OPTIONS_INIT;
14 -
13 + int stateless_rpc = 0;
14 + int advertise_capabilities = 0;
15 struct option options[] = {
16 - OPT_BOOL(0, "stateless-rpc", &opts.stateless_rpc,
16 + OPT_BOOL(0, "stateless-rpc", &stateless_rpc,
17 N_("quit after a single request/response exchange")),
18 - OPT_BOOL(0, "advertise-capabilities", &opts.advertise_capabilities,
18 + OPT_BOOL(0, "advertise-capabilities", &advertise_capabilities,
19 N_("exit immediately after advertising capabilities")),
20 OPT_END()
21 };
@@ -25,7 +25,11 @@ int cmd__serve_v2(int argc, const char **argv)
25 argc = parse_options(argc, argv, prefix, options, serve_usage,
26 PARSE_OPT_KEEP_DASHDASH |
27 PARSE_OPT_KEEP_UNKNOWN);
28 - serve(&opts);
28 +
29 + if (advertise_capabilities)
30 + protocol_v2_advertise_capabilities();
31 + else
32 + protocol_v2_serve_loop(stateless_rpc);
33
34 return 0;
35 }
upload-pack.c
+11 -7
@@ -1214,7 +1214,7 @@ static int send_ref(const char *refname, const struct object_id *oid,
1214 " allow-tip-sha1-in-want" : "",
1215 (data->allow_uor & ALLOW_REACHABLE_SHA1) ?
1216 " allow-reachable-sha1-in-want" : "",
1217 - data->stateless_rpc ? " no-done" : "",
1217 + data->no_done ? " no-done" : "",
1218 symref_info.buf,
1219 data->allow_filter ? " filter" : "",
1220 session_id.buf,
@@ -1329,7 +1329,8 @@ static int upload_pack_config(const char *var, const char *value, void *cb_data)
1329 return parse_hide_refs_config(var, value, "uploadpack");
1330 }
1331
1332 -void upload_pack(struct upload_pack_options *options)
1332 +void upload_pack(const int advertise_refs, const int stateless_rpc,
1333 + const int timeout)
1334 {
1335 struct packet_reader reader;
1336 struct upload_pack_data data;
@@ -1338,14 +1339,17 @@ void upload_pack(struct upload_pack_options *options)
1339
1340 git_config(upload_pack_config, &data);
1341
1341 - data.stateless_rpc = options->stateless_rpc;
1342 - data.daemon_mode = options->daemon_mode;
1343 - data.timeout = options->timeout;
1342 + data.stateless_rpc = stateless_rpc;
1343 + data.timeout = timeout;
1344 + if (data.timeout)
1345 + data.daemon_mode = 1;
1346
1347 head_ref_namespaced(find_symref, &data.symref);
1348
1347 - if (options->advertise_refs || !data.stateless_rpc) {
1349 + if (advertise_refs || !data.stateless_rpc) {
1350 reset_timeout(data.timeout);
1351 + if (advertise_refs)
1352 + data.no_done = 1;
1353 head_ref_namespaced(send_ref, &data);
1354 for_each_namespaced_ref(send_ref, &data);
1355 advertise_shallow_grafts(1);
@@ -1355,7 +1359,7 @@ void upload_pack(struct upload_pack_options *options)
1359 for_each_namespaced_ref(check_ref, NULL);
1360 }
1361
1358 - if (!options->advertise_refs) {
1362 + if (!advertise_refs) {
1363 packet_reader_init(&reader, 0, NULL, 0,
1364 PACKET_READ_CHOMP_NEWLINE |
1365 PACKET_READ_DIE_ON_ERR_PACKET);
upload-pack.h
+2 -8
@@ -1,14 +1,8 @@
1 #ifndef UPLOAD_PACK_H
2 #define UPLOAD_PACK_H
3
4 -struct upload_pack_options {
5 - int stateless_rpc;
6 - int advertise_refs;
7 - unsigned int timeout;
8 - int daemon_mode;
9 -};
10 -
11 -void upload_pack(struct upload_pack_options *options);
4 +void upload_pack(const int advertise_refs, const int stateless_rpc,
5 + const int timeout);
6
7 struct repository;
8 struct packet_reader;