remote-ext: simplify git pkt-line generation

We format a pkt-line into a heap buffer, which requires
manual computation of the required size, and uses some bare
sprintf calls. We could use a strbuf instead, which would
take care of the computation for us. But it's even easier
still to use packet_write(). Besides handling the formatting
and writing for us, it fixes two things:

1. Our manual max-size check used 0xFFFF, while technically
LARGE_PACKET_MAX is slightly smaller than this.

2. Our packet will now be output as part of
GIT_TRACE_PACKET debugging.

Unfortunately packet_write() does not let us build up the
buffer progressively, so we do have to repeat ourselves a
little depending on the "vhost" setting, but the end result
is still far more readable than the original.

Since there were no tests covering this feature at all,
we'll add a few into t5802.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index 3b8c22cc75ed04eaf450f9d5a1e9e71b914339eb..e3cd25d580b0f1ee14b4a5037ceae8dc20b31f30 100644 (file)
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "transport.h"
 #include "run-command.h"
+#include "pkt-line.h"
 
 /*
  * URL syntax:
@@ -142,36 +143,11 @@ static const char **parse_argv(const char *arg, const char *service)
 static void send_git_request(int stdin_fd, const char *serv, const char *repo,
        const char *vhost)
 {
-       size_t bufferspace;
-       size_t wpos = 0;
-       char *buffer;
-
-       /*
-        * Request needs 12 bytes extra if there is vhost (xxxx \0host=\0) and
-        * 6 bytes extra (xxxx \0) if there is no vhost.
-        */
-       if (vhost)
-               bufferspace = strlen(serv) + strlen(repo) + strlen(vhost) + 12;
+       if (!vhost)
+               packet_write(stdin_fd, "%s %s%c", serv, repo, 0);
        else
-               bufferspace = strlen(serv) + strlen(repo) + 6;
-
-       if (bufferspace > 0xFFFF)
-               die("Request too large to send");
-       buffer = xmalloc(bufferspace);
-
-       /* Make the packet. */
-       wpos = sprintf(buffer, "%04x%s %s%c", (unsigned)bufferspace,
-               serv, repo, 0);
-
-       /* Add vhost if any. */
-       if (vhost)
-               sprintf(buffer + wpos, "host=%s%c", vhost, 0);
-
-       /* Send the request */
-       if (write_in_full(stdin_fd, buffer, bufferspace) < 0)
-               die_errno("Failed to send request");
-
-       free(buffer);
+               packet_write(stdin_fd, "%s %s%chost=%s%c", serv, repo, 0,
+                            vhost, 0);
 }
 
 static int run_child(const char *arg, const char *service)

diff --git a/t/t5802-connect-helper.sh b/t/t5802-connect-helper.sh
index 878faf2b6318412df756f5b8cd2c00410f476e12..b7a7f9d5886f1e2f3fc2d65bfcf1785b890638ac 100755 (executable)
--- a/t/t5802-connect-helper.sh
+++ b/t/t5802-connect-helper.sh
@@ -69,4 +69,32 @@ test_expect_success 'update backfilled tag without primary transfer' '
        test_cmp expect actual
 '
 
+
+test_expect_success 'set up fake git-daemon' '
+       mkdir remote &&
+       git init --bare remote/one.git &&
+       mkdir remote/host &&
+       git init --bare remote/host/two.git &&
+       write_script fake-daemon <<-\EOF &&
+       git daemon --inetd \
+               --informative-errors \
+               --export-all \
+               --base-path="$TRASH_DIRECTORY/remote" \
+               --interpolated-path="$TRASH_DIRECTORY/remote/%H%D" \
+               "$TRASH_DIRECTORY/remote"
+       EOF
+       export TRASH_DIRECTORY &&
+       PATH=$TRASH_DIRECTORY:$PATH
+'
+
+test_expect_success 'ext command can connect to git daemon (no vhost)' '
+       rm -rf dst &&
+       git clone "ext::fake-daemon %G/one.git" dst
+'
+
+test_expect_success 'ext command can connect to git daemon (vhost)' '
+       rm -rf dst &&
+       git clone "ext::fake-daemon %G/two.git %Vhost" dst
+'
+
 test_done