mbox series

[v5,0/6] support remote archive via stateless transport

Message ID cover.1705411391.git.zhiyou.jx@alibaba-inc.com (mailing list archive)
Headers show
Series support remote archive via stateless transport | expand

Message

Jiang Xin Jan. 16, 2024, 1:39 p.m. UTC 
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.

# Changes since v4

1. Change commit messages and order of commits.
2. Split the last commit of v4 into three seperate commits.


# range-diff v4...v5

1:  da80391037 ! 1:  f3fef46c05 transport-helper: no connection restriction in connect_helper
    @@ Commit message
         was for transport that supports the ".connect" method. The
         "connect_helper()" function protected itself from getting called for a
         transport without the method before calling process_connect_service(),
    -    which did not work with such a transport.
    +    which only worked with the ".connect" method.
     
         Later, commit edc9caf7 (transport-helper: introduce stateless-connect,
         2018-03-15) added a way for a transport without the ".connect" method
    -    to establish a "stateless" connection in protocol-v2, which
    -    process_connect_service() was taught to handle the "stateless"
    -    connection, making the old safety valve in its caller that insisted
    -    that ".connect" method must be defined too strict, and forgot to loosen
    -    it.
    +    to establish a "stateless" connection in protocol-v2, where
    +    process_connect_service() was taught to handle the ".stateless_connect"
    +    method, making the old protection too strict. But commit edc9caf7 forgot
    +    to adjust this protection accordingly.
     
         Remove the restriction in the "connect_helper()" function and give the
         function "process_connect_service()" the opportunity to establish a
    @@ Commit message
         protocol.
     
         Helped-by: Junio C Hamano <gitster@pobox.com>
    +    Helped-by: Linus Arver <linusa@google.com>
         Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
     
      ## transport-helper.c ##
-:  ---------- > 2:  6be331b22d remote-curl: supports git-upload-archive service
-:  ---------- > 3:  aabc8e1a2a transport-helper: protocol-v2 supports upload-archive
4:  a21a80dae9 ! 4:  fdab4abb43 archive: support remote archive from stateless transport
    @@ Metadata
     Author: Jiang Xin <zhiyou.jx@alibaba-inc.com>
     
      ## Commit message ##
    -    archive: support remote archive from stateless transport
    +    http-backend: new rpc-service for git-upload-archive
     
    -    Even though we can establish a stateless connection, we still cannot
    -    archive the remote repository using a stateless HTTP protocol. Try the
    -    following steps to make it work.
    +    Add new rpc-service "upload-archive" in http-backend to add server side
    +    support for remote archive over HTTP/HTTPS protocols.
     
    -     1. Add support for "git-upload-archive" service in "http-backend".
    -
    -     2. Use the URL ".../info/refs?service=git-upload-pack" to detect the
    -        protocol version, instead of use the "git-upload-archive" service.
    -
    -     3. "git-archive" does not expect to see protocol version and
    -        capabilities when connecting to remote-helper, so do not send them
    -        in "remote-curl.c" for the "git-upload-archive" service.
    +    Also add new test cases in t5003. In the test case "archive remote http
    +    repository", git-archive exits with a non-0 exit code even though we
    +    create the archive correctly. It will be fixed in a later commit.
     
         Helped-by: Eric Sunshine <sunshine@sunshineco.com>
         Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
 		if (ret)

    ... (omitted) ...

3:  870dc5fd21 ! 5:  6ac0c8e105 transport-helper: call do_take_over() in connect_helper
    @@ Commit message
         After successfully connecting to the smart transport by calling
         process_connect_service() in connect_helper(), run do_take_over() to
         replace the old vtable with a new one which has methods ready for the
    -    smart transport connection.
    +    smart transport connection. This will fix the exit code of git-archive
    +    in test case "archive remote http repository" of t5003.
     
         The connect_helper() function is used as the connect method of the
         vtable in "transport-helper.c", and it is called by transport_connect()
    @@ Commit message
         transport_connect() so far is in "builtin/archive.c". Without running
         do_take_over(), it may fail to call transport_disconnect() in
         run_remote_archiver() of "builtin/archive.c". This is because for a
    -    stateless connection or a service like "git-upload-pack-archive", the
    +    stateless connection and a service like "git-upload-archive", the
         remote helper may receive a SIGPIPE signal and exit early. To have a
         graceful disconnect method by calling do_take_over() will solve this
         issue.
     
    -    The subsequent commit will introduce remote archive over a stateless-rpc
    -    connection.
    -
    +    Helped-by: Linus Arver <linusa@google.com>
         Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
     
    + ## t/t5003-archive-zip.sh ##
    +@@ t/t5003-archive-zip.sh: test_expect_success 'remote archive does not work with protocol v1' '
    + '
    + 
    + test_expect_success 'archive remote http repository' '
    +-	test_must_fail git archive --remote="$HTTPD_URL/auth/smart/bare.git" \
    ++	git archive --remote="$HTTPD_URL/auth/smart/bare.git" \
    + 		--output=remote-http.zip HEAD &&
    + 	test_cmp_bin d.zip remote-http.zip
    + '
    +
      ## transport-helper.c ##
     @@ transport-helper.c: static int connect_helper(struct transport *transport, const char *name,
      
2:  2f7060f7c5 = 6:  423a89c593 transport-helper: call do_take_over() in process_connect

Jiang Xin (6):
  transport-helper: no connection restriction in connect_helper
  remote-curl: supports git-upload-archive service
  transport-helper: protocol-v2 supports upload-archive
  http-backend: new rpc-service for git-upload-archive
  transport-helper: call do_take_over() in connect_helper
  transport-helper: call do_take_over() in process_connect

 http-backend.c         | 13 ++++++++++---
 remote-curl.c          | 14 +++++++++++---
 t/t5003-archive-zip.sh | 34 ++++++++++++++++++++++++++++++++++
 transport-helper.c     | 29 +++++++++++++----------------
 4 files changed, 68 insertions(+), 22 deletions(-)

Comments

Linus Arver Jan. 20, 2024, 8:43 p.m. UTC | #1 
Jiang Xin <worldhello.net@gmail.com> writes:

This v5 looks good code-wise. I've made small suggestions to make the
commit messages better, but they are just nits and you are free to
ignore them.

If you choose to reroll one more time, one additional thing you could do
is to use the word "protocol_v2" in all commit messages because that's
how that term looks like in the code (unless the "protocol-v2" string is
already the standard term used elsewhere).

Thanks.
Jiang Xin Jan. 21, 2024, 4:09 a.m. UTC | #2 
On Sun, Jan 21, 2024 at 4:43 AM Linus Arver <linusa@google.com> wrote:
>
> Jiang Xin <worldhello.net@gmail.com> writes:
>
> This v5 looks good code-wise. I've made small suggestions to make the
> commit messages better, but they are just nits and you are free to
> ignore them.

Thanks for helping me refine commit messages. I will update them based
on your suggestions in next reroll.

> If you choose to reroll one more time, one additional thing you could do
> is to use the word "protocol_v2" in all commit messages because that's
> how that term looks like in the code (unless the "protocol-v2" string is
> already the standard term used elsewhere).

Will s/protocol_v2/protocol v2/