transport: add --show-service option

From: ZheNing Hu <[email protected]>

Without using protocol v2, the git server needs to send a pktline "# service=$servicename" to the git client first. This often requires the git server to implement it independently, but it can be delegated to the git receive-pack and git upload-pack to complete the work proactively. Therefore, the --show-service option is added to git receive-pack and git upload-pack, which can be used to send the "# service=$servicename" pktline, making the logic of the git server more concise.

Note that this --show-service option can only be used together with --http-backend-info-refs and it is not applicable when using protocol v2.

Signed-off-by ZheNing Hu <[email protected]>

---

Quoted 14 lines · ZheNing Hu via GitGitGadget

From: ZheNing Hu <[email protected]>

Without using protocol v2, the git server needs to send a pktline "# service=$servicename" to the git client first. This often requires the git server to implement it independently, but it can be delegated to the git receive-pack and git upload-pack to complete the work proactively. Therefore, the --show-service option is added to git receive-pack and git upload-pack, which can be used to send the "# service=$servicename" pktline, making the logic of the git server more concise.

Note that this --show-service option can only be used together with --http-backend-info-refs and it is not applicable when using protocol v2.

Quoted 2 lines

Signed-off-by ZheNing Hu <[email protected]>

---

The above is a very well written description of why we _can_ do this, but it is unclear why we would _want_ to do this. What do we gain by letting the individual services to declare "we are serving this!"? Do we lose an extra fork & process (it does not seem to be the case)?

Quoted 5 lines

The above is a very well written description of why we _can_ do this, but it is unclear why we would _want_ to do this. What do we gain by letting the individual services to declare "we are serving this!"? Do we lose an extra fork & process (it does not seem to be the case)?

When I was implementing a small git http server, I wanted to delegate everything to upload-pack and receive-pack. Everything else was fine, the only thing I was not satisfied with was the need to send an additional "#service=$servername" message when the protocol is v1.

From: ZheNing Hu <[email protected]>

Without using protocol v2, the git server needs to send a pktline "#service=$servicename" to the git client first. The servername here can only be selected in "git-receive-pack and "git-upload-pack", neither the git client nor the server depends on this servername to perform any functions. Howerver, , implementing this capability independently can be cumbersome for the git server, as seen in [1].

To simplify this process, the --show-service option was added to git-receive-pack and git-upload-pack. This option can be used to send the '#service=$servicename' pktline, making the logic of the git server more concise. Note that this option can only be used together with --http-backend-info-refs and is not applicable when using protocol v2.

[1]: https://gitlab.com/gitlab-org/gitaly/-/blob/master/internal/gitaly/service/smarthttp/inforefs.go#L82

Signed-off-by ZheNing Hu <[email protected]>

---
[RFC] transport: add --show-service option

When the protocol is not v2, the git client requires that the first pktline reply for info refs be "# service=servicename", which requires the git server to implement pktline capability, e.g. [1] , which may be a bit cumbersome.

Delegating this feature to git upload-pack and git receive-pack via "--show-service" can simplify server implementation.

v1. add --show-service to git upload-pack and git receive-pack. v2. amend the git commit message to explain the reason for adding the option.

[1]:
https://gitlab.com/gitlab-org/gitaly/-/blob/master/internal/gitaly/service/smarthttp/inforefs.go#L82

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1523%2Fadlternative%2Fzh%2Finfo-ref-service-output-opt-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1523/adlternative/zh/info-ref-service-output-opt-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1523

Range-diff vs v1:

1: a2d33d6857a ! 1: b8aa0e7e053 [RFC] transport: add --show-service option
@@ Commit message
[RFC] transport: add --show-service option

Without using protocol v2, the git server needs to send a pktline
- "# service=$servicename" to the git client first. This often
- requires the git server to implement it independently, but it can
- be delegated to the git receive-pack and git upload-pack to complete
- the work proactively. Therefore, the --show-service option is added
- to git receive-pack and git upload-pack, which can be used to send
- the "# service=$servicename" pktline, making the logic of the git
- server more concise.
+ "#service=$servicename" to the git client first. The servername
+ here can only be selected in "git-receive-pack and "git-upload-pack",
+ neither the git client nor the server depends on this servername to
+ perform any functions. Howerver, , implementing this capability
+ independently can be cumbersome for the git server, as seen in [1].

