* [PATCH 1/2] transport-helper: no connection restriction in connect_helper
@ 2023-09-19 6:41 Jiang Xin
2023-09-19 6:41 ` [PATCH 2/2] archive: support remote archive from stateless transport Jiang Xin
2023-09-19 17:18 ` [PATCH 1/2] transport-helper: no connection restriction in connect_helper Junio C Hamano
0 siblings, 2 replies; 4+ messages in thread
From: Jiang Xin @ 2023-09-19 6:41 UTC (permalink / raw)
To: Git List, Junio C Hamano, Ilari Liusvaara; +Cc: Jiang Xin
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
For protocol-v2, "stateless-connection" can be used to establish a
stateless connection between the client and the server, but trying to
establish http connection by calling "transport->vtable->connect" will
fail. This restriction was first introduced in commit b236752a87
(Support remote archive from all smart transports, 2009-12-09) by
adding a limitation in the "connect_helper()" function.
Remove the restriction in the "connect_helper()" function and use the
logic in the "process_connect_service()" function to check the protocol
version and service name. By this way, we can make a connection and do
something useful. E.g., in a later commit, implements remote archive
for a repository over HTTP protocol.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
transport-helper.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 49811ef176..2e127d24a5 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -662,8 +662,6 @@ static int connect_helper(struct transport *transport, const char *name,
/* Get_helper so connect is inited. */
get_helper(transport);
- if (!data->connect)
- die(_("operation not supported by protocol"));
if (!process_connect_service(transport, name, exec))
die(_("can't connect to subservice %s"), name);
--
2.40.1.49.g40e13c3520.dirty
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] archive: support remote archive from stateless transport
2023-09-19 6:41 [PATCH 1/2] transport-helper: no connection restriction in connect_helper Jiang Xin
@ 2023-09-19 6:41 ` Jiang Xin
2023-09-19 17:18 ` [PATCH 1/2] transport-helper: no connection restriction in connect_helper Junio C Hamano
1 sibling, 0 replies; 4+ messages in thread
From: Jiang Xin @ 2023-09-19 6:41 UTC (permalink / raw)
To: Git List, Junio C Hamano, Ilari Liusvaara; +Cc: Jiang Xin
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
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.
1. Add support for "git-upload-archive" service in "http-backend".
2. Unable to access the URL ".../info/refs?service=git-upload-archive"
to detect the protocol version, use the "git-upload-pack" service
instead.
3. "git-archive" does not resolve the protocol version and capabilities
when connecting to remote-helper, so the remote-helper should not
send them.
4. "git-archive" may not be able to disconnect the stateless
connection. Run "do_take_over()" to take_over the transfer for
a graceful disconnect function.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
http-backend.c | 15 +++++++++++++--
remote-curl.c | 14 +++++++++++---
t/t5003-archive-zip.sh | 30 ++++++++++++++++++++++++++++++
transport-helper.c | 5 ++++-
4 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/http-backend.c b/http-backend.c
index ff07b87e64..ed3bed965a 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -38,6 +38,7 @@ struct rpc_service {
static struct rpc_service rpc_service[] = {
{ "upload-pack", "uploadpack", 1, 1 },
{ "receive-pack", "receivepack", 0, -1 },
+ { "upload-archive", "uploadarchive", 0, -1 },
};
static struct string_list *get_parameters(void)
@@ -639,10 +640,19 @@ static void check_content_type(struct strbuf *hdr, const char *accepted_type)
static void service_rpc(struct strbuf *hdr, char *service_name)
{
- const char *argv[] = {NULL, "--stateless-rpc", ".", NULL};
+ const char *argv[4];
struct rpc_service *svc = select_service(hdr, service_name);
struct strbuf buf = STRBUF_INIT;
+ if (!strcmp(service_name, "git-upload-archive")) {
+ argv[1] = ".";
+ argv[2] = NULL;
+ } else {
+ argv[1] = "--stateless-rpc";
+ argv[2] = ".";
+ argv[3] = NULL;
+ }
+
strbuf_reset(&buf);
strbuf_addf(&buf, "application/x-git-%s-request", svc->name);
check_content_type(hdr, buf.buf);
@@ -723,7 +733,8 @@ static struct service_cmd {
{"GET", "/objects/pack/pack-[0-9a-f]{64}\\.idx$", get_idx_file},
{"POST", "/git-upload-pack$", service_rpc},
- {"POST", "/git-receive-pack$", service_rpc}
+ {"POST", "/git-receive-pack$", service_rpc},
+ {"POST", "/git-upload-archive$", service_rpc}
};
static int bad_request(struct strbuf *hdr, const struct service_cmd *c)
diff --git a/remote-curl.c b/remote-curl.c
index ef05752ca5..ce6cb8ac05 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -1447,8 +1447,14 @@ static int stateless_connect(const char *service_name)
* establish a stateless connection, otherwise we need to tell the
* client to fallback to using other transport helper functions to
* complete their request.
+ *
+ * The "git-upload-archive" service is a read-only operation. Fallback
+ * to use "git-upload-pack" service to discover protocol version.
*/
- discover = discover_refs(service_name, 0);
+ if (!strcmp(service_name, "git-upload-archive"))
+ discover = discover_refs("git-upload-pack", 0);
+ else
+ discover = discover_refs(service_name, 0);
if (discover->version != protocol_v2) {
printf("fallback\n");
fflush(stdout);
@@ -1486,9 +1492,11 @@ static int stateless_connect(const char *service_name)
/*
* Dump the capability listing that we got from the server earlier
- * during the info/refs request.
+ * during the info/refs request. This does not work with the
+ * "git-upload-archive" service.
*/
- write_or_die(rpc.in, discover->buf, discover->len);
+ if (strcmp(service_name, "git-upload-archive"))
+ write_or_die(rpc.in, discover->buf, discover->len);
/* Until we see EOF keep sending POSTs */
while (1) {
diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh
index fc499cdff0..80123c1e06 100755
--- a/t/t5003-archive-zip.sh
+++ b/t/t5003-archive-zip.sh
@@ -239,4 +239,34 @@ check_zip with_untracked2
check_added with_untracked2 untracked one/untracked
check_added with_untracked2 untracked two/untracked
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+test_expect_success "setup for HTTP protocol" '
+ cp -R bare.git "$HTTPD_DOCUMENT_ROOT_PATH/bare.git" &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/bare.git" \
+ config http.uploadpack true &&
+ set_askpass user@host pass@host
+'
+
+setup_askpass_helper
+
+test_expect_success 'remote archive does not work with protocol v1' '
+ test_when_finished "rm -f d5.zip" &&
+ test_must_fail git -c protocol.version=1 archive \
+ --remote="$HTTPD_URL/auth/smart/bare.git" \
+ --output=d5.zip HEAD >actual 2>&1 &&
+ cat >expect <<-EOF &&
+ fatal: can${SQ}t connect to subservice git-upload-archive
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'archive remote http repository' '
+ test_when_finished "rm -f d5.zip" &&
+ git archive --remote="$HTTPD_URL/auth/smart/bare.git" \
+ --output=d5.zip HEAD &&
+ test_cmp_bin d.zip d5.zip
+'
+
test_done
diff --git a/transport-helper.c b/transport-helper.c
index 2e127d24a5..91381be622 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -628,7 +628,8 @@ static int process_connect_service(struct transport *transport,
ret = run_connect(transport, &cmdbuf);
} else if (data->stateless_connect &&
(get_protocol_version_config() == protocol_v2) &&
- !strcmp("git-upload-pack", name)) {
+ (!strcmp("git-upload-pack", name) ||
+ !strcmp("git-upload-archive", name))) {
strbuf_addf(&cmdbuf, "stateless-connect %s\n", name);
ret = run_connect(transport, &cmdbuf);
if (ret)
@@ -668,6 +669,8 @@ static int connect_helper(struct transport *transport, const char *name,
fd[0] = data->helper->out;
fd[1] = data->helper->in;
+
+ do_take_over(transport);
return 0;
}
--
2.40.1.49.g40e13c3520.dirty
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] transport-helper: no connection restriction in connect_helper
2023-09-19 6:41 [PATCH 1/2] transport-helper: no connection restriction in connect_helper Jiang Xin
2023-09-19 6:41 ` [PATCH 2/2] archive: support remote archive from stateless transport Jiang Xin
@ 2023-09-19 17:18 ` Junio C Hamano
2023-09-20 0:20 ` Jiang Xin
1 sibling, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2023-09-19 17:18 UTC (permalink / raw)
To: Jiang Xin; +Cc: Git List, Ilari Liusvaara, Jiang Xin
Jiang Xin <worldhello.net@gmail.com> writes:
> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> For protocol-v2, "stateless-connection" can be used to establish a
> stateless connection between the client and the server, but trying to
> establish http connection by calling "transport->vtable->connect" will
> fail. This restriction was first introduced in commit b236752a87
> (Support remote archive from all smart transports, 2009-12-09) by
> adding a limitation in the "connect_helper()" function.
The above description may not be technically wrong per-se, but I
found it confusing. The ".connect method must be defined" you are
removing was added back when there was no "stateless" variant of the
connection initiation. Many codepaths added by that patch did "if
.connect is there, call it, but otherwise die()" and I think the
code you were removing was added as a safety valve, not a limitation
or restriction. Later, process_connect_service() learned to handle
the .stateless_connect bit as a fallback for transports without
.connect method defined, and the commit added that feature, edc9caf7
(transport-helper: introduce stateless-connect, 2018-03-15), forgot
that the caller did not allow this fallback.
When b236752a (Support remote archive from all smart
transports, 2009-12-09) added "remote archive" support for
"smart transports", it was for transport that supports the
.connect method. connect_helper() function protected itself
from getting called for a transport without the method
before calling process_connect_service(), which did not work
wuth such a transport.
Later, 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, 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.
or something along that line would have been easire to follow, at
least to me.
> Remove the restriction in the "connect_helper()" function and use the
> logic in the "process_connect_service()" function to check the protocol
> version and service name. By this way, we can make a connection and do
> something useful. E.g., in a later commit, implements remote archive
> for a repository over HTTP protocol.
OK.
b236752a87 was to allow "remote archive from all smart transports",
but unfortunately HTTP was not among "smart transports". This
series is to update smart HTTP transport (aka "stateless") to also
support it? Interesting.
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
> transport-helper.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/transport-helper.c b/transport-helper.c
> index 49811ef176..2e127d24a5 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -662,8 +662,6 @@ static int connect_helper(struct transport *transport, const char *name,
>
> /* Get_helper so connect is inited. */
> get_helper(transport);
> - if (!data->connect)
> - die(_("operation not supported by protocol"));
>
> if (!process_connect_service(transport, name, exec))
> die(_("can't connect to subservice %s"), name);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] transport-helper: no connection restriction in connect_helper
2023-09-19 17:18 ` [PATCH 1/2] transport-helper: no connection restriction in connect_helper Junio C Hamano
@ 2023-09-20 0:20 ` Jiang Xin
0 siblings, 0 replies; 4+ messages in thread
From: Jiang Xin @ 2023-09-20 0:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Ilari Liusvaara, Brandon Williams
On Wed, Sep 20, 2023 at 1:19 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jiang Xin <worldhello.net@gmail.com> writes:
>
> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> >
> > For protocol-v2, "stateless-connection" can be used to establish a
> > stateless connection between the client and the server, but trying to
> > establish http connection by calling "transport->vtable->connect" will
> > fail. This restriction was first introduced in commit b236752a87
> > (Support remote archive from all smart transports, 2009-12-09) by
> > adding a limitation in the "connect_helper()" function.
>
> The above description may not be technically wrong per-se, but I
> found it confusing. The ".connect method must be defined" you are
> removing was added back when there was no "stateless" variant of the
> connection initiation. Many codepaths added by that patch did "if
> .connect is there, call it, but otherwise die()" and I think the
> code you were removing was added as a safety valve, not a limitation
> or restriction. Later, process_connect_service() learned to handle
> the .stateless_connect bit as a fallback for transports without
> .connect method defined, and the commit added that feature, edc9caf7
> (transport-helper: introduce stateless-connect, 2018-03-15), forgot
> that the caller did not allow this fallback.
>
> When b236752a (Support remote archive from all smart
> transports, 2009-12-09) added "remote archive" support for
> "smart transports", it was for transport that supports the
> .connect method. connect_helper() function protected itself
> from getting called for a transport without the method
> before calling process_connect_service(), which did not work
> wuth such a transport.
>
> Later, 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, 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.
>
> or something along that line would have been easire to follow, at
> least to me.
>
These explanations are very clear and helpful, thank you.
--
Jiang Xin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-09-20 0:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-19 6:41 [PATCH 1/2] transport-helper: no connection restriction in connect_helper Jiang Xin
2023-09-19 6:41 ` [PATCH 2/2] archive: support remote archive from stateless transport Jiang Xin
2023-09-19 17:18 ` [PATCH 1/2] transport-helper: no connection restriction in connect_helper Junio C Hamano
2023-09-20 0:20 ` Jiang Xin
Code repositories for project(s) associated with this public inbox
https://80x24.org/pub/scm/git/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).