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>

diff --git a/builtin/upload-pack.c b/builtin/upload-pack.c
index 6da8fa2607c6b831c69a76c5bb2ec5caec444d54..8506030a648e5b1866b233c93de4d2cb632fd9be 100644 (file)
--- a/builtin/upload-pack.c
+++ b/builtin/upload-pack.c
@@ -16,16 +16,17 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
 {
        const char *dir;
        int strict = 0;
-       struct upload_pack_options opts = { 0 };
-       struct serve_options serve_opts = SERVE_OPTIONS_INIT;
+       int advertise_refs = 0;
+       int stateless_rpc = 0;
+       int timeout = 0;
        struct option options[] = {
-               OPT_BOOL(0, "stateless-rpc", &opts.stateless_rpc,
+               OPT_BOOL(0, "stateless-rpc", &stateless_rpc,
                         N_("quit after a single request/response exchange")),
-               OPT_BOOL(0, "advertise-refs", &opts.advertise_refs,
+               OPT_BOOL(0, "advertise-refs", &advertise_refs,
                         N_("exit immediately after initial ref advertisement")),
                OPT_BOOL(0, "strict", &strict,
                         N_("do not try <directory>/.git/ if <directory> is no Git directory")),
-               OPT_INTEGER(0, "timeout", &opts.timeout,
+               OPT_INTEGER(0, "timeout", &timeout,
                            N_("interrupt transfer after <n> seconds of inactivity")),
                OPT_END()
        };
@@ -38,9 +39,6 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
        if (argc != 1)
                usage_with_options(upload_pack_usage, options);
 
-       if (opts.timeout)
-               opts.daemon_mode = 1;
-
        setup_path();
 
        dir = argv[0];
@@ -50,21 +48,22 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
 
        switch (determine_protocol_version_server()) {
        case protocol_v2:
-               serve_opts.advertise_capabilities = opts.advertise_refs;
-               serve_opts.stateless_rpc = opts.stateless_rpc;
-               serve(&serve_opts);
+               if (advertise_refs)
+                       protocol_v2_advertise_capabilities();
+               else
+                       protocol_v2_serve_loop(stateless_rpc);
                break;
        case protocol_v1:
                /*
                 * v1 is just the original protocol with a version string,
                 * so just fall through after writing the version string.
                 */
-               if (opts.advertise_refs || !opts.stateless_rpc)
+               if (advertise_refs || !stateless_rpc)
                        packet_write_fmt(1, "version 1\n");
 
                /* fallthrough */
        case protocol_v0:
-               upload_pack(&opts);
+               upload_pack(advertise_refs, stateless_rpc, timeout);
                break;
        case protocol_unknown_version:
                BUG("unknown protocol version");

diff --git a/http-backend.c b/http-backend.c
index b329bf63f097fadda16cbd4c8e1c06134b28d6a8..d37463cec8bf814ff4552364c37871249324f28d 100644 (file)
--- a/http-backend.c
+++ b/http-backend.c
@@ -534,7 +534,7 @@ static void get_info_refs(struct strbuf *hdr, char *arg)
 
        if (service_name) {
                const char *argv[] = {NULL /* service name */,
-                       "--stateless-rpc", "--advertise-refs",
+                       "--advertise-refs",
                        ".", NULL};
                struct rpc_service *svc = select_service(hdr, service_name);
 

diff --git a/serve.c b/serve.c
index 412c2cd13174417f779ba42fcdecee7f27ea37b7..1817edc7f57a30492a60a814ae5507b718ecb87f 100644 (file)
--- a/serve.c
+++ b/serve.c
@@ -106,7 +106,7 @@ static struct protocol_capability capabilities[] = {
        },
 };
 
-static void advertise_capabilities(void)
+void protocol_v2_advertise_capabilities(void)
 {
        struct strbuf capability = STRBUF_INIT;
        struct strbuf value = STRBUF_INIT;
@@ -303,24 +303,16 @@ static int process_request(void)
        return 0;
 }
 
-/* Main serve loop for protocol version 2 */
-void serve(struct serve_options *options)
+void protocol_v2_serve_loop(int stateless_rpc)
 {
-       if (options->advertise_capabilities || !options->stateless_rpc) {
-               advertise_capabilities();
-               /*
-                * If only the list of capabilities was requested exit
-                * immediately after advertising capabilities
-                */
-               if (options->advertise_capabilities)
-                       return;
-       }
+       if (!stateless_rpc)
+               protocol_v2_advertise_capabilities();
 
        /*
         * If stateless-rpc was requested then exit after
         * a single request/response exchange
         */
-       if (options->stateless_rpc) {
+       if (stateless_rpc) {
                process_request();
        } else {
                for (;;)

diff --git a/serve.h b/serve.h
index 56da77a87af40f1f81e6382bc0c1488628f649f3..f946cf904a242db5106625e280d7daa671348516 100644 (file)
--- a/serve.h
+++ b/serve.h
@@ -1,11 +1,7 @@
 #ifndef SERVE_H
 #define SERVE_H
 
-struct serve_options {
-       unsigned advertise_capabilities;
-       unsigned stateless_rpc;
-};
-#define SERVE_OPTIONS_INIT { 0 }
-void serve(struct serve_options *options);
+void protocol_v2_advertise_capabilities(void);
+void protocol_v2_serve_loop(int stateless_rpc);
 
 #endif /* SERVE_H */

diff --git a/t/helper/test-serve-v2.c b/t/helper/test-serve-v2.c
index aee35e5aef40431d5f572c71fa6aeebbe1bcf344..28e905afc36afdc3202ec68ac915b1ddc6a08781 100644 (file)
--- a/t/helper/test-serve-v2.c
+++ b/t/helper/test-serve-v2.c
@@ -10,12 +10,12 @@ static char const * const serve_usage[] = {
 
 int cmd__serve_v2(int argc, const char **argv)
 {
-       struct serve_options opts = SERVE_OPTIONS_INIT;
-
+       int stateless_rpc = 0;
+       int advertise_capabilities = 0;
        struct option options[] = {
-               OPT_BOOL(0, "stateless-rpc", &opts.stateless_rpc,
+               OPT_BOOL(0, "stateless-rpc", &stateless_rpc,
                         N_("quit after a single request/response exchange")),
-               OPT_BOOL(0, "advertise-capabilities", &opts.advertise_capabilities,
+               OPT_BOOL(0, "advertise-capabilities", &advertise_capabilities,
                         N_("exit immediately after advertising capabilities")),
                OPT_END()
        };
@@ -25,7 +25,11 @@ int cmd__serve_v2(int argc, const char **argv)
        argc = parse_options(argc, argv, prefix, options, serve_usage,
                             PARSE_OPT_KEEP_DASHDASH |
                             PARSE_OPT_KEEP_UNKNOWN);
-       serve(&opts);
+
+       if (advertise_capabilities)
+               protocol_v2_advertise_capabilities();
+       else
+               protocol_v2_serve_loop(stateless_rpc);
 
        return 0;
 }

diff --git a/upload-pack.c b/upload-pack.c
index ed60a9abd60cf90f4525ac80efb05a1060ed000b..fc38f04d983aa8394a97541a7cabc4b29b6cba6d 100644 (file)
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1214,7 +1214,7 @@ static int send_ref(const char *refname, const struct object_id *oid,
                                     " allow-tip-sha1-in-want" : "",
                             (data->allow_uor & ALLOW_REACHABLE_SHA1) ?
                                     " allow-reachable-sha1-in-want" : "",
-                            data->stateless_rpc ? " no-done" : "",
+                            data->no_done ? " no-done" : "",
                             symref_info.buf,
                             data->allow_filter ? " filter" : "",
                             session_id.buf,
@@ -1329,7 +1329,8 @@ static int upload_pack_config(const char *var, const char *value, void *cb_data)
        return parse_hide_refs_config(var, value, "uploadpack");
 }
 
-void upload_pack(struct upload_pack_options *options)
+void upload_pack(const int advertise_refs, const int stateless_rpc,
+                const int timeout)
 {
        struct packet_reader reader;
        struct upload_pack_data data;
@@ -1338,14 +1339,17 @@ void upload_pack(struct upload_pack_options *options)
 
        git_config(upload_pack_config, &data);
 
-       data.stateless_rpc = options->stateless_rpc;
-       data.daemon_mode = options->daemon_mode;
-       data.timeout = options->timeout;
+       data.stateless_rpc = stateless_rpc;
+       data.timeout = timeout;
+       if (data.timeout)
+               data.daemon_mode = 1;
 
        head_ref_namespaced(find_symref, &data.symref);
 
-       if (options->advertise_refs || !data.stateless_rpc) {
+       if (advertise_refs || !data.stateless_rpc) {
                reset_timeout(data.timeout);
+               if (advertise_refs)
+                       data.no_done = 1;
                head_ref_namespaced(send_ref, &data);
                for_each_namespaced_ref(send_ref, &data);
                advertise_shallow_grafts(1);
@@ -1355,7 +1359,7 @@ void upload_pack(struct upload_pack_options *options)
                for_each_namespaced_ref(check_ref, NULL);
        }
 
-       if (!options->advertise_refs) {
+       if (!advertise_refs) {
                packet_reader_init(&reader, 0, NULL, 0,
                                   PACKET_READ_CHOMP_NEWLINE |
                                   PACKET_READ_DIE_ON_ERR_PACKET);

diff --git a/upload-pack.h b/upload-pack.h
index 63e3252c98d13be5b72cdfe5ff96c01a91a2a9cb..d6ee25ea98e1b350377a3b9fb6e3750e516517f8 100644 (file)
--- a/upload-pack.h
+++ b/upload-pack.h
@@ -1,14 +1,8 @@
 #ifndef UPLOAD_PACK_H
 #define UPLOAD_PACK_H
 
-struct upload_pack_options {
-       int stateless_rpc;
-       int advertise_refs;
-       unsigned int timeout;
-       int daemon_mode;
-};
-
-void upload_pack(struct upload_pack_options *options);
+void upload_pack(const int advertise_refs, const int stateless_rpc,
+                const int timeout);
 
 struct repository;
 struct packet_reader;