- Note that this --show-service option can only be used together with
- --http-backend-info-refs and it is not applicable when using protocol v2.
+ To simplify this process, the --show-service option was added to
+ git-receive-pack and git-upload-pack. This option can be used to
+ send the '#service=$servicename' pktline, making the logic of the
+ git server more concise. Note that this option can only be used
+ together with --http-backend-info-refs and is not applicable when
+ using protocol v2.
+
+ [1]: https://gitlab.com/gitlab-org/gitaly/-/blob/master/internal/gitaly/service/smarthttp/inforefs.go#L82

Signed-off-by ZheNing Hu <[email protected]>

 Documentation/git-receive-pack.txt |  10 +++
 Documentation/git-upload-pack.txt  |  13 +++-
 builtin/receive-pack.c             |  14 +++-
 builtin/upload-pack.c              |  17 +++-
 http-backend.c                     |   7 +-
 t/t5555-http-smart-common.sh       | 120 +++++++++++++++++++++++++++++
 6 files changed, 171 insertions(+), 10 deletions(-)

Documentation/git-receive-pack.txt +10 -0

index 65ff518ccff..e16d364f394 100644
@@ -46,6 +46,16 @@ OPTIONS
        `$GIT_URL/info/refs?service=git-receive-pack` requests. See
        `--http-backend-info-refs` in linkgit:git-upload-pack[1].
 
+--show-service::
+       Output the "# service=git-receive-pack" pktline and the
+       "0000" flush pktline firstly. Since the git client needs
+       the git server to send the first pktline
+       "# service=$servicename", this option allows the git
+       server to delegate the functionality of sending this pktline
+       to `git-receive-pack`.
+       Note that this option can only be used together with
+       `--http-backend-info-refs`.
+
 PRE-RECEIVE HOOK
 ----------------
 Before any ref is updated, if $GIT_DIR/hooks/pre-receive file exists

Documentation/git-upload-pack.txt +12 -1

index b656b475675..7052708d03e 100644
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git-upload-pack' [--[no-]strict] [--timeout=<n>] [--stateless-rpc]
-                 [--advertise-refs] <directory>
+                 [--advertise-refs] [--show-service] <directory>
 
 DESCRIPTION
 -----------

@@ -44,6 +44,17 @@ OPTIONS
        documentation. Also understood by
        linkgit:git-receive-pack[1].
 
+--show-service::
+       Output the "# service=git-upload-pack" pktline and the
+       "0000" flush pktline firstly. Since the git client needs
+       the git server to send the first pktline
+       "# service=$servicename", this option allows the git
+       server to delegate the functionality of sending this pktline
+       to `git-upload-pack`.
+       Note that this option can only be used together with
+       `--http-backend-info-refs` and it is not applicable when
+       using protocol v2.
+
 <directory>::
        The repository to sync from.
 

builtin/receive-pack.c +13 -1

index 9109552533d..eb45c1f72af 100644
@@ -2485,6 +2485,7 @@ static int delete_only(struct command *commands)
 int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 {
        int advertise_refs = 0;
+       int show_service = 0;
        struct command *commands;
        struct oid_array shallow = OID_ARRAY_INIT;
        struct oid_array ref = OID_ARRAY_INIT;

@@ -2497,8 +2498,10 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
                OPT_HIDDEN_BOOL(0, "http-backend-info-refs", &advertise_refs, NULL),
                OPT_ALIAS(0, "advertise-refs", "http-backend-info-refs"),
                OPT_HIDDEN_BOOL(0, "reject-thin-pack-for-testing", &reject_thin, NULL),
+               OPT_BOOL(0, "show-service", &show_service, N_("show service information")),
                OPT_END()
        };
+       enum protocol_version version = determine_protocol_version_server();
 
        packet_trace_identity("receive-pack");
 

@@ -2525,7 +2528,16 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
        else if (0 <= receive_unpack_limit)
                unpack_limit = receive_unpack_limit;
 
-       switch (determine_protocol_version_server()) {
+       if (show_service) {
+               if (!advertise_refs)
+                       die(_("options '%s' and '%s' should be used together"), "--show-service", "--http-backend-info-refs");
+               if (version != protocol_v2) {
+                       packet_write_fmt(1, "# service=git-receive-pack\n");
+                       packet_flush(1);
+               }
+       }
+
+       switch (version) {
        case protocol_v2:
                /*
                 * push support for protocol v2 has not been implemented yet,

builtin/upload-pack.c +15 -2

index beb9dd08610..e84eb3735b4 100644
@@ -11,7 +11,7 @@
 
 static const char * const upload_pack_usage[] = {
        N_("git-upload-pack [--[no-]strict] [--timeout=<n>] [--stateless-rpc]\n"
-          "                [--advertise-refs] <directory>"),
+          "                [--advertise-refs] [--show-service] <directory>"),
        NULL
 };
 

@@ -22,6 +22,7 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
        int advertise_refs = 0;
        int stateless_rpc = 0;
        int timeout = 0;
+       int show_service = 0;
        struct option options[] = {
                OPT_BOOL(0, "stateless-rpc", &stateless_rpc,
                         N_("quit after a single request/response exchange")),

@@ -32,8 +33,10 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
                         N_("do not try <directory>/.git/ if <directory> is no Git directory")),
                OPT_INTEGER(0, "timeout", &timeout,
                            N_("interrupt transfer after <n> seconds of inactivity")),
+               OPT_BOOL(0, "show-service", &show_service, N_("show service information")),
                OPT_END()
        };
+       enum protocol_version version = determine_protocol_version_server();
 
        packet_trace_identity("upload-pack");
        read_replace_refs = 0;

@@ -50,7 +53,17 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
        if (!enter_repo(dir, strict))
                die("'%s' does not appear to be a git repository", dir);
 
-       switch (determine_protocol_version_server()) {
+
+       if (show_service) {
+               if (!advertise_refs)
+                       die(_("options '%s' and '%s' should be used together"), "--show-service", "--http-backend-info-refs");
+               if (version != protocol_v2) {
+                       packet_write_fmt(1, "# service=git-upload-pack\n");
+                       packet_flush(1);
+               }
+       }
+
+       switch (version) {
        case protocol_v2:
                if (advertise_refs)
                        protocol_v2_advertise_capabilities();

http-backend.c +1 -6

index 89aad1b42c7..74c2c7bb606 100644
@@ -539,6 +539,7 @@ static void get_info_refs(struct strbuf *hdr, char *arg UNUSED)
        if (service_name) {
                const char *argv[] = {NULL /* service name */,
                        "--http-backend-info-refs",
+                       "--show-service",
                        ".", NULL};
                struct rpc_service *svc = select_service(hdr, service_name);
 

@@ -547,12 +548,6 @@ static void get_info_refs(struct strbuf *hdr, char *arg UNUSED)
                hdr_str(hdr, content_type, buf.buf);
                end_headers(hdr);
 
-
-               if (determine_protocol_version_server() != protocol_v2) {
-                       packet_write_fmt(1, "# service=git-%s\n", svc->name);
-                       packet_flush(1);
-               }
-
                argv[0] = svc->name;
                run_service(argv, 0);
 

t/t5555-http-smart-common.sh +120 -0

index b1cfe8b7dba..32431266eb9 100755
@@ -159,4 +159,124 @@ test_expect_success 'git receive-pack --advertise-refs: v2' '
        test_cmp actual expect
 '
 
+test_expect_success 'git upload-pack --advertise-refs --show-service: v0' '
+       # With no specified protocol
+       cat >expect <<-EOF &&
+       # service=git-upload-pack
+       0000
+       $(git rev-parse HEAD) HEAD
+       $(git rev-parse HEAD) $(git symbolic-ref HEAD)
+       0000
+       EOF
+
+       git upload-pack --advertise-refs --show-service . >out 2>err &&
+       test-tool pkt-line unpack <out >actual &&
+       test_must_be_empty err &&
+       test_cmp actual expect &&
+
+       # With explicit v0
+       GIT_PROTOCOL=version=0 \
+       git upload-pack --advertise-refs --show-service . >out 2>err &&
+       test-tool pkt-line unpack <out >actual 2>err &&
+       test_must_be_empty err &&
+       test_cmp actual expect
+
+'
+
+test_expect_success 'git receive-pack --advertise-refs --show-service: v0' '
+       # With no specified protocol
+       cat >expect <<-EOF &&
+       # service=git-receive-pack
+       0000
+       $(git rev-parse HEAD) $(git symbolic-ref HEAD)
+       0000
+       EOF
+
+       git receive-pack --advertise-refs --show-service . >out 2>err &&
+       test-tool pkt-line unpack <out >actual &&
+       test_must_be_empty err &&
+       test_cmp actual expect &&
+
+       # With explicit v0
+       GIT_PROTOCOL=version=0 \
+       git receive-pack --advertise-refs --show-service . >out 2>err &&
+       test-tool pkt-line unpack <out >actual 2>err &&
+       test_must_be_empty err &&
+       test_cmp actual expect
+
+'
+
+test_expect_success 'git upload-pack --advertise-refs --show-service: v1' '
+       # With no specified protocol
+       cat >expect <<-EOF &&
+       # service=git-upload-pack
+       0000
+       version 1
+       $(git rev-parse HEAD) HEAD
+       $(git rev-parse HEAD) $(git symbolic-ref HEAD)
+       0000
+       EOF
+
+       GIT_PROTOCOL=version=1 \
+       git upload-pack --advertise-refs --show-service . >out &&
+
+       test-tool pkt-line unpack <out >actual 2>err &&
+       test_must_be_empty err &&
+       test_cmp actual expect
+'
+
+test_expect_success 'git receive-pack --advertise-refs --show-service: v1' '
+       # With no specified protocol
+       cat >expect <<-EOF &&
+       # service=git-receive-pack
+       0000
+       version 1
+       $(git rev-parse HEAD) $(git symbolic-ref HEAD)
+       0000
+       EOF
+
+       GIT_PROTOCOL=version=1 \
+       git receive-pack --advertise-refs --show-service . >out &&
+
+       test-tool pkt-line unpack <out >actual 2>err &&
+       test_must_be_empty err &&
+       test_cmp actual expect
+'
+
+test_expect_success 'git upload-pack --advertise-refs --show-service: v2' '
+       cat >expect <<-EOF &&
+       version 2
+       agent=FAKE
+       ls-refs=unborn
+       fetch=shallow wait-for-done
+       server-option
+       object-format=$(test_oid algo)
+       object-info
+       0000
+       EOF
+
+       GIT_PROTOCOL=version=2 \
+       GIT_USER_AGENT=FAKE \
+       git upload-pack --advertise-refs --show-service . >out 2>err &&
+
+       test-tool pkt-line unpack <out >actual &&
+       test_must_be_empty err &&
+       test_cmp actual expect
+'
+
+test_expect_success 'git receive-pack --advertise-refs --show-service: v2' '
+       # There is no v2 yet for receive-pack, implicit v0
+       cat >expect <<-EOF &&
+       $(git rev-parse HEAD) $(git symbolic-ref HEAD)
+       0000
+       EOF
+
+       GIT_PROTOCOL=version=2 \
+       git receive-pack --advertise-refs --show-service . >out 2>err &&
+
+       test-tool pkt-line unpack <out >actual &&
+       test_must_be_empty err &&
+       test_cmp actual expect
+'
+
 test_done

base-commit: 7580f92ffa970b9484ac214f7b53cec5e26ca4bc --
gitgitgadget