[RFC PATCH v2 00/16] Return of smart HTTP

[RFC PATCH v2 00/16] Return of smart HTTP

[Shawn O. Pearce]

The series has gotten a lot larger since my last posting, but I have
what appears to be a fully working client *AND* server implementation
for both fetch and push, and the client should be supporting deeping
shallow repositories over the smart variant of HTTP.

I've dropped the documentation patch from the series for now as
I have quite a few edits queued up from folks in the last round
(thanks for those!) that I have not yet applied.  So there is no
point in sending that particular patch again.

This series is still lacking:

* The HTTP protocol documentation
* Manual page for git-http-backend
* Tests for the smart http transport code (existing tests pass)
* ack from the CC'd maintainers  :-)

For those who are really curious, this applies to 'master' and I'm
running my local server under Apache with this as my configuration:

 -- httpd.conf <8--
 LoadModule cgi_module /usr/libexec/apache2/mod_cgi.so
 LoadModule alias_module /usr/libexec/apache2/mod_alias.so
 LoadModule env_module /usr/libexec/apache2/mod_env.so

 Listen 127.0.0.1:8079

 ServerRoot /home/spearce/test
 ErrorLog error_log
 PidFile httpd.pid
 LockFile httpd.lock

 <IfModule mime_module>
 TypesConfig /etc/mime.types
 </IfModule>

 DocumentRoot /home/spearce/test/www

 SetEnv GIT_EXEC_PATH /home/spearce/cgit
 ScriptAlias /git/ /home/spearce/cgit/git-http-backend/
 --<8--

 $ httpd -f httpd.conf
 $ git clone --bare foo.git /home/spearce/test/www/test.git

 URL is ... http://localhost:8070/git/test.git


Shawn O. Pearce (16):
  pkt-line: Add strbuf based functions
  pkt-line: Make packet_read_line easier to debug
  fetch-pack: Use a strbuf to compose the want list
  Move "get_ack()" back to fetch-pack
  Add multi_ack_2 capability to fetch-pack/upload-pack
  remote-curl: Refactor walker initialization
  remote-helpers: Fetch more than one ref in a batch
  remote-helpers: Support custom transport options
  Move WebDAV HTTP push under remote-curl
  Git-aware CGI to provide dumb HTTP transport
  Add one shot RPC options to upload-pack, receive-pack
  Smart fetch and push over HTTP: server side
  Discover refs via smart HTTP server when available
  Smart push over HTTP: client side
  Smart fetch over HTTP: client side
  Smart HTTP fetch: gzip requests

 .gitignore                           |    1 +
 Documentation/config.txt             |    8 +
 Documentation/git-remote-helpers.txt |   79 ++++
 Makefile                             |    1 +
 builtin-fetch-pack.c                 |  210 ++++++++--
 builtin-receive-pack.c               |   26 +-
 builtin-send-pack.c                  |  116 +++++-
 cache.h                              |    1 -
 commit.c                             |   10 +-
 commit.h                             |    2 +-
 connect.c                            |   21 -
 fetch-pack.h                         |    3 +-
 http-backend.c                       |  451 +++++++++++++++++++++
 http-push.c                          |   43 ++-
 pkt-line.c                           |   83 ++++-
 pkt-line.h                           |    4 +
 remote-curl.c                        |  729 +++++++++++++++++++++++++++++++--
 send-pack.h                          |    3 +-
 sideband.c                           |   11 +-
 transport-helper.c                   |  264 ++++++++++++-
 transport.c                          |   32 +--
 upload-pack.c                        |   71 +++-
 22 files changed, 1955 insertions(+), 214 deletions(-)
 create mode 100644 http-backend.c

[RFC PATCH v2 01/16] pkt-line: Add strbuf based functions

[Shawn O. Pearce]

These routines help to work with pkt-line values inside of a strbuf,
permitting simple formatting of buffered network messages.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 pkt-line.c |   81 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
 pkt-line.h |    4 +++
 2 files changed, 73 insertions(+), 12 deletions(-)

diff --git a/pkt-line.c b/pkt-line.c
index b691abe..2333d96 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -42,17 +42,19 @@ void packet_flush(int fd)
 	safe_write(fd, "0000", 4);
 }
 
+void packet_buf_flush(struct strbuf *buf)
+{
+	strbuf_add(buf, "0000", 4);
+}
+
 #define hex(a) (hexchar[(a) & 15])
-void packet_write(int fd, const char *fmt, ...)
+static char buffer[1000];
+static unsigned format_packet(const char *fmt, va_list args)
 {
-	static char buffer[1000];
 	static char hexchar[] = "0123456789abcdef";
-	va_list args;
 	unsigned n;
 
-	va_start(args, fmt);
 	n = vsnprintf(buffer + 4, sizeof(buffer) - 4, fmt, args);
-	va_end(args);
 	if (n >= sizeof(buffer)-4)
 		die("protocol error: impossibly long line");
 	n += 4;
@@ -60,9 +62,31 @@ void packet_write(int fd, const char *fmt, ...)
 	buffer[1] = hex(n >> 8);
 	buffer[2] = hex(n >> 4);
 	buffer[3] = hex(n);
+	return n;
+}
+
+void packet_write(int fd, const char *fmt, ...)
+{
+	va_list args;
+	unsigned n;
+
+	va_start(args, fmt);
+	n = format_packet(fmt, args);
+	va_end(args);
 	safe_write(fd, buffer, n);
 }
 
+void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
+{
+	va_list args;
+	unsigned n;
+
+	va_start(args, fmt);
+	n = format_packet(fmt, args);
+	va_end(args);
+	strbuf_add(buf, buffer, n);
+}
+
 static void safe_read(int fd, void *buffer, unsigned size)
 {
 	ssize_t ret = read_in_full(fd, buffer, size);
@@ -72,15 +96,11 @@ static void safe_read(int fd, void *buffer, unsigned size)
 		die("The remote end hung up unexpectedly");
 }
 
-int packet_read_line(int fd, char *buffer, unsigned size)
+static int packet_length(unsigned *ret_len, const char *linelen)
 {
 	int n;
-	unsigned len;
-	char linelen[4];
-
-	safe_read(fd, linelen, 4);
+	unsigned len = 0;
 
-	len = 0;
 	for (n = 0; n < 4; n++) {
 		unsigned char c = linelen[n];
 		len <<= 4;
@@ -96,8 +116,20 @@ int packet_read_line(int fd, char *buffer, unsigned size)
 			len += c - 'A' + 10;
 			continue;
 		}
-		die("protocol error: bad line length character");
+		return -1;
 	}
+	*ret_len = len;
+	return 0;
+}
+
+int packet_read_line(int fd, char *buffer, unsigned size)
+{
+	unsigned len;
+	char linelen[4];
+
+	safe_read(fd, linelen, 4);
+	if (packet_length(&len, linelen))
+		die("protocol error: bad line length character");
 	if (!len)
 		return 0;
 	len -= 4;
@@ -107,3 +139,28 @@ int packet_read_line(int fd, char *buffer, unsigned size)
 	buffer[len] = 0;
 	return len;
 }
+
+int packet_get_line(struct strbuf *out,
+	char **src_buf, size_t *src_len)
+{
+	unsigned len;
+
+	if (*src_len < 4 || packet_length(&len, *src_buf))
+		return -1;
+	if (!len) {
+		*src_buf += 4;
+		*src_len -= 4;
+		return 0;
+	}
+	if (*src_len < len)
+		return -2;
+
+	*src_buf += 4;
+	*src_len -= 4;
+	len -= 4;
+
+	strbuf_add(out, *src_buf, len);
+	*src_buf += len;
+	*src_len -= len;
+	return len;
+}
diff --git a/pkt-line.h b/pkt-line.h
index 9df653f..1e5dcfe 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -2,14 +2,18 @@
 #define PKTLINE_H
 
 #include "git-compat-util.h"
+#include "strbuf.h"
 
 /*
  * Silly packetized line writing interface
  */
 void packet_flush(int fd);
 void packet_write(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+void packet_buf_flush(struct strbuf *buf);
+void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
 
 int packet_read_line(int fd, char *buffer, unsigned size);
+int packet_get_line(struct strbuf *out, char **src_buf, size_t *src_len);
 ssize_t safe_write(int, const void *, ssize_t);
 
 #endif
-- 
1.6.5.52.g0ff2e

Re: [RFC PATCH v2 01/16] pkt-line: Add strbuf based functions

[Johannes Sixt]

Shawn O. Pearce schrieb:
> -int packet_read_line(int fd, char *buffer, unsigned size)
> +static int packet_length(unsigned *ret_len, const char *linelen)
>  {
>  	int n;
> -	unsigned len;
> -	char linelen[4];
> -
> -	safe_read(fd, linelen, 4);
> +	unsigned len = 0;
>  
> -	len = 0;
>  	for (n = 0; n < 4; n++) {
>  		unsigned char c = linelen[n];
>  		len <<= 4;
> @@ -96,8 +116,20 @@ int packet_read_line(int fd, char *buffer, unsigned size)
>  			len += c - 'A' + 10;
>  			continue;
>  		}
> -		die("protocol error: bad line length character");
> +		return -1;
>  	}
> +	*ret_len = len;
> +	return 0;
> +}

len can be signed: Valid lengths fit into a signed int. Then you can
'return len;' on success and 'return -1;' on failure and don't need return
the result by reference. packet_read_line() ultimately converts it to int
anyway:

> +int packet_read_line(int fd, char *buffer, unsigned size)
> +{
> +	unsigned len;
> +	char linelen[4];
> +
> +	safe_read(fd, linelen, 4);
> +	if (packet_length(&len, linelen))
> +		die("protocol error: bad line length character");
>  	if (!len)
>  		return 0;
>  	len -= 4;
> @@ -107,3 +139,28 @@ int packet_read_line(int fd, char *buffer, unsigned size)
>  	buffer[len] = 0;
>  	return len;
>  }

-- Hannes

Re: [RFC PATCH v2 01/16] pkt-line: Add strbuf based functions

[Shawn O. Pearce]

Johannes Sixt <j.sixt@viscovery.net> wrote:
> Shawn O. Pearce schrieb:
> > -int packet_read_line(int fd, char *buffer, unsigned size)
> > +static int packet_length(unsigned *ret_len, const char *linelen)
...
> > +	*ret_len = len;
> > +	return 0;
> > +}
> 
> len can be signed: Valid lengths fit into a signed int. Then you can
> 'return len;' on success and 'return -1;' on failure and don't need return
> the result by reference. packet_read_line() ultimately converts it to int
> anyway:

Great catch, thanks.  This is actually from a prior version of code
where I was exposing this function to callers... but even then the
method could have just returned int with the value because as you
point out, all valid lengths fit in int and must be >= 0.
 
-- 
Shawn.

[RFC PATCH v2 02/16] pkt-line: Make packet_read_line easier to debug

[Shawn O. Pearce]

When there is an error parsing the 4 byte length component we now
NUL terminate the string and display it as part of the die message,
this may hint as to what data was misunderstood by the application.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 pkt-line.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/pkt-line.c b/pkt-line.c
index 2333d96..350f173 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -125,11 +125,13 @@ static int packet_length(unsigned *ret_len, const char *linelen)
 int packet_read_line(int fd, char *buffer, unsigned size)
 {
 	unsigned len;
-	char linelen[4];
+	char linelen[5];
 
 	safe_read(fd, linelen, 4);
-	if (packet_length(&len, linelen))
-		die("protocol error: bad line length character");
+	if (packet_length(&len, linelen)) {
+		linelen[4] = '\0';
+		die("protocol error: bad line length character: %s", linelen);
+	}
 	if (!len)
 		return 0;
 	len -= 4;
-- 
1.6.5.52.g0ff2e

[RFC PATCH v2 03/16] fetch-pack: Use a strbuf to compose the want list

[Shawn O. Pearce]

This change is being offered as a refactoring to make later
commits in the smart HTTP series easier.

By changing the enabled capabilities to be formatted in a strbuf
it is easier to add a new capability to the set of supported
capabilities.

By formatting the want portion of the request into a strbuf and
writing it as a whole block we can later decide to hold onto
the req_buf (instead of releasing it) to recycle in stateless
communications.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-fetch-pack.c |   52 ++++++++++++++++++++++++++++++++-----------------
 commit.c             |   10 +++-----
 commit.h             |    2 +-
 3 files changed, 39 insertions(+), 25 deletions(-)

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 629735f..783c2b0 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -165,6 +165,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 	const unsigned char *sha1;
 	unsigned in_vain = 0;
 	int got_continue = 0;
+	struct strbuf req_buf = STRBUF_INIT;
 
 	if (marked)
 		for_each_ref(clear_marks, NULL);
@@ -175,6 +176,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 	fetching = 0;
 	for ( ; refs ; refs = refs->next) {
 		unsigned char *remote = refs->old_sha1;
+		const char *remote_hex;
 		struct object *o;
 
 		/*
@@ -192,27 +194,36 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 			continue;
 		}
 
-		if (!fetching)
-			packet_write(fd[1], "want %s%s%s%s%s%s%s%s\n",
-				     sha1_to_hex(remote),
-				     (multi_ack ? " multi_ack" : ""),
-				     (use_sideband == 2 ? " side-band-64k" : ""),
-				     (use_sideband == 1 ? " side-band" : ""),
-				     (args.use_thin_pack ? " thin-pack" : ""),
-				     (args.no_progress ? " no-progress" : ""),
-				     (args.include_tag ? " include-tag" : ""),
-				     (prefer_ofs_delta ? " ofs-delta" : ""));
-		else
-			packet_write(fd[1], "want %s\n", sha1_to_hex(remote));
+		remote_hex = sha1_to_hex(remote);
+		if (!fetching) {
+			struct strbuf c = STRBUF_INIT;
+			if (multi_ack)          strbuf_addstr(&c, " multi_ack");
+			if (use_sideband == 2)  strbuf_addstr(&c, " side-band-64k");
+			if (use_sideband == 1)  strbuf_addstr(&c, " side-band");
+			if (args.use_thin_pack) strbuf_addstr(&c, " thin-pack");
+			if (args.no_progress)   strbuf_addstr(&c, " no-progress");
+			if (args.include_tag)   strbuf_addstr(&c, " include-tag");
+			if (prefer_ofs_delta)   strbuf_addstr(&c, " ofs-delta");
+			packet_buf_write(&req_buf, "want %s%s\n", remote_hex, c.buf);
+			strbuf_release(&c);
+		} else
+			packet_buf_write(&req_buf, "want %s\n", remote_hex);
 		fetching++;
 	}
+
+	if (!fetching) {
+		strbuf_release(&req_buf);
+		packet_flush(fd[1]);
+		return 1;
+	}
+
 	if (is_repository_shallow())
-		write_shallow_commits(fd[1], 1);
+		write_shallow_commits(&req_buf, 1);
 	if (args.depth > 0)
-		packet_write(fd[1], "deepen %d", args.depth);
-	packet_flush(fd[1]);
-	if (!fetching)
-		return 1;
+		packet_buf_write(&req_buf, "deepen %d", args.depth);
+	packet_buf_flush(&req_buf);
+
+	safe_write(fd[1], req_buf.buf, req_buf.len);
 
 	if (args.depth > 0) {
 		char line[1024];
@@ -296,6 +307,8 @@ done:
 		multi_ack = 0;
 		flushes++;
 	}
+	strbuf_release(&req_buf);
+
 	while (flushes || multi_ack) {
 		int ack = get_ack(fd[0], result_sha1);
 		if (ack) {
@@ -809,6 +822,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 
 	if (args.depth > 0) {
 		struct cache_time mtime;
+		struct strbuf sb = STRBUF_INIT;
 		char *shallow = git_path("shallow");
 		int fd;
 
@@ -826,12 +840,14 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 
 		fd = hold_lock_file_for_update(&lock, shallow,
 					       LOCK_DIE_ON_ERROR);
-		if (!write_shallow_commits(fd, 0)) {
+		if (!write_shallow_commits(&sb, 0)
+		 || write_in_full(fd, sb.buf, sb.len) != sb.len) {
 			unlink_or_warn(shallow);
 			rollback_lock_file(&lock);
 		} else {
 			commit_lock_file(&lock);
 		}
+		strbuf_release(&sb);
 	}
 
 	reprepare_packed_git();
diff --git a/commit.c b/commit.c
index fedbd5e..471efb0 100644
--- a/commit.c
+++ b/commit.c
@@ -199,7 +199,7 @@ struct commit_graft *lookup_commit_graft(const unsigned char *sha1)
 	return commit_graft[pos];
 }
 
-int write_shallow_commits(int fd, int use_pack_protocol)
+int write_shallow_commits(struct strbuf *out, int use_pack_protocol)
 {
 	int i, count = 0;
 	for (i = 0; i < commit_graft_nr; i++)
@@ -208,12 +208,10 @@ int write_shallow_commits(int fd, int use_pack_protocol)
 				sha1_to_hex(commit_graft[i]->sha1);
 			count++;
 			if (use_pack_protocol)
-				packet_write(fd, "shallow %s", hex);
+				packet_buf_write(out, "shallow %s", hex);
 			else {
-				if (write_in_full(fd, hex,  40) != 40)
-					break;
-				if (write_str_in_full(fd, "\n") != 1)
-					break;
+				strbuf_addstr(out, hex);
+				strbuf_addch(out, '\n');
 			}
 		}
 	return count;
diff --git a/commit.h b/commit.h
index f4fc5c5..817c75c 100644
--- a/commit.h
+++ b/commit.h
@@ -131,7 +131,7 @@ extern struct commit_list *get_octopus_merge_bases(struct commit_list *in);
 
 extern int register_shallow(const unsigned char *sha1);
 extern int unregister_shallow(const unsigned char *sha1);
-extern int write_shallow_commits(int fd, int use_pack_protocol);
+extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol);
 extern int is_repository_shallow(void);
 extern struct commit_list *get_shallow_commits(struct object_array *heads,
 		int depth, int shallow_flag, int not_shallow_flag);
-- 
1.6.5.52.g0ff2e

[RFC PATCH v2 04/16] Move "get_ack()" back to fetch-pack

[Shawn O. Pearce]

In 41cb7488 Linus moved this function to connect.c for reuse inside
of the git-clone-pack command.  That was 2005, but in 2006 Junio
retired git-clone-pack in commit efc7fa53.  Since then the only
caller has been fetch-pack.  Since this ACK/NAK exchange is only
used by the fetch-pack/upload-pack protocol we should keep move
it back to a private detail of fetch-pack.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-fetch-pack.c |   21 +++++++++++++++++++++
 cache.h              |    1 -
 connect.c            |   21 ---------------------
 3 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 783c2b0..7c09d46 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -157,6 +157,27 @@ static const unsigned char *get_rev(void)
 	return commit->object.sha1;
 }
 
+static int get_ack(int fd, unsigned char *result_sha1)
+{
+	static char line[1000];
+	int len = packet_read_line(fd, line, sizeof(line));
+
+	if (!len)
+		die("git fetch-pack: expected ACK/NAK, got EOF");
+	if (line[len-1] == '\n')
+		line[--len] = 0;
+	if (!strcmp(line, "NAK"))
+		return 0;
+	if (!prefixcmp(line, "ACK ")) {
+		if (!get_sha1_hex(line+4, result_sha1)) {
+			if (strstr(line+45, "continue"))
+				return 2;
+			return 1;
+		}
+	}
+	die("git fetch_pack: expected ACK/NAK, got '%s'", line);
+}
+
 static int find_common(int fd[2], unsigned char *result_sha1,
 		       struct ref *refs)
 {
diff --git a/cache.h b/cache.h
index a5eeead..4e283be 100644
--- a/cache.h
+++ b/cache.h
@@ -856,7 +856,6 @@ extern struct ref *find_ref_by_name(const struct ref *list, const char *name);
 extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
 extern int finish_connect(struct child_process *conn);
 extern int path_match(const char *path, int nr, char **match);
-extern int get_ack(int fd, unsigned char *result_sha1);
 struct extra_have_objects {
 	int nr, alloc;
 	unsigned char (*array)[20];
diff --git a/connect.c b/connect.c
index 7945e38..839a103 100644
--- a/connect.c
+++ b/connect.c
@@ -107,27 +107,6 @@ int server_supports(const char *feature)
 		strstr(server_capabilities, feature) != NULL;
 }
 
-int get_ack(int fd, unsigned char *result_sha1)
-{
-	static char line[1000];
-	int len = packet_read_line(fd, line, sizeof(line));
-
-	if (!len)
-		die("git fetch-pack: expected ACK/NAK, got EOF");
-	if (line[len-1] == '\n')
-		line[--len] = 0;
-	if (!strcmp(line, "NAK"))
-		return 0;
-	if (!prefixcmp(line, "ACK ")) {
-		if (!get_sha1_hex(line+4, result_sha1)) {
-			if (strstr(line+45, "continue"))
-				return 2;
-			return 1;
-		}
-	}
-	die("git fetch_pack: expected ACK/NAK, got '%s'", line);
-}
-
 int path_match(const char *path, int nr, char **match)
 {
 	int i;
-- 
1.6.5.52.g0ff2e

[RFC PATCH v2 05/16] Add multi_ack_2 capability to fetch-pack/upload-pack

[Shawn O. Pearce]

When multi_ack_2 is enabled the ACK continue messages returned by the
remote upload-pack are broken out to describe the different states
within the peer.  This permits the client to better understand the
server's in-memory state.

The fetch-pack/upload-pack protocol now looks like:

NAK
---------------------------------
  Always sent in response to "done" if there was no common base
  selected from the "have" lines (or no have lines were sent).

  * no multi_ack or multi_ack_2:

    Sent when the client has sent a pkt-line flush ("0000") and
    the server has not yet found a common base object.

  * either multi_ack or multi_ack_2:

    Always sent in response to a pkt-line flush.

ACK %s
-----------------------------------
  * no multi_ack or multi_ack_2:

    Sent in response to "have" when the object exists on the remote
    side and is therefore an object in common between the peers.
    The argument is the SHA-1 of the common object.

  * either multi_ack or multi_ack_2:

    Sent in response to "done" if there are common objects.
    The argument is the last SHA-1 determined to be common.

ACK %s continue
-----------------------------------
  * multi_ack only:

    Sent in response to "have".

    The remote side wants the client to consider this object as
    common, and immediately stop transmitting additional "have"
    lines for objects that are reachable from it.  The reason
    the client should stop is not given, but is one of the two
    cases below available under multi_ack_2.

ACK %s common
-----------------------------------
  * multi_ack_2 only:

    Sent in response to "have".  Both sides have this object.
    Like with "ACK %s continue" above the client should stop
    sending have lines reachable for objects from the argument.

ACK %s ready
-----------------------------------
  * multi_ack_2 only:

    Sent in response to "have".

    The client should stop transmitting objects which are reachable
    from the argument, and send "done" soon to get the objects.

    If the remote side has the specified object, it should
    first send an "ACK %s common" message prior to sending
    "ACK %s ready".

    Clients may still submit additional "have" lines if there are
    more side branches for the client to explore that might be added
    to the common set and reduce the number of objects to transfer.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-fetch-pack.c |   41 ++++++++++++++++++++++++++++++++---------
 upload-pack.c        |   31 ++++++++++++++++++-------------
 2 files changed, 50 insertions(+), 22 deletions(-)

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 7c09d46..b68b3eb 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -157,7 +157,15 @@ static const unsigned char *get_rev(void)
 	return commit->object.sha1;
 }
 
-static int get_ack(int fd, unsigned char *result_sha1)
+enum ack_type {
+	NAK = 0,
+	ACK,
+	ACK_continue,
+	ACK_common,
+	ACK_ready
+};
+
+static enum ack_type get_ack(int fd, unsigned char *result_sha1)
 {
 	static char line[1000];
 	int len = packet_read_line(fd, line, sizeof(line));
@@ -167,12 +175,16 @@ static int get_ack(int fd, unsigned char *result_sha1)
 	if (line[len-1] == '\n')
 		line[--len] = 0;
 	if (!strcmp(line, "NAK"))
-		return 0;
+		return NAK;
 	if (!prefixcmp(line, "ACK ")) {
 		if (!get_sha1_hex(line+4, result_sha1)) {
 			if (strstr(line+45, "continue"))
-				return 2;
-			return 1;
+				return ACK_continue;
+			if (strstr(line+45, "common"))
+				return ACK_common;
+			if (strstr(line+45, "ready"))
+				return ACK_ready;
+			return ACK;
 		}
 	}
 	die("git fetch_pack: expected ACK/NAK, got '%s'", line);
@@ -218,7 +230,8 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 		remote_hex = sha1_to_hex(remote);
 		if (!fetching) {
 			struct strbuf c = STRBUF_INIT;
-			if (multi_ack)          strbuf_addstr(&c, " multi_ack");
+			if (multi_ack == 2)     strbuf_addstr(&c, " multi_ack_2");
+			if (multi_ack == 1)     strbuf_addstr(&c, " multi_ack");
 			if (use_sideband == 2)  strbuf_addstr(&c, " side-band-64k");
 			if (use_sideband == 1)  strbuf_addstr(&c, " side-band");
 			if (args.use_thin_pack) strbuf_addstr(&c, " thin-pack");
@@ -298,18 +311,23 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 				if (args.verbose && ack)
 					fprintf(stderr, "got ack %d %s\n", ack,
 							sha1_to_hex(result_sha1));
-				if (ack == 1) {
+				switch (ack) {
+				case ACK:
 					flushes = 0;
 					multi_ack = 0;
 					retval = 0;
 					goto done;
-				} else if (ack == 2) {
+				case ACK_common:
+				case ACK_ready:
+				case ACK_continue: {
 					struct commit *commit =
 						lookup_commit(result_sha1);
 					mark_common(commit, 0, 1);
 					retval = 0;
 					in_vain = 0;
 					got_continue = 1;
+					break;
+					}
 				}
 			} while (ack);
 			flushes--;
@@ -336,7 +354,7 @@ done:
 			if (args.verbose)
 				fprintf(stderr, "got ack (%d) %s\n", ack,
 					sha1_to_hex(result_sha1));
-			if (ack == 1)
+			if (ack == ACK)
 				return 0;
 			multi_ack = 1;
 			continue;
@@ -618,7 +636,12 @@ static struct ref *do_fetch_pack(int fd[2],
 
 	if (is_repository_shallow() && !server_supports("shallow"))
 		die("Server does not support shallow clients");
-	if (server_supports("multi_ack")) {
+	if (server_supports("multi_ack_2")) {
+		if (args.verbose)
+			fprintf(stderr, "Server supports multi_ack_2\n");
+		multi_ack = 2;
+	}
+	else if (server_supports("multi_ack")) {
 		if (args.verbose)
 			fprintf(stderr, "Server supports multi_ack\n");
 		multi_ack = 1;
diff --git a/upload-pack.c b/upload-pack.c
index 38ddac2..5024b59 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -498,7 +498,7 @@ static int get_common_commits(void)
 {
 	static char line[1000];
 	unsigned char sha1[20];
-	char hex[41], last_hex[41];
+	char last_hex[41];
 
 	save_commit_buffer = 0;
 
@@ -515,19 +515,22 @@ static int get_common_commits(void)
 		if (!prefixcmp(line, "have ")) {
 			switch (got_sha1(line+5, sha1)) {
 			case -1: /* they have what we do not */
-				if (multi_ack && ok_to_give_up())
-					packet_write(1, "ACK %s continue\n",
-						     sha1_to_hex(sha1));
+				if (multi_ack && ok_to_give_up()) {
+					const char *hex = sha1_to_hex(sha1);
+					if (multi_ack == 2)
+						packet_write(1, "ACK %s ready\n", hex);
+					else
+						packet_write(1, "ACK %s continue\n", hex);
+				}
 				break;
 			default:
-				memcpy(hex, sha1_to_hex(sha1), 41);
-				if (multi_ack) {
-					const char *msg = "ACK %s continue\n";
-					packet_write(1, msg, hex);
-					memcpy(last_hex, hex, 41);
-				}
+				memcpy(last_hex, sha1_to_hex(sha1), 41);
+				if (multi_ack == 2)
+					packet_write(1, "ACK %s common\n", last_hex);
+				else if (multi_ack)
+					packet_write(1, "ACK %s continue\n", last_hex);
 				else if (have_obj.nr == 1)
-					packet_write(1, "ACK %s\n", hex);
+					packet_write(1, "ACK %s\n", last_hex);
 				break;
 			}
 			continue;
@@ -587,7 +590,9 @@ static void receive_needs(void)
 		    get_sha1_hex(line+5, sha1_buf))
 			die("git upload-pack: protocol error, "
 			    "expected to get sha, not '%s'", line);
-		if (strstr(line+45, "multi_ack"))
+		if (strstr(line+45, "multi_ack_2"))
+			multi_ack = 2;
+		else if (strstr(line+45, "multi_ack"))
 			multi_ack = 1;
 		if (strstr(line+45, "thin-pack"))
 			use_thin_pack = 1;
@@ -681,7 +686,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 {
 	static const char *capabilities = "multi_ack thin-pack side-band"
 		" side-band-64k ofs-delta shallow no-progress"
-		" include-tag";
+		" include-tag multi_ack_2";
 	struct object *o = parse_object(sha1);
 
 	if (!o)
-- 
1.6.5.52.g0ff2e

Re: [RFC PATCH v2 05/16] Add multi_ack_2 capability to fetch-pack/upload-pack

[Jakub Narebski]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> When multi_ack_2 is enabled the ACK continue messages returned by the
> remote upload-pack are broken out to describe the different states
> within the peer.  This permits the client to better understand the
> server's in-memory state.

Errr... can't you find better name than multi_ack_2?  Perhaps
multi_ack_detailed or something...

> 
> The fetch-pack/upload-pack protocol now looks like:
[...]

> ACK %s continue
> -----------------------------------
>   * multi_ack only:
> 
>     Sent in response to "have".
> 
>     The remote side wants the client to consider this object as
>     common, and immediately stop transmitting additional "have"
>     lines for objects that are reachable from it.  The reason
>     the client should stop is not given, but is one of the two
>     cases below available under multi_ack_2.
> 
> ACK %s common
> -----------------------------------
>   * multi_ack_2 only:
> 
>     Sent in response to "have".  Both sides have this object.
>     Like with "ACK %s continue" above the client should stop
>     sending have lines reachable for objects from the argument.
> 
> ACK %s ready
> -----------------------------------
>   * multi_ack_2 only:
> 
>     Sent in response to "have".
> 
>     The client should stop transmitting objects which are reachable
>     from the argument, and send "done" soon to get the objects.
> 
>     If the remote side has the specified object, it should
>     first send an "ACK %s common" message prior to sending
>     "ACK %s ready".
> 
>     Clients may still submit additional "have" lines if there are
>     more side branches for the client to explore that might be added
>     to the common set and reduce the number of objects to transfer.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [RFC PATCH v2 05/16] Add multi_ack_2 capability to fetch-pack/upload-pack

[Shawn O. Pearce]

Jakub Narebski <jnareb@gmail.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > When multi_ack_2 is enabled the ACK continue messages returned by the
> > remote upload-pack are broken out to describe the different states
> > within the peer.  This permits the client to better understand the
> > server's in-memory state.
> 
> Errr... can't you find better name than multi_ack_2?  Perhaps
> multi_ack_detailed or something...

Uh... yes.  Good idea.  Thanks.  :-)
 
-- 
Shawn.

[RFC PATCH v2 06/16] remote-curl: Refactor walker initialization

[Shawn O. Pearce]

We will need the walker, url and remote in other functions as the
code grows larger to support smart HTTP.  Extract this out into a
set of globals we can easily reference once configured.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
CC: Daniel Barkalow <barkalow@iabervon.org>
---
 remote-curl.c |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index ad6a163..4628ee8 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -4,7 +4,17 @@
 #include "walker.h"
 #include "http.h"
 
-static struct ref *get_refs(struct walker *walker, const char *url)
+static struct remote *remote;
+static const char *url;
+static struct walker *walker;
+
+static void init_walker(void)
+{
+	if (!walker)
+		walker = get_http_walker(url, remote);
+}
+
+static struct ref *get_refs(void)
 {
 	struct strbuf buffer = STRBUF_INIT;
 	char *data, *start, *mid;
@@ -20,6 +30,7 @@ static struct ref *get_refs(struct walker *walker, const char *url)
 	refs_url = xmalloc(strlen(url) + 11);
 	sprintf(refs_url, "%s/info/refs", url);
 
+	init_walker();
 	http_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE);
 	switch (http_ret) {
 	case HTTP_OK:
@@ -77,10 +88,7 @@ static struct ref *get_refs(struct walker *walker, const char *url)
 
 int main(int argc, const char **argv)
 {
-	struct remote *remote;
 	struct strbuf buf = STRBUF_INIT;
-	const char *url;
-	struct walker *walker = NULL;
 
 	setup_git_directory();
 	if (argc < 2) {
@@ -101,8 +109,7 @@ int main(int argc, const char **argv)
 			break;
 		if (!prefixcmp(buf.buf, "fetch ")) {
 			char *obj = buf.buf + strlen("fetch ");
-			if (!walker)
-				walker = get_http_walker(url, remote);
+			init_walker();
 			walker->get_all = 1;
 			walker->get_tree = 1;
 			walker->get_history = 1;
@@ -113,11 +120,8 @@ int main(int argc, const char **argv)
 			printf("\n");
 			fflush(stdout);
 		} else if (!strcmp(buf.buf, "list")) {
-			struct ref *refs;
+			struct ref *refs = get_refs();
 			struct ref *posn;
-			if (!walker)
-				walker = get_http_walker(url, remote);
-			refs = get_refs(walker, url);
 			for (posn = refs; posn; posn = posn->next) {
 				if (posn->symref)
 					printf("@%s %s\n", posn->symref, posn->name);
-- 
1.6.5.52.g0ff2e

[RFC PATCH v2 07/16] remote-helpers: Fetch more than one ref in a batch

[Shawn O. Pearce]

Some network protocols (e.g. native git://) are able to fetch more
than one ref at a time and reduce the overall transfer cost by
combining the requests into a single exchange.  Instead of feeding
each fetch request one at a time to the helper, feed all of them
at once so the helper can decide whether or not it should batch them.

Because 'fetch' was already released in 1.6.5 we introduce the new
fetch-multiple capability/command to signal that the helper wants
to use batch oriented approach to fetching refs.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
CC: Daniel Barkalow <barkalow@iabervon.org>
---
 Documentation/git-remote-helpers.txt |   18 ++++++
 remote-curl.c                        |   98 ++++++++++++++++++++++++++++++----
 transport-helper.c                   |   58 +++++++++++++++-----
 3 files changed, 149 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 173ee23..e10ce99 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -43,6 +43,15 @@ Commands are given by the caller on the helper's standard input, one per line.
 +
 Supported if the helper has the "fetch" capability.
 
+'fetch-multiple'::
+	Fetches multiple objects at once.  The fetch-multiple
+	command is followed by one or more 'fetch' lines as above,
+	and then a blank line to terminate the batch.  Outputs a
+	single blank line when the entire batch is complete.
+	Optionally may output a 'lock <file>' line indicating a
+	file under GIT_DIR/objects/pack which is keeping a pack
+	until refs can be suitably updated.
+
 If a fatal error occurs, the program writes the error message to
 stderr and exits. The caller should expect that a suitable error
 message has been printed if the child closes the connection without
@@ -57,11 +66,20 @@ CAPABILITIES
 'fetch'::
 	This helper supports the 'fetch' command.
 
+'fetch-multiple'::
+	This helper supports the 'fetch-multiple' command.
+
 REF LIST ATTRIBUTES
 -------------------
 
 None are defined yet, but the caller must accept any which are supplied.
 
+FETCH OPTIONS
+-------------
+
+'option verbose'::
+	Print more verbose activity messages to stderr.
+
 Documentation
 -------------
 Documentation by Daniel Barkalow.
diff --git a/remote-curl.c b/remote-curl.c
index 4628ee8..34ca4e7 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -86,6 +86,84 @@ static struct ref *get_refs(void)
 	return refs;
 }
 
+static int fetch_dumb(int nr_heads, struct ref **to_fetch)
+{
+	char **targets = xmalloc(nr_heads * sizeof(char*));
+	int ret, i;
+
+	for (i = 0; i < nr_heads; i++)
+		targets[i] = xstrdup(sha1_to_hex(to_fetch[i]->old_sha1));
+
+	init_walker();
+	walker->get_all = 1;
+	walker->get_tree = 1;
+	walker->get_history = 1;
+	walker->get_verbosely = 0;
+	walker->get_recover = 0;
+	ret = walker_fetch(walker, nr_heads, targets, NULL, NULL);
+
+	for (i = 0; i < nr_heads; i++)
+		free(targets[i]);
+	free(targets);
+
+	return ret ? error("Fetch failed.") : 0;
+}
+
+static void parse_fetch(struct strbuf *buf, int multiple)
+{
+	struct ref **to_fetch = NULL;
+	struct ref *list_head = NULL;
+	struct ref **list = &list_head;
+	int alloc_heads = 0, nr_heads = 0;
+
+	do {
+		if (!prefixcmp(buf->buf, "fetch ")) {
+			char *p = buf->buf + strlen("fetch ");
+			char *name;
+			struct ref *ref;
+			unsigned char old_sha1[20];
+
+			if (strlen(p) < 40 || get_sha1_hex(p, old_sha1))
+				die("protocol error: expected sha/ref, got %s'", p);
+			if (p[40] == ' ')
+				name = p + 41;
+			else if (!p[40])
+				name = "";
+			else
+				die("protocol error: expected sha/ref, got %s'", p);
+
+			ref = alloc_ref(name);
+			hashcpy(ref->old_sha1, old_sha1);
+
+			*list = ref;
+			list = &ref->next;
+
+			ALLOC_GROW(to_fetch, nr_heads + 1, alloc_heads);
+			to_fetch[nr_heads++] = ref;
+
+			if (!multiple)
+				break;
+		}
+		else
+			die("http transport does not support %s", buf->buf);
+
+		strbuf_reset(buf);
+		if (strbuf_getline(buf, stdin, '\n') == EOF)
+			return;
+		if (!*buf->buf)
+			break;
+	} while (1);
+
+	if (fetch_dumb(nr_heads, to_fetch))
+		exit(128); /* error already reported */
+	free_refs(list_head);
+	free(to_fetch);
+
+	printf("\n");
+	fflush(stdout);
+	strbuf_reset(buf);
+}
+
 int main(int argc, const char **argv)
 {
 	struct strbuf buf = STRBUF_INIT;
@@ -108,17 +186,14 @@ int main(int argc, const char **argv)
 		if (strbuf_getline(&buf, stdin, '\n') == EOF)
 			break;
 		if (!prefixcmp(buf.buf, "fetch ")) {
-			char *obj = buf.buf + strlen("fetch ");
-			init_walker();
-			walker->get_all = 1;
-			walker->get_tree = 1;
-			walker->get_history = 1;
-			walker->get_verbosely = 0;
-			walker->get_recover = 0;
-			if (walker_fetch(walker, 1, &obj, NULL, NULL))
-				die("Fetch failed.");
-			printf("\n");
-			fflush(stdout);
+			parse_fetch(&buf, 0);
+
+		} else if (!strcmp(buf.buf, "fetch-multiple")) {
+			strbuf_reset(&buf);
+			if (strbuf_getline(&buf, stdin, '\n') == EOF)
+				break;
+			parse_fetch(&buf, 1);
+
 		} else if (!strcmp(buf.buf, "list")) {
 			struct ref *refs = get_refs();
 			struct ref *posn;
@@ -132,6 +207,7 @@ int main(int argc, const char **argv)
 			fflush(stdout);
 		} else if (!strcmp(buf.buf, "capabilities")) {
 			printf("fetch\n");
+			printf("fetch-multiple\n");
 			printf("\n");
 			fflush(stdout);
 		} else {
diff --git a/transport-helper.c b/transport-helper.c
index f57e84c..eb66e0c 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -10,7 +10,9 @@ struct helper_data
 {
 	const char *name;
 	struct child_process *helper;
-	unsigned fetch : 1;
+	FILE *out;
+	unsigned fetch : 1,
+		fetch_multiple : 1;
 };
 
 static struct child_process *get_helper(struct transport *transport)
@@ -18,7 +20,6 @@ static struct child_process *get_helper(struct transport *transport)
 	struct helper_data *data = transport->data;
 	struct strbuf buf = STRBUF_INIT;
 	struct child_process *helper;
-	FILE *file;
 
 	if (data->helper)
 		return data->helper;
@@ -39,15 +40,17 @@ static struct child_process *get_helper(struct transport *transport)
 
 	write_str_in_full(helper->in, "capabilities\n");
 
-	file = xfdopen(helper->out, "r");
+	data->out = xfdopen(helper->out, "r");
 	while (1) {
-		if (strbuf_getline(&buf, file, '\n') == EOF)
+		if (strbuf_getline(&buf, data->out, '\n') == EOF)
 			exit(128); /* child died, message supplied already */
 
 		if (!*buf.buf)
 			break;
 		if (!strcmp(buf.buf, "fetch"))
 			data->fetch = 1;
+		if (!strcmp(buf.buf, "fetch-multiple"))
+			data->fetch_multiple = 1;
 	}
 	return data->helper;
 }
@@ -58,6 +61,7 @@ static int disconnect_helper(struct transport *transport)
 	if (data->helper) {
 		write_str_in_full(data->helper->in, "\n");
 		close(data->helper->in);
+		fclose(data->out);
 		finish_command(data->helper);
 		free((char *)data->helper->argv[0]);
 		free(data->helper->argv);
@@ -67,14 +71,37 @@ static int disconnect_helper(struct transport *transport)
 	return 0;
 }
 
+static void perform_fetch_command(struct transport *transport,
+	struct strbuf *buf)
+{
+	struct helper_data *data = transport->data;
+	size_t n = buf->len;
+
+	if (write_in_full(data->helper->in, buf->buf, n) != n)
+		exit(128);
+
+	while (1) {
+		strbuf_reset(buf);
+		if (strbuf_getline(buf, data->out, '\n') == EOF)
+			exit(128); /* child died, message supplied already */
+		if (!prefixcmp(buf->buf, "lock "))
+			transport->pack_lockfile = xstrdup(buf->buf + 5);
+		else if (!buf->len)
+			break;
+	}
+	strbuf_reset(buf);
+}
+
 static int fetch_with_fetch(struct transport *transport,
 			    int nr_heads, const struct ref **to_fetch)
 {
-	struct child_process *helper = get_helper(transport);
-	FILE *file = xfdopen(helper->out, "r");
+	struct helper_data *data = transport->data;
 	int i;
 	struct strbuf buf = STRBUF_INIT;
 
+	if (data->fetch_multiple)
+		strbuf_addstr(&buf, "fetch-multiple\n");
+
 	for (i = 0; i < nr_heads; i++) {
 		const struct ref *posn = to_fetch[i];
 		if (posn->status & REF_STATUS_UPTODATE)
@@ -82,12 +109,16 @@ static int fetch_with_fetch(struct transport *transport,
 
 		strbuf_addf(&buf, "fetch %s %s\n",
 			    sha1_to_hex(posn->old_sha1), posn->name);
-		write_in_full(helper->in, buf.buf, buf.len);
-		strbuf_reset(&buf);
+		if (!data->fetch_multiple)
+			perform_fetch_command(transport, &buf);
+	}
 
-		if (strbuf_getline(&buf, file, '\n') == EOF)
-			exit(128); /* child died, message supplied already */
+	if (data->fetch_multiple) {
+		strbuf_addch(&buf, '\n');
+		perform_fetch_command(transport, &buf);
 	}
+
+	strbuf_release(&buf);
 	return 0;
 }
 
@@ -105,7 +136,7 @@ static int fetch(struct transport *transport,
 	if (!count)
 		return 0;
 
-	if (data->fetch)
+	if (data->fetch || data->fetch_multiple)
 		return fetch_with_fetch(transport, nr_heads, to_fetch);
 
 	return -1;
@@ -113,21 +144,20 @@ static int fetch(struct transport *transport,
 
 static struct ref *get_refs_list(struct transport *transport, int for_push)
 {
+	struct helper_data *data = transport->data;
 	struct child_process *helper;
 	struct ref *ret = NULL;
 	struct ref **tail = &ret;
 	struct ref *posn;
 	struct strbuf buf = STRBUF_INIT;
-	FILE *file;
 
 	helper = get_helper(transport);
 
 	write_str_in_full(helper->in, "list\n");
 
-	file = xfdopen(helper->out, "r");
 	while (1) {
 		char *eov, *eon;
-		if (strbuf_getline(&buf, file, '\n') == EOF)
+		if (strbuf_getline(&buf, data->out, '\n') == EOF)
 			exit(128); /* child died, message supplied already */
 
 		if (!*buf.buf)
-- 
1.6.5.52.g0ff2e

Re: [RFC PATCH v2 07/16] remote-helpers: Fetch more than one ref in a batch

[Daniel Barkalow]

On Mon, 12 Oct 2009, Shawn O. Pearce wrote:

> Some network protocols (e.g. native git://) are able to fetch more
> than one ref at a time and reduce the overall transfer cost by
> combining the requests into a single exchange.  Instead of feeding
> each fetch request one at a time to the helper, feed all of them
> at once so the helper can decide whether or not it should batch them.
> 
> Because 'fetch' was already released in 1.6.5 we introduce the new
> fetch-multiple capability/command to signal that the helper wants
> to use batch oriented approach to fetching refs.

In 1.6.5, there's no way to call a helper other than git-remote-curl, and 
no way to call git-remote-curl unless 1.6.5 was built with it. So I think 
the protocol is not set in stone quite yet. It's documentated for being an 
API, and it's supposed to be that, but it's not quite there in this 
version.

I think it should be generally a good idea to have a start signal and an 
end signal for a block of fetches (and, with the foreign stuff, it would 
be useful to have transport-helper tell the helper process when it was 
done making requests, so the helper process could tell the gfi process to 
exit and stop consuming the helper process's output). At worst, if the 
helper doesn't care, it can just ignore this information.

	-Daniel
*This .sig left intentionally blank*

Re: [RFC PATCH v2 07/16] remote-helpers: Fetch more than one ref in a batch

[Shawn O. Pearce]

Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Mon, 12 Oct 2009, Shawn O. Pearce wrote:
> > Some network protocols (e.g. native git://) are able to fetch more
> > than one ref at a time and reduce the overall transfer cost by
> > combining the requests into a single exchange.  Instead of feeding
> > each fetch request one at a time to the helper, feed all of them
> > at once so the helper can decide whether or not it should batch them.
> > 
> > Because 'fetch' was already released in 1.6.5 we introduce the new
> > fetch-multiple capability/command to signal that the helper wants
> > to use batch oriented approach to fetching refs.
> 
> In 1.6.5, there's no way to call a helper other than git-remote-curl, and 
> no way to call git-remote-curl unless 1.6.5 was built with it. So I think 
> the protocol is not set in stone quite yet. It's documentated for being an 
> API, and it's supposed to be that, but it's not quite there in this 
> version.

I originally had this change as just redefining "fetch" to be batch
oriented and requiring a blank line to terminate the batch group,
but reconsidered when 1.6.5 shipped and I realized this code was
in 1.6.5.

But, yea, you are right, with no way to invoke anything except
remote-curl there really isn't that much of a problem if we change
the protocol.

-- 
Shawn.

[RFC PATCH v2 08/16] remote-helpers: Support custom transport options

[Shawn O. Pearce]

Some transports, like the native pack transport implemented by
fetch-pack, support useful features like depth or include tags.
These should be exposed if the underlying helper knows how to
use them and is based upon the same infrastructure.

Helpers must advertise the options they support, any attempt
to set an unsupported option will cause a failure.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
CC: Daniel Barkalow <barkalow@iabervon.org>
---
 Documentation/git-remote-helpers.txt |   20 ++++++++++
 remote-curl.c                        |   16 ++++++-
 transport-helper.c                   |   70 ++++++++++++++++++++++++++++++++++
 3 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index e10ce99..334ab30 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -46,6 +46,7 @@ Supported if the helper has the "fetch" capability.
 'fetch-multiple'::
 	Fetches multiple objects at once.  The fetch-multiple
 	command is followed by one or more 'fetch' lines as above,
+	zero or more 'option' lines for the supported options,
 	and then a blank line to terminate the batch.  Outputs a
 	single blank line when the entire batch is complete.
 	Optionally may output a 'lock <file>' line indicating a
@@ -69,6 +70,9 @@ CAPABILITIES
 'fetch-multiple'::
 	This helper supports the 'fetch-multiple' command.
 
+'option' <name>::
+	This helper supports the option <name> under fetch-multiple.
+
 REF LIST ATTRIBUTES
 -------------------
 
@@ -76,10 +80,26 @@ None are defined yet, but the caller must accept any which are supplied.
 
 FETCH OPTIONS
 -------------
+To enable an option the helper must list it in 'capabilities'.
 
 'option verbose'::
 	Print more verbose activity messages to stderr.
 
+'option uploadpack' <command>::
+	The program to use on the remote side to generate a pack.
+
+'option depth' <depth>::
+	Deepen the history of a shallow repository.
+
+'option keep'::
+	Keep the transferred pack(s) with .keep files.
+
+'option followtags'::
+	Aggressively fetch annotated tags if possible.
+
+'option thin'::
+	Transfer the data as a thin pack if possible.
+
 Documentation
 -------------
 Documentation by Daniel Barkalow.
diff --git a/remote-curl.c b/remote-curl.c
index 34ca4e7..e5d9768 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -86,7 +86,12 @@ static struct ref *get_refs(void)
 	return refs;
 }
 
-static int fetch_dumb(int nr_heads, struct ref **to_fetch)
+struct fetch_args {
+	unsigned verbose : 1;
+};
+
+static int fetch_dumb(struct fetch_args *args,
+	int nr_heads, struct ref **to_fetch)
 {
 	char **targets = xmalloc(nr_heads * sizeof(char*));
 	int ret, i;
@@ -98,7 +103,7 @@ static int fetch_dumb(int nr_heads, struct ref **to_fetch)
 	walker->get_all = 1;
 	walker->get_tree = 1;
 	walker->get_history = 1;
-	walker->get_verbosely = 0;
+	walker->get_verbosely = args->verbose;
 	walker->get_recover = 0;
 	ret = walker_fetch(walker, nr_heads, targets, NULL, NULL);
 
@@ -115,7 +120,9 @@ static void parse_fetch(struct strbuf *buf, int multiple)
 	struct ref *list_head = NULL;
 	struct ref **list = &list_head;
 	int alloc_heads = 0, nr_heads = 0;
+	struct fetch_args args;
 
+	memset(&args, 0, sizeof(args));
 	do {
 		if (!prefixcmp(buf->buf, "fetch ")) {
 			char *p = buf->buf + strlen("fetch ");
@@ -144,6 +151,8 @@ static void parse_fetch(struct strbuf *buf, int multiple)
 			if (!multiple)
 				break;
 		}
+		else if (!strcmp(buf->buf, "option verbose"))
+			args.verbose = 1;
 		else
 			die("http transport does not support %s", buf->buf);
 
@@ -154,7 +163,7 @@ static void parse_fetch(struct strbuf *buf, int multiple)
 			break;
 	} while (1);
 
-	if (fetch_dumb(nr_heads, to_fetch))
+	if (fetch_dumb(&args, nr_heads, to_fetch))
 		exit(128); /* error already reported */
 	free_refs(list_head);
 	free(to_fetch);
@@ -208,6 +217,7 @@ int main(int argc, const char **argv)
 		} else if (!strcmp(buf.buf, "capabilities")) {
 			printf("fetch\n");
 			printf("fetch-multiple\n");
+			printf("option verbose\n");
 			printf("\n");
 			fflush(stdout);
 		} else {
diff --git a/transport-helper.c b/transport-helper.c
index eb66e0c..bb6cd1b 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -5,12 +5,15 @@
 #include "commit.h"
 #include "diff.h"
 #include "revision.h"
+#include "string-list.h"
+
 
 struct helper_data
 {
 	const char *name;
 	struct child_process *helper;
 	FILE *out;
+	struct string_list options;
 	unsigned fetch : 1,
 		fetch_multiple : 1;
 };
@@ -51,6 +54,11 @@ static struct child_process *get_helper(struct transport *transport)
 			data->fetch = 1;
 		if (!strcmp(buf.buf, "fetch-multiple"))
 			data->fetch_multiple = 1;
+		if (!prefixcmp(buf.buf, "option ")) {
+			const char *name = buf.buf + strlen("option ");
+			if (!string_list_lookup(name, &data->options))
+				string_list_insert(xstrdup(name), &data->options);
+		}
 	}
 	return data->helper;
 }
@@ -68,6 +76,65 @@ static int disconnect_helper(struct transport *transport)
 		free(data->helper);
 		data->helper = NULL;
 	}
+	string_list_clear(&data->options, 1);
+	free(data);
+	return 0;
+}
+
+static int save_option(struct transport *transport,
+			  const char *name, const char *value)
+{
+	struct helper_data *data = transport->data;
+	struct string_list_item *s;
+
+	s = string_list_lookup(name, &data->options);
+	if (!s)
+		return 1;
+	free(s->util);
+	s->util = value ? xstrdup(value) : NULL;
+	return 0;
+}
+
+static int set_helper_option(struct transport *transport,
+			  const char *name, const char *value)
+{
+	struct helper_data *data = transport->data;
+	int is_bool = 0;
+
+	get_helper(transport);
+
+	if (!data->fetch_multiple)
+		return 1;
+
+	if (!strcmp(name, TRANS_OPT_THIN))
+		is_bool = 1;
+	else if (!strcmp(name, TRANS_OPT_KEEP))
+		is_bool = 1;
+	else if (!strcmp(name, TRANS_OPT_FOLLOWTAGS))
+		is_bool = 1;
+
+	if (is_bool)
+		value = value ? "" : NULL;
+	return save_option(transport, name, value);
+}
+
+static void standard_options(struct transport *transport)
+{
+	save_option(transport, "verbose", transport->verbose ? "" : NULL);
+}
+
+static int print_options(struct string_list_item *s, void *arg)
+{
+	struct strbuf *buf = arg;
+	char *name = s->string;
+	char *value = s->util;
+
+	if (!value)
+		return 0;
+	else if (*value)
+		strbuf_addf(buf, "option %s %s\n", name, value);
+	else
+		strbuf_addf(buf, "option %s\n", name);
 	return 0;
 }
 
@@ -114,6 +181,8 @@ static int fetch_with_fetch(struct transport *transport,
 	}
 
 	if (data->fetch_multiple) {
+		standard_options(transport);
+		for_each_string_list(print_options, &data->options, &buf);
 		strbuf_addch(&buf, '\n');
 		perform_fetch_command(transport, &buf);
 	}
@@ -191,6 +260,7 @@ int transport_helper_init(struct transport *transport, const char *name)
 	data->name = name;
 
 	transport->data = data;
+	transport->set_option = set_helper_option;
 	transport->get_refs_list = get_refs_list;
 	transport->fetch = fetch;
 	transport->disconnect = disconnect_helper;
-- 
1.6.5.52.g0ff2e

Re: [RFC PATCH v2 08/16] remote-helpers: Support custom transport options

[Daniel Barkalow]

On Mon, 12 Oct 2009, Shawn O. Pearce wrote:

> Some transports, like the native pack transport implemented by
> fetch-pack, support useful features like depth or include tags.
> These should be exposed if the underlying helper knows how to
> use them and is based upon the same infrastructure.
>
> Helpers must advertise the options they support, any attempt
> to set an unsupported option will cause a failure.
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> CC: Daniel Barkalow <barkalow@iabervon.org>
> ---
>  Documentation/git-remote-helpers.txt |   20 ++++++++++
>  remote-curl.c                        |   16 ++++++-
>  transport-helper.c                   |   70 ++++++++++++++++++++++++++++++++++
>  3 files changed, 103 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
> index e10ce99..334ab30 100644
> --- a/Documentation/git-remote-helpers.txt
> +++ b/Documentation/git-remote-helpers.txt
> @@ -46,6 +46,7 @@ Supported if the helper has the "fetch" capability.
>  'fetch-multiple'::
>  	Fetches multiple objects at once.  The fetch-multiple
>  	command is followed by one or more 'fetch' lines as above,
> +	zero or more 'option' lines for the supported options,
>  	and then a blank line to terminate the batch.  Outputs a
>  	single blank line when the entire batch is complete.
>  	Optionally may output a 'lock <file>' line indicating a
> @@ -69,6 +70,9 @@ CAPABILITIES
>  'fetch-multiple'::
>  	This helper supports the 'fetch-multiple' command.
>  
> +'option' <name>::
> +	This helper supports the option <name> under fetch-multiple.
> +

I'm a bit surprised that the options only apply in a fetch-multiple 
section, rather than getting set at the beginning and applying to 
everything for that run. At least, I think an "option" command should be 
useable outside of a fetch-multiple (or possible future grouping 
construct) and have global scope.

>  REF LIST ATTRIBUTES
>  -------------------
>  
> @@ -76,10 +80,26 @@ None are defined yet, but the caller must accept any which are supplied.
>  
>  FETCH OPTIONS
>  -------------
> +To enable an option the helper must list it in 'capabilities'.
>  
>  'option verbose'::
>  	Print more verbose activity messages to stderr.

I think you mis-split the above part; your previoud patch declared this 
option without declaring any way to use it. Might be worth allowing 
multiple "verboses" and "quiet" or "option verbosity quiet"/"option 
verbosity verbose verbose".

> +'option uploadpack' <command>::
> +	The program to use on the remote side to generate a pack.

I sort of feel like the helper ought to read this one out of the config 
file itself if it wants it. In general, it would be good to have 
transport.c and remote.c out of the business of knowing this sort of 
protocol-specific (albiet specific now to two protocols) information. (Of 
course, the native protocol's transport methods are in transport.c, so 
that's there, but I'd like to move that to a transport-native.c someday.)

> +'option depth' <depth>::
> +	Deepen the history of a shallow repository.
> +
> +'option keep'::
> +	Keep the transferred pack(s) with .keep files.
> +
> +'option followtags'::
> +	Aggressively fetch annotated tags if possible.

I assume this means to fetch tags which annotate objects we have or are 
fetching? (As opposed to fetching any annotated tag we could possibly 
fetch, even if we don't otherwise care about the tag or the thing it 
tags.) It's obvious in the context of git's config options, but I'd like 
this document to avoid assuming that context, and the option could apply 
more generally.

> +
> +'option thin'::
> +	Transfer the data as a thin pack if possible.

Does anyone still use non-default thinness? 

	-Daniel
*This .sig left intentionally blank*

Re: [RFC PATCH v2 08/16] remote-helpers: Support custom transport options

[Shawn O. Pearce]

Daniel Barkalow <barkalow@iabervon.org> wrote:
> > diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
> > +'option' <name>::
> > +	This helper supports the option <name> under fetch-multiple.
> > +
> 
> I'm a bit surprised that the options only apply in a fetch-multiple 
> section, rather than getting set at the beginning and applying to 
> everything for that run. At least, I think an "option" command should be 
> useable outside of a fetch-multiple (or possible future grouping 
> construct) and have global scope.

In hindsight, I agree with you.

I'll respin the series so the set_option method in the transport
forwards the options immediately to the helper and lets the helper
decide whether it accepts or rejects the option string.  This will
clean up the capabilities interface since we no longer need to dump
the list of options we support in the helper, and as you point out,
it will make a lot more sense to just set the options for this
transport instance.
 
> >  REF LIST ATTRIBUTES
> >  -------------------
> >  
> > @@ -76,10 +80,26 @@ None are defined yet, but the caller must accept any which are supplied.
> >  
> >  FETCH OPTIONS
> >  -------------
> > +To enable an option the helper must list it in 'capabilities'.
> >  
> >  'option verbose'::
> >  	Print more verbose activity messages to stderr.
> 
> I think you mis-split the above part; your previoud patch declared this 
> option without declaring any way to use it. Might be worth allowing 
> multiple "verboses" and "quiet" or "option verbosity quiet"/"option 
> verbosity verbose verbose".

Hmmph.  "option verbosity verbose verbose" is a bit verbose, don't
you think?  :-)

I think we should just forward the verbosity setting from the
frontend: "option verbosity [0-n]" where n is the number of
times -v appeared on the command line/how verbose the user wants.
 
> > +'option uploadpack' <command>::
> > +	The program to use on the remote side to generate a pack.
> 
> I sort of feel like the helper ought to read this one out of the config 
> file itself if it wants it.

Eh, true, but you can also set this on the command line.  An open
question I still have for myself is how to set this in HTTP
transports.

The reason why I care is Gerrit Code Review has overloaded the
'git-receive-pack' executable and taught it more command line flags:

  $ ssh r git receive-pack -h
  git receive-pack PROJECT.git [--cc EMAIL ...] [--help (-h)] [--reviewer (--re) EMAIL ...]

   PROJECT.git             : project name
   --cc EMAIL              : CC user on change(s)
   --help (-h)             : display this help text
   --reviewer (--re) EMAIL : request reviewer for change(s)

Which is typically invoked as:

  git push --receive-pack "git-receive-pack --reviewer spearce@spearce.org" URL REFSPEC

Folks actually have scripts which make this invocation for them, so
they can insert in the proper reviewer and/or cc arguments.  Since
the arguments vary its hard to set this up in the configuration file.

Over SSH this is fine, we obtain the arguments off the SSH command
line string and its no big deal.  Over git:// this would fail as
git-daemon can't parse the line anymore.  Over HTTP this also is not
going to work since the service can't receive arbitrary arguements.

My primary motivator for doing smart HTTP now is folks who are
stuck behind firewalls that permit only HTTP through their local
proxy servers are unable to communicate with a Gerrit Code Review
instance over SSH on port 29418.  That --reviewer flag above is a
very useful feature of Gerrit that I somehow have to support for
the HTTP transport too.

I started down the road of currying this data into the backend by
at least exposing the option to the helper.  How the helper reads
and uses it is up to the helper.

But given that the value can come in from the command line or from
the configuration file, I think this should be handled by fetch
or push porcelain and fed through the helper protocol, and not be
something that the helper reads from the config directly.

> In general, it would be good to have 
> transport.c and remote.c out of the business of knowing this sort of 
> protocol-specific (albiet specific now to two protocols) information. (Of 
> course, the native protocol's transport methods are in transport.c, so 
> that's there, but I'd like to move that to a transport-native.c someday.)

Agreed, but I have no solution for you due to the --receive-pack
and --upload-pack arguments supported by the command line git push
and git fetch/pull porcelain.

But I have been trying to extend the helper interface in a way
that would allow us to eject the native transport code entirely
into a helper.  We may never bother, there are some advantages to
being in the push/fetch client process, but I also didn't want to
get stuck in a corner.

I think with my series we do almost everything we need to support
native git:// in an external helper process rather than builtin.
We honor the pack lock file system used by fetch to maintain safe
concurrent mutations.  We use push_refs API and signal back the
complete information from the remote side.  We permit arbitrary
message strings per ref to be returned by the helper.  Etc.
 
> > +'option followtags'::
> > +	Aggressively fetch annotated tags if possible.
> 
> I assume this means to fetch tags which annotate objects we have or are 
> fetching? (As opposed to fetching any annotated tag we could possibly 
> fetch, even if we don't otherwise care about the tag or the thing it 
> tags.) It's obvious in the context of git's config options, but I'd like 
> this document to avoid assuming that context, and the option could apply 
> more generally.

Yes.  I'll extend the documentation further in the next iteration.
 
> > +'option thin'::
> > +	Transfer the data as a thin pack if possible.
> 
> Does anyone still use non-default thinness? 

Its a command line option on the porcelain.  Until we remove
the command line flag I think we should still try to honor it
in implementations that understand that notion.

-- 
Shawn.

Re: [RFC PATCH v2 08/16] remote-helpers: Support custom transport options

[Daniel Barkalow]

On Tue, 13 Oct 2009, Shawn O. Pearce wrote:

> Daniel Barkalow <barkalow@iabervon.org> wrote:
> > > diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
> > > +'option' <name>::
> > > +	This helper supports the option <name> under fetch-multiple.
> > > +
> > 
> > I'm a bit surprised that the options only apply in a fetch-multiple 
> > section, rather than getting set at the beginning and applying to 
> > everything for that run. At least, I think an "option" command should be 
> > useable outside of a fetch-multiple (or possible future grouping 
> > construct) and have global scope.
> 
> In hindsight, I agree with you.
> 
> I'll respin the series so the set_option method in the transport
> forwards the options immediately to the helper and lets the helper
> decide whether it accepts or rejects the option string.  This will
> clean up the capabilities interface since we no longer need to dump
> the list of options we support in the helper, and as you point out,
> it will make a lot more sense to just set the options for this
> transport instance.

Great. That also makes the "fetch-multiple" start flag no longer 
strictly necessary.

> > >  REF LIST ATTRIBUTES
> > >  -------------------
> > >  
> > > @@ -76,10 +80,26 @@ None are defined yet, but the caller must accept any which are supplied.
> > >  
> > >  FETCH OPTIONS
> > >  -------------
> > > +To enable an option the helper must list it in 'capabilities'.
> > >  
> > >  'option verbose'::
> > >  	Print more verbose activity messages to stderr.
> > 
> > I think you mis-split the above part; your previoud patch declared this 
> > option without declaring any way to use it. Might be worth allowing 
> > multiple "verboses" and "quiet" or "option verbosity quiet"/"option 
> > verbosity verbose verbose".
> 
> Hmmph.  "option verbosity verbose verbose" is a bit verbose, don't
> you think?  :-)
> 
> I think we should just forward the verbosity setting from the
> frontend: "option verbosity [0-n]" where n is the number of
> times -v appeared on the command line/how verbose the user wants.

I think it can go down to -1. Also, I remember needing to have a "-v" 
get eaten by the fetch/pull itself, so that you can have fetch/pull tell 
you stuff without having the actual protocol interaction getting more 
verbose, so it's not exactly the same numbers that get through.

> > > +'option uploadpack' <command>::
> > > +	The program to use on the remote side to generate a pack.
> > 
> > I sort of feel like the helper ought to read this one out of the config 
> > file itself if it wants it.
> 
> Eh, true, but you can also set this on the command line.  An open
> question I still have for myself is how to set this in HTTP
> transports.

Good point. I doubt people really want to change the executable name for 
the same remote between different runs, though.

> The reason why I care is Gerrit Code Review has overloaded the
> 'git-receive-pack' executable and taught it more command line flags:
> 
>   $ ssh r git receive-pack -h
>   git receive-pack PROJECT.git [--cc EMAIL ...] [--help (-h)] [--reviewer (--re) EMAIL ...]
> 
>    PROJECT.git             : project name
>    --cc EMAIL              : CC user on change(s)
>    --help (-h)             : display this help text
>    --reviewer (--re) EMAIL : request reviewer for change(s)
> 
> Which is typically invoked as:
> 
>   git push --receive-pack "git-receive-pack --reviewer spearce@spearce.org" URL REFSPEC

It seems to me like what we really want is a channel to communicate extra 
information to hooks on the server side (or a modified command on the 
server). Like:

git push --recv-opt "--reviewer spearce@spearce.org"

or something like that. This would arrange to pass that option to whatever 
server-side code is responsible for accepting pushes, without specifying 
how it gets there.

> Folks actually have scripts which make this invocation for them, so
> they can insert in the proper reviewer and/or cc arguments.  Since
> the arguments vary its hard to set this up in the configuration file.
> 
> Over SSH this is fine, we obtain the arguments off the SSH command
> line string and its no big deal.  Over git:// this would fail as
> git-daemon can't parse the line anymore.  Over HTTP this also is not
> going to work since the service can't receive arbitrary arguements.
> 
> My primary motivator for doing smart HTTP now is folks who are
> stuck behind firewalls that permit only HTTP through their local
> proxy servers are unable to communicate with a Gerrit Code Review
> instance over SSH on port 29418.  That --reviewer flag above is a
> very useful feature of Gerrit that I somehow have to support for
> the HTTP transport too.
> 
> I started down the road of currying this data into the backend by
> at least exposing the option to the helper.  How the helper reads
> and uses it is up to the helper.
> 
> But given that the value can come in from the command line or from
> the configuration file, I think this should be handled by fetch
> or push porcelain and fed through the helper protocol, and not be
> something that the helper reads from the config directly.

I agree that the porcelain and transport.c should handle "--reviewer 
spearce@spearce.org", but I still think they shouldn't handle the
"git-receive-pack" part, and it would probably be easier on everyone if it 
was separated in the porcelain, and the native transport knew to stick it 
into the receive-pack command line.

> > In general, it would be good to have 
> > transport.c and remote.c out of the business of knowing this sort of 
> > protocol-specific (albiet specific now to two protocols) information. (Of 
> > course, the native protocol's transport methods are in transport.c, so 
> > that's there, but I'd like to move that to a transport-native.c someday.)
> 
> Agreed, but I have no solution for you due to the --receive-pack
> and --upload-pack arguments supported by the command line git push
> and git fetch/pull porcelain.
> 
> But I have been trying to extend the helper interface in a way
> that would allow us to eject the native transport code entirely
> into a helper.  We may never bother, there are some advantages to
> being in the push/fetch client process, but I also didn't want to
> get stuck in a corner.
> 
> I think with my series we do almost everything we need to support
> native git:// in an external helper process rather than builtin.
> We honor the pack lock file system used by fetch to maintain safe
> concurrent mutations.  We use push_refs API and signal back the
> complete information from the remote side.  We permit arbitrary
> message strings per ref to be returned by the helper.  Etc.

I think it would be a worthwhile exercise to actually write the series to 
eject all of the transports into helpers. Then I think we should probably 
make the extensions to the helper code this requires, eject the rsync one, 
and put bundles and native in contrib instead of using them.

> > > +'option followtags'::
> > > +	Aggressively fetch annotated tags if possible.
> > 
> > I assume this means to fetch tags which annotate objects we have or are 
> > fetching? (As opposed to fetching any annotated tag we could possibly 
> > fetch, even if we don't otherwise care about the tag or the thing it 
> > tags.) It's obvious in the context of git's config options, but I'd like 
> > this document to avoid assuming that context, and the option could apply 
> > more generally.
> 
> Yes.  I'll extend the documentation further in the next iteration.
>  
> > > +'option thin'::
> > > +	Transfer the data as a thin pack if possible.
> > 
> > Does anyone still use non-default thinness? 
> 
> Its a command line option on the porcelain.  Until we remove
> the command line flag I think we should still try to honor it
> in implementations that understand that notion.

Actually, the command line supports turning it on, and it defaults to on. 
So I think your helper can safely assume that it's on. :)

	-Daniel
*This .sig left intentionally blank*

Re: [RFC PATCH v2 08/16] remote-helpers: Support custom transport options

[Shawn O. Pearce]

Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Tue, 13 Oct 2009, Shawn O. Pearce wrote:
> > > > +'option thin'::
> > > > +	Transfer the data as a thin pack if possible.
> > > 
> > > Does anyone still use non-default thinness? 
> > 
> > Its a command line option on the porcelain.
> 
> Actually, the command line supports turning it on, and it defaults to on. 
> So I think your helper can safely assume that it's on. :)

For fetch it defaults to "on", but for push I think it defaults
to "off".  Turning it on when pushing on a low bandwidth network
connection might actually be useful to an end-user.

-- 
Shawn.

Re: [RFC PATCH v2 08/16] remote-helpers: Support custom transport options

[Daniel Barkalow]

On Tue, 13 Oct 2009, Shawn O. Pearce wrote:

> Daniel Barkalow <barkalow@iabervon.org> wrote:
> > On Tue, 13 Oct 2009, Shawn O. Pearce wrote:
> > > > > +'option thin'::
> > > > > +	Transfer the data as a thin pack if possible.
> > > > 
> > > > Does anyone still use non-default thinness? 
> > > 
> > > Its a command line option on the porcelain.
> > 
> > Actually, the command line supports turning it on, and it defaults to on. 
> > So I think your helper can safely assume that it's on. :)
> 
> For fetch it defaults to "on", but for push I think it defaults
> to "off".  Turning it on when pushing on a low bandwidth network
> connection might actually be useful to an end-user.

Nope, on ~line 849 of transport.c, it gets set for all native-transport 
handlers, and never gets turned off. Looks like a misconversion 2 years 
ago defaulting "data->thin" to 1 instead of 0, but it seems not to have 
caused problems.

	-Daniel
*This .sig left intentionally blank*

Re: [RFC PATCH v2 08/16] remote-helpers: Support custom transport options

[Shawn O. Pearce]

Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Tue, 13 Oct 2009, Shawn O. Pearce wrote:
> > 
> > For fetch it defaults to "on", but for push I think it defaults
> > to "off".
> 
> Nope, on ~line 849 of transport.c, it gets set for all native-transport 
> handlers, and never gets turned off. Looks like a misconversion 2 years 
> ago defaulting "data->thin" to 1 instead of 0, but it seems not to have 
> caused problems.

The rationale for why it was off by default on push was to avoid
making suboptimal packs on the server due to needing to complete
the thin pack out.  If its been off for 2 years I guess its not
that big of a deal.  We can probably leave thin out then and just
start deprecating the option.


Actually... right now I'm more concerned about why a negotiation
over the native protocol is making the client mention "have v1.5.5"
(yes, the tag object) when I'm trying to fetch between two git.git
repositories that are only 1 Junio work day apart.

In the grand scheme of things over TCP or SSH, that's a few extra
have lines.  Over the stateless HTTP its why the final request is
bloating out to 8 KiB worth of have lines.  In one Junio work day
we should only be seeing a few hundred new objects, yea pu rewinds,
but v1.5.5 isn't the best common base available to us... IMHO we
shouldn't have even mentioned it.

But fixing this is independent of smart HTTP, I'll try to circle
back later and look at why I'm seeing this oddity in the common
commit negotiation.

-- 
Shawn.

[RFC PATCH v2 09/16] Move WebDAV HTTP push under remote-curl

[Shawn O. Pearce]

The remote helper interface now supports the push capability,
which can be used to ask the implementation to push one or more
specs to the remote repository.  For remote-curl we implement this
by calling the existing WebDAV based git-http-push executable.

Internally the helper interface uses the push_refs transport hook
so that the complexity of the refspec parsing and matching can be
reused between remote implementations.  When possible however the
helper protocol uses source ref name rather than the source SHA-1,
thereby allowing the helper to access this name if it is useful.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
CC: Daniel Barkalow <barkalow@iabervon.org>
CC: Tay Ray Chuan <rctay89@gmail.com>
CC: Mike Hommey <mh@glandium.org>
---
 Documentation/git-remote-helpers.txt |   43 ++++++++++-
 http-push.c                          |   43 ++++++++---
 remote-curl.c                        |  101 ++++++++++++++++++++++---
 transport-helper.c                   |  140 +++++++++++++++++++++++++++++++++-
 transport.c                          |   31 --------
 5 files changed, 302 insertions(+), 56 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 334ab30..f8234d0 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -34,6 +34,10 @@ Commands are given by the caller on the helper's standard input, one per line.
 	value of the ref. A space-separated list of attributes follows
 	the name; unrecognized attributes are ignored. After the
 	complete list, outputs a blank line.
++
+If 'push' is supported this may be called as 'list for-push'
+to obtain the current refs prior to sending one or more 'push'
+commands to the helper.
 
 'fetch' <sha1> <name>::
 	Fetches the given object, writing the necessary objects to the
@@ -53,6 +57,22 @@ Supported if the helper has the "fetch" capability.
 	file under GIT_DIR/objects/pack which is keeping a pack
 	until refs can be suitably updated.
 
+'push' +<src>:<dst>::
+	Pushes the given <src> commit or branch locally to the
+	remote branch described by <dst>.  A batch sequence of
+	one or more push commands is terminated with a blank line.
++
+Zero or more protocol options may be entered after the last 'push'
+command, before the batch's terminating blank line.
++
+When the push is complete, outputs one or more 'ok <dst>' or
+'error <dst> <why>?' lines to indicate success or failure of
+each pushed ref.  The status report output is terminated by
+a blank line.  The option field <why> may be quoted in a C
+style string if it contains an LF.
++
+Supported if the helper has the "push" capability.
+
 If a fatal error occurs, the program writes the error message to
 stderr and exits. The caller should expect that a suitable error
 message has been printed if the child closes the connection without
@@ -70,8 +90,12 @@ CAPABILITIES
 'fetch-multiple'::
 	This helper supports the 'fetch-multiple' command.
 
+'push'::
+	This helper supports the 'push' command.
+
 'option' <name>::
-	This helper supports the option <name> under fetch-multiple.
+	This helper supports the option <name> under fetch-multiple
+	and push.
 
 REF LIST ATTRIBUTES
 -------------------
@@ -100,6 +124,23 @@ To enable an option the helper must list it in 'capabilities'.
 'option thin'::
 	Transfer the data as a thin pack if possible.
 
+PUSH OPTIONS
+------------
+
+'option dry-run':
+	Pretend like the push update will take place, but don't
+	actually perform actions which would modify the state of
+	the remote side.
+
+'option verbose':
+	Be more verbose in progress output to stderr.
+
+'option thin'::
+	Transfer the data as a thin pack if possible.
+
+'option receivepack' <command>::
+	The program to use on the remote side to receive a pack.
+
 Documentation
 -------------
 Documentation by Daniel Barkalow.
diff --git a/http-push.c b/http-push.c
index 00e83dc..9010ccc 100644
--- a/http-push.c
+++ b/http-push.c
@@ -78,6 +78,7 @@ static int push_verbosely;
 static int push_all = MATCH_REFS_NONE;
 static int force_all;
 static int dry_run;
+static int helper_status;
 
 static struct object_list *objects;
 
@@ -1813,6 +1814,10 @@ int main(int argc, char **argv)
 				dry_run = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--helper-status")) {
+				helper_status = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--verbose")) {
 				push_verbosely = 1;
 				http_is_verbose = 1;
@@ -1941,9 +1946,14 @@ int main(int argc, char **argv)
 
 		if (is_null_sha1(ref->peer_ref->new_sha1)) {
 			if (delete_remote_branch(ref->name, 1) == -1) {
-				error("Could not remove %s", ref->name);
+				if (helper_status)
+					printf("error %s cannot remove\n", ref->name);
+				else
+					error("Could not remove %s", ref->name);
 				rc = -4;
 			}
+			else if (helper_status)
+				printf("ok %s\n", ref->name);
 			new_refs++;
 			continue;
 		}
@@ -1951,6 +1961,8 @@ int main(int argc, char **argv)
 		if (!hashcmp(ref->old_sha1, ref->peer_ref->new_sha1)) {
 			if (push_verbosely || 1)
 				fprintf(stderr, "'%s': up-to-date\n", ref->name);
+			if (helper_status)
+				printf("ok %s up to date\n", ref->name);
 			continue;
 		}
 
@@ -1968,12 +1980,15 @@ int main(int argc, char **argv)
 				 * commits at the remote end and likely
 				 * we were not up to date to begin with.
 				 */
-				error("remote '%s' is not an ancestor of\n"
-				      "local '%s'.\n"
-				      "Maybe you are not up-to-date and "
-				      "need to pull first?",
-				      ref->name,
-				      ref->peer_ref->name);
+				if (helper_status)
+					printf("error %s non-fast forward\n", ref->name);
+				else
+					error("remote '%s' is not an ancestor of\n"
+						  "local '%s'.\n"
+						  "Maybe you are not up-to-date and "
+						  "need to pull first?",
+						  ref->name,
+						  ref->peer_ref->name);
 				rc = -2;
 				continue;
 			}
@@ -1987,14 +2002,20 @@ int main(int argc, char **argv)
 		if (strcmp(ref->name, ref->peer_ref->name))
 			fprintf(stderr, " using '%s'", ref->peer_ref->name);
 		fprintf(stderr, "\n  from %s\n  to   %s\n", old_hex, new_hex);
-		if (dry_run)
+		if (dry_run) {
+			if (helper_status)
+				printf("ok %s\n", ref->name);
 			continue;
+		}
 
 		/* Lock remote branch ref */
 		ref_lock = lock_remote(ref->name, LOCK_TIME);
 		if (ref_lock == NULL) {
-			fprintf(stderr, "Unable to lock remote branch %s\n",
-				ref->name);
+			if (helper_status)
+				printf("error %s lock error\n", ref->name);
+			else
+				fprintf(stderr, "Unable to lock remote branch %s\n",
+					ref->name);
 			rc = 1;
 			continue;
 		}
@@ -2045,6 +2066,8 @@ int main(int argc, char **argv)
 
 		if (!rc)
 			fprintf(stderr, "    done\n");
+		if (helper_status)
+			printf("%s %s\n", !rc ? "ok" : "error", ref->name);
 		unlock_remote(ref_lock);
 		check_locks();
 	}
diff --git a/remote-curl.c b/remote-curl.c
index e5d9768..000bb52 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -1,8 +1,11 @@
 #include "cache.h"
+#include "exec_cmd.h"
 #include "remote.h"
 #include "strbuf.h"
 #include "walker.h"
 #include "http.h"
+#include "run-command.h"
+
 
 static struct remote *remote;
 static const char *url;
@@ -86,6 +89,20 @@ static struct ref *get_refs(void)
 	return refs;
 }
 
+static void output_refs(struct ref *refs)
+{
+	struct ref *posn;
+	for (posn = refs; posn; posn = posn->next) {
+		if (posn->symref)
+			printf("@%s %s\n", posn->symref, posn->name);
+		else
+			printf("%s %s\n", sha1_to_hex(posn->old_sha1), posn->name);
+	}
+	printf("\n");
+	fflush(stdout);
+	free_refs(refs);
+}
+
 struct fetch_args {
 	unsigned verbose : 1;
 };
@@ -173,10 +190,74 @@ static void parse_fetch(struct strbuf *buf, int multiple)
 	strbuf_reset(buf);
 }
 
+struct push_args {
+	unsigned dry_run : 1,
+		verbose : 1;
+};
+
+static int push_dav(struct push_args *args, int nr_spec, char **specs)
+{
+	const char **argv = xmalloc((10 + nr_spec) * sizeof(char*));
+	int argc = 0, i;
+
+	argv[argc++] = "http-push";
+	argv[argc++] = "--helper-status";
+	if (args->dry_run)
+		argv[argc++] = "--dry-run";
+	if (args->verbose)
+		argv[argc++] = "--verbose";
+	argv[argc++] = url;
+	for (i = 0; i < nr_spec; i++)
+		argv[argc++] = specs[i];
+	argv[argc++] = NULL;
+
+	if (run_command_v_opt(argv, RUN_GIT_CMD))
+		die("git-%s failed", argv[0]);
+	free(argv);
+	return 0;
+}
+
+static void parse_push(struct strbuf *buf)
+{
+	char **specs = NULL;
+	int alloc_spec = 0, nr_spec = 0, i;
+	struct push_args args;
+
+	memset(&args, 0, sizeof(args));
+	do {
+		if (!prefixcmp(buf->buf, "push ")) {
+			ALLOC_GROW(specs, nr_spec + 1, alloc_spec);
+			specs[nr_spec++] = xstrdup(buf->buf + 5);
+
+		} else if (!strcmp(buf->buf, "option dry-run"))
+			args.dry_run = 1;
+		else if (!strcmp(buf->buf, "option verbose"))
+			args.verbose = 1;
+		else
+			die("http transport does not support %s", buf->buf);
+
+		strbuf_reset(buf);
+		if (strbuf_getline(buf, stdin, '\n') == EOF)
+			return;
+		if (!*buf->buf)
+			break;
+	} while (1);
+
+	if (push_dav(&args, nr_spec, specs))
+		exit(128); /* error already reported */
+	for (i = 0; i < nr_spec; i++)
+		free(specs[i]);
+	free(specs);
+
+	printf("\n");
+	fflush(stdout);
+}
+
 int main(int argc, const char **argv)
 {
 	struct strbuf buf = STRBUF_INIT;
 
+	git_extract_argv0_path(argv[0]);
 	setup_git_directory();
 	if (argc < 2) {
 		fprintf(stderr, "Remote needed\n");
@@ -204,19 +285,19 @@ int main(int argc, const char **argv)
 			parse_fetch(&buf, 1);
 
 		} else if (!strcmp(buf.buf, "list")) {
-			struct ref *refs = get_refs();
-			struct ref *posn;
-			for (posn = refs; posn; posn = posn->next) {
-				if (posn->symref)
-					printf("@%s %s\n", posn->symref, posn->name);
-				else
-					printf("%s %s\n", sha1_to_hex(posn->old_sha1), posn->name);
-			}
-			printf("\n");
-			fflush(stdout);
+			output_refs(get_refs());
+
+		} else if (!strcmp(buf.buf, "list for-push")) {
+			output_refs(get_refs());
+
+		} else if (!prefixcmp(buf.buf, "push ")) {
+			parse_push(&buf);
+
 		} else if (!strcmp(buf.buf, "capabilities")) {
 			printf("fetch\n");
 			printf("fetch-multiple\n");
+			printf("push\n");
+			printf("option dry-run\n");
 			printf("option verbose\n");
 			printf("\n");
 			fflush(stdout);
diff --git a/transport-helper.c b/transport-helper.c
index bb6cd1b..60fdb16 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1,6 +1,6 @@
 #include "cache.h"
 #include "transport.h"
-
+#include "quote.h"
 #include "run-command.h"
 #include "commit.h"
 #include "diff.h"
@@ -15,7 +15,8 @@ struct helper_data
 	FILE *out;
 	struct string_list options;
 	unsigned fetch : 1,
-		fetch_multiple : 1;
+		fetch_multiple : 1,
+		push : 1;
 };
 
 static struct child_process *get_helper(struct transport *transport)
@@ -54,6 +55,8 @@ static struct child_process *get_helper(struct transport *transport)
 			data->fetch = 1;
 		if (!strcmp(buf.buf, "fetch-multiple"))
 			data->fetch_multiple = 1;
+		if (!strcmp(buf.buf, "push"))
+			data->push = 1;
 		if (!prefixcmp(buf.buf, "option ")) {
 			const char *name = buf.buf + strlen("option ");
 			if (!string_list_lookup(name, &data->options))
@@ -103,7 +106,7 @@ static int set_helper_option(struct transport *transport,
 
 	get_helper(transport);
 
-	if (!data->fetch_multiple)
+	if (!data->fetch_multiple && !data->push)
 		return 1;
 
 	if (!strcmp(name, TRANS_OPT_THIN))
@@ -211,6 +214,131 @@ static int fetch(struct transport *transport,
 	return -1;
 }
 
+static int push_refs(struct transport *transport,
+		struct ref *remote_refs, int flags)
+{
+	int force_all = flags & TRANSPORT_PUSH_FORCE;
+	int mirror = flags & TRANSPORT_PUSH_MIRROR;
+	struct helper_data *data = transport->data;
+	struct strbuf buf = STRBUF_INIT;
+	struct child_process *helper;
+	struct ref *ref;
+
+	if (!remote_refs)
+		return 0;
+
+	helper = get_helper(transport);
+	if (!data->push)
+		return 1;
+
+	for (ref = remote_refs; ref; ref = ref->next) {
+		if (ref->peer_ref)
+			hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
+		else if (!mirror)
+			continue;
+
+		ref->deletion = is_null_sha1(ref->new_sha1);
+		if (!ref->deletion &&
+			!hashcmp(ref->old_sha1, ref->new_sha1)) {
+			ref->status = REF_STATUS_UPTODATE;
+			continue;
+		}
+
+		if (force_all)
+			ref->force = 1;
+
+		strbuf_addstr(&buf, "push ");
+		if (!ref->deletion) {
+			if (ref->force)
+				strbuf_addch(&buf, '+');
+			if (ref->peer_ref)
+				strbuf_addstr(&buf, ref->peer_ref->name);
+			else
+				strbuf_addstr(&buf, sha1_to_hex(ref->new_sha1));
+		}
+		strbuf_addch(&buf, ':');
+		strbuf_addstr(&buf, ref->name);
+		strbuf_addch(&buf, '\n');
+	}
+
+	standard_options(transport);
+	if (flags & TRANSPORT_PUSH_DRY_RUN) {
+		if (save_option(transport, "dry-run", ""))
+			die("helper %s does not support dry-run", data->name);
+	}
+	if (flags & TRANSPORT_PUSH_VERBOSE)
+		save_option(transport, "verbose", "");
+	for_each_string_list(print_options, &data->options, &buf);
+	strbuf_addch(&buf, '\n');
+
+	if (write_in_full(helper->in, buf.buf, buf.len) != buf.len)
+		exit(128);
+
+	ref = remote_refs;
+	while (1) {
+		char *refname, *msg;
+		int status;
+
+		strbuf_reset(&buf);
+		if (strbuf_getline(&buf, data->out, '\n') == EOF)
+			exit(128); /* child died, message supplied already */
+		if (!buf.len)
+			break;
+
+		if (!prefixcmp(buf.buf, "ok ")) {
+			status = REF_STATUS_OK;
+			refname = buf.buf + 3;
+		} else if (!prefixcmp(buf.buf, "error ")) {
+			status = REF_STATUS_REMOTE_REJECT;
+			refname = buf.buf + 6;
+		} else
+			die("expected ok/error, helper said '%s'\n", buf.buf);
+
+		msg = strchr(refname, ' ');
+		if (msg) {
+			struct strbuf msg_buf = STRBUF_INIT;
+			const char *end;
+
+			*msg++ = '\0';
+			if (!unquote_c_style(&msg_buf, msg, &end))
+				msg = strbuf_detach(&msg_buf, NULL);
+			else
+				msg = xstrdup(msg);
+			strbuf_release(&msg_buf);
+
+			if (!strcmp(msg, "no match")) {
+				status = REF_STATUS_NONE;
+				free(msg);
+				msg = NULL;
+			}
+			else if (!strcmp(msg, "up to date")) {
+				status = REF_STATUS_UPTODATE;
+				free(msg);
+				msg = NULL;
+			}
+			else if (!strcmp(msg, "non-fast forward")) {
+				status = REF_STATUS_REJECT_NONFASTFORWARD;
+				free(msg);
+				msg = NULL;
+			}
+		}
+
+		if (ref)
+			ref = find_ref_by_name(ref, refname);
+		if (!ref)
+			ref = find_ref_by_name(remote_refs, refname);
+		if (!ref) {
+			warning("helper reported unexpected status of %s", refname);
+			continue;
+		}
+
+		ref->status = status;
+		ref->remote_status = msg;
+	}
+	strbuf_release(&buf);
+	return 0;
+}
+
 static struct ref *get_refs_list(struct transport *transport, int for_push)
 {
 	struct helper_data *data = transport->data;
@@ -222,7 +350,10 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
 
 	helper = get_helper(transport);
 
-	write_str_in_full(helper->in, "list\n");
+	if (data->push && for_push)
+		write_str_in_full(helper->in, "list for-push\n");
+	else
+		write_str_in_full(helper->in, "list\n");
 
 	while (1) {
 		char *eov, *eon;
@@ -263,6 +394,7 @@ int transport_helper_init(struct transport *transport, const char *name)
 	transport->set_option = set_helper_option;
 	transport->get_refs_list = get_refs_list;
 	transport->fetch = fetch;
+	transport->push_refs = push_refs;
 	transport->disconnect = disconnect_helper;
 	return 0;
 }
diff --git a/transport.c b/transport.c
index 644a30a..6d9652d 100644
--- a/transport.c
+++ b/transport.c
@@ -349,35 +349,6 @@ static int rsync_transport_push(struct transport *transport,
 	return result;
 }
 
-#ifndef NO_CURL
-static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)
-{
-	const char **argv;
-	int argc;
-
-	if (flags & TRANSPORT_PUSH_MIRROR)
-		return error("http transport does not support mirror mode");
-
-	argv = xmalloc((refspec_nr + 12) * sizeof(char *));
-	argv[0] = "http-push";
-	argc = 1;
-	if (flags & TRANSPORT_PUSH_ALL)
-		argv[argc++] = "--all";
-	if (flags & TRANSPORT_PUSH_FORCE)
-		argv[argc++] = "--force";
-	if (flags & TRANSPORT_PUSH_DRY_RUN)
-		argv[argc++] = "--dry-run";
-	if (flags & TRANSPORT_PUSH_VERBOSE)
-		argv[argc++] = "--verbose";
-	argv[argc++] = transport->url;
-	while (refspec_nr--)
-		argv[argc++] = *refspec++;
-	argv[argc] = NULL;
-	return !!run_command_v_opt(argv, RUN_GIT_CMD);
-}
-
-#endif
-
 struct bundle_transport_data {
 	int fd;
 	struct bundle_header header;
@@ -826,8 +797,6 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		transport_helper_init(ret, "curl");
 #ifdef NO_CURL
 		error("git was compiled without libcurl support.");
-#else
-		ret->push = curl_transport_push;
 #endif
 
 	} else if (is_local(url) && is_file(url)) {
-- 
1.6.5.52.g0ff2e

Re: [RFC PATCH v2 09/16] Move WebDAV HTTP push under remote-curl

[Mike Hommey]

On Mon, Oct 12, 2009 at 07:25:08PM -0700, Shawn O. Pearce wrote:
> The remote helper interface now supports the push capability,
> which can be used to ask the implementation to push one or more
> specs to the remote repository.  For remote-curl we implement this
> by calling the existing WebDAV based git-http-push executable.
> 
> Internally the helper interface uses the push_refs transport hook
> so that the complexity of the refspec parsing and matching can be
> reused between remote implementations.  When possible however the
> helper protocol uses source ref name rather than the source SHA-1,
> thereby allowing the helper to access this name if it is useful.

It's been a while I haven't followed changes in the remote code but this
looks nice, though I haven't checked thoroughly. I guess the next step
would be to kill http-push as an external program.

Mike

Re: [RFC PATCH v2 09/16] Move WebDAV HTTP push under remote-curl

[Johannes Schindelin]

Hi,

On Tue, 13 Oct 2009, Mike Hommey wrote:

> On Mon, Oct 12, 2009 at 07:25:08PM -0700, Shawn O. Pearce wrote:
> > The remote helper interface now supports the push capability, which 
> > can be used to ask the implementation to push one or more specs to the 
> > remote repository.  For remote-curl we implement this by calling the 
> > existing WebDAV based git-http-push executable.
> > 
> > Internally the helper interface uses the push_refs transport hook so 
> > that the complexity of the refspec parsing and matching can be reused 
> > between remote implementations.  When possible however the helper 
> > protocol uses source ref name rather than the source SHA-1, thereby 
> > allowing the helper to access this name if it is useful.
> 
> It's been a while I haven't followed changes in the remote code but this 
> looks nice, though I haven't checked thoroughly. I guess the next step 
> would be to kill http-push as an external program.

Ilari made signs on IRC that he got something working.  After, ahem, 
saying a few things about the design of the remote helpers.

Ciao,
Dscho

[RFC PATCH v2 10/16] Git-aware CGI to provide dumb HTTP transport

[Shawn O. Pearce]

The git-http-backend CGI can be configured into any Apache server
using ScriptAlias, such as with the following configuration:

  LoadModule cgi_module /usr/libexec/apache2/mod_cgi.so
  LoadModule alias_module /usr/libexec/apache2/mod_alias.so
  ScriptAlias /git/ /usr/libexec/git-core/git-http-backend/

Repositories are accessed via the translated PATH_INFO.

The CGI is backwards compatible with the dumb client, allowing all
older HTTP clients to continue to download repositories which are
managed by the CGI.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .gitignore     |    1 +
 Makefile       |    1 +
 http-backend.c |  261 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 263 insertions(+), 0 deletions(-)
 create mode 100644 http-backend.c

diff --git a/.gitignore b/.gitignore
index 51a37b1..353d22f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -55,6 +55,7 @@ git-get-tar-commit-id
 git-grep
 git-hash-object
 git-help
+git-http-backend
 git-http-fetch
 git-http-push
 git-imap-send
diff --git a/Makefile b/Makefile
index fea237b..271c290 100644
--- a/Makefile
+++ b/Makefile
@@ -365,6 +365,7 @@ PROGRAMS += git-show-index$X
 PROGRAMS += git-unpack-file$X
 PROGRAMS += git-upload-pack$X
 PROGRAMS += git-var$X
+PROGRAMS += git-http-backend$X
 
 # List built-in command $C whose implementation cmd_$C() is not in
 # builtin-$C.o but is linked in as part of some other command.
diff --git a/http-backend.c b/http-backend.c
new file mode 100644
index 0000000..39cfd25
--- /dev/null
+++ b/http-backend.c
@@ -0,0 +1,261 @@
+#include "cache.h"
+#include "refs.h"
+#include "pkt-line.h"
+#include "object.h"
+#include "tag.h"
+#include "exec_cmd.h"
+#include "run-command.h"
+
+static const char content_type[] = "Content-Type";
+static const char content_length[] = "Content-Length";
+
+static char buffer[1024];
+
+static const char *http_date(unsigned long time)
+{
+	return show_date(time, 0, DATE_RFC2822);
+}
+
+static void format_write(const char *fmt, ...)
+{
+	va_list args;
+	unsigned n;
+
+	va_start(args, fmt);
+	n = vsnprintf(buffer, sizeof(buffer), fmt, args);
+	va_end(args);
+	if (n >= sizeof(buffer))
+		die("protocol error: impossibly long line");
+
+	safe_write(1, buffer, n);
+}
+
+static void write_status(unsigned code, const char *msg)
+{
+	format_write("Status: %u %s\r\n", code, msg);
+}
+
+static void write_header(const char *name, const char *value)
+{
+	format_write("%s: %s\r\n", name, value);
+}
+
+static void end_headers(void)
+{
+	safe_write(1, "\r\n", 2);
+}
+
+static void write_nocache(void)
+{
+	write_header("Expires", "Fri, 01 Jan 1980 00:00:00 GMT");
+	write_header("Pragma", "no-cache");
+	write_header("Cache-Control", "no-cache, max-age=0, must-revalidate");
+}
+
+static void write_cache_forever(void)
+{
+	unsigned long now = time(NULL);
+	write_header("Date", http_date(now));
+	write_header("Expires", http_date(now + 31536000));
+	write_header("Cache-Control", "public, max-age=31536000");
+}
+
+static NORETURN void not_found(const char *err, ...)
+{
+	va_list params;
+
+	write_status(404, "Not Found");
+	write_nocache();
+	end_headers();
+
+	va_start(params, err);
+	if (err && *err) {
+		vsnprintf(buffer, sizeof(buffer), err, params);
+		fprintf(stderr, "%s\n", buffer);
+	}
+	va_end(params);
+	exit(0);
+}
+
+static void write_file(const char *the_type, const char *name)
+{
+	const char *p = git_path("%s", name);
+	int fd;
+	struct stat sb;
+	uintmax_t remaining;
+
+	fd = open(p, O_RDONLY);
+	if (fd < 0)
+		not_found("Cannot open '%s': %s", p, strerror(errno));
+	if (fstat(fd, &sb) < 0)
+		die_errno("Cannot stat '%s'", p);
+	remaining = (uintmax_t)sb.st_size;
+
+	write_header(content_type, the_type);
+	write_header("Last-Modified", http_date(sb.st_mtime));
+	format_write("Content-Length: %" PRIuMAX "\r\n", remaining);
+	end_headers();
+
+	while (remaining) {
+		ssize_t n = xread(fd, buffer, sizeof(buffer));
+		if (n < 0)
+			die_errno("Cannot read '%s'", p);
+		n = safe_write(1, buffer, n);
+		if (n <= 0)
+			break;
+	}
+	close(fd);
+}
+
+static void get_text_file(char *name)
+{
+	write_nocache();
+	write_file("text/plain; charset=utf-8", name);
+}
+
+static void get_loose_object(char *name)
+{
+	write_cache_forever();
+	write_file("application/x-git-loose-object", name);
+}
+
+static void get_pack_file(char *name)
+{
+	write_cache_forever();
+	write_file("application/x-git-packed-objects", name);
+}
+
+static void get_idx_file(char *name)
+{
+	write_cache_forever();
+	write_file("application/x-git-packed-objects-toc", name);
+}
+
+static int show_text_ref(const char *name, const unsigned char *sha1,
+	int flag, void *cb_data)
+{
+	struct object *o = parse_object(sha1);
+	if (!o)
+		return 0;
+
+	format_write("%s\t%s\n", sha1_to_hex(sha1), name);
+	if (o->type == OBJ_TAG) {
+		o = deref_tag(o, name, 0);
+		if (!o)
+			return 0;
+		format_write("%s\t%s^{}\n", sha1_to_hex(o->sha1), name);
+	}
+
+	return 0;
+}
+
+static void get_info_refs(char *arg)
+{
+	write_nocache();
+	write_header(content_type, "text/plain; charset=utf-8");
+	end_headers();
+
+	for_each_ref(show_text_ref, NULL);
+}
+
+static void get_info_packs(char *arg)
+{
+	size_t objdirlen = strlen(get_object_directory());
+	struct packed_git *p;
+
+	write_nocache();
+	write_header(content_type, "text/plain; charset=utf-8");
+	end_headers();
+
+	prepare_packed_git();
+	for (p = packed_git; p; p = p->next) {
+		if (!p->pack_local)
+			continue;
+		format_write("P %s\n", p->pack_name + objdirlen + 6);
+	}
+	safe_write(1, "\n", 1);
+}
+
+static NORETURN void die_webcgi(const char *err, va_list params)
+{
+	write_status(500, "Internal Server Error");
+	write_nocache();
+	end_headers();
+
+	vsnprintf(buffer, sizeof(buffer), err, params);
+	fprintf(stderr, "fatal: %s\n", buffer);
+	exit(0);
+}
+
+static struct service_cmd {
+	const char *method;
+	const char *pattern;
+	void (*imp)(char *);
+} services[] = {
+	{"GET", "/HEAD$", get_text_file},
+	{"GET", "/info/refs$", get_info_refs},
+	{"GET", "/objects/info/packs$", get_info_packs},
+	{"GET", "/objects/info/[^/]*$", get_text_file},
+	{"GET", "/objects/[0-9a-f]{2}/[0-9a-f]{38}$", get_loose_object},
+	{"GET", "/objects/pack/pack-[0-9a-f]{40}\\.pack$", get_pack_file},
+	{"GET", "/objects/pack/pack-[0-9a-f]{40}\\.idx$", get_idx_file}
+};
+
+int main(int argc, char **argv)
+{
+	char *dir = getenv("PATH_TRANSLATED");
+	char *input_method = getenv("REQUEST_METHOD");
+	struct service_cmd *cmd = NULL;
+	char *cmd_arg = NULL;
+	int i;
+
+	set_die_routine(die_webcgi);
+
+	if (!dir)
+		die("No PATH_TRANSLATED from server");
+	if (!input_method)
+		die("No REQUEST_METHOD from server");
+	if (!strcmp(input_method, "HEAD"))
+		input_method = "GET";
+
+	for (i = 0; i < ARRAY_SIZE(services); i++) {
+		struct service_cmd *c = &services[i];
+		regex_t re;
+		regmatch_t out[1];
+
+		if (regcomp(&re, c->pattern, REG_EXTENDED))
+			die("Bogus regex in service table: %s", c->pattern);
+		if (!regexec(&re, dir, 1, out, 0)) {
+			size_t n = out[0].rm_eo - out[0].rm_so;
+
+			if (strcmp(input_method, c->method)) {
+				const char *proto = getenv("SERVER_PROTOCOL");
+				if (proto && !strcmp(proto, "HTTP/1.1"))
+					write_status(405, "Method Not Allowed");
+				else
+					write_status(400, "Bad Request");
+				write_nocache();
+				end_headers();
+				return 0;
+			}
+
+			cmd = c;
+			cmd_arg = xmalloc(n);
+			strncpy(cmd_arg, dir + out[0].rm_so + 1, n);
+			cmd_arg[n] = '\0';
+			dir[out[0].rm_so] = 0;
+			break;
+		}
+		regfree(&re);
+	}
+
+	if (!cmd)
+		not_found("Request not supported: '%s'", dir);
+
+	setup_path();
+	if (!enter_repo(dir, 0))
+		not_found("Not a git repository: '%s'", dir);
+
+	cmd->imp(cmd_arg);
+	return 0;
+}
-- 
1.6.5.52.g0ff2e

Re: [RFC PATCH v2 10/16] Git-aware CGI to provide dumb HTTP transport

[Johannes Sixt]

Shawn O. Pearce schrieb:
> +int main(int argc, char **argv)
> +{
> +	char *dir = getenv("PATH_TRANSLATED");
> +	char *input_method = getenv("REQUEST_METHOD");
> +	struct service_cmd *cmd = NULL;
> +	char *cmd_arg = NULL;
> +	int i;
> +

Please insert here:

	git_extract_argv0_path(argv[0]);

> +	set_die_routine(die_webcgi);
> ...

-- Hannes

[RFC PATCH v2 11/16] Add one shot RPC options to upload-pack, receive-pack

[Shawn O. Pearce]

When --one-shot-rpc is passed as a command line parameter to
upload-pack or receive-pack the programs now assume they may
perform only a single read-write cycle with stdin and stdout.
This fits with the HTTP POST request processing model where a
program may read the request, write a response, and must exit.

When --advertise-refs is passed as a command line parameter only
the initial ref advertisement is output, and the program exits
immediately.  This fits with the HTTP GET request model, where
no request content is received but a response must be produced.

HTTP headers and/or environment are not processed here, but
instead are assumed to be handled by the program invoking
either service backend.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-receive-pack.c |   26 ++++++++++++++++++++------
 upload-pack.c          |   40 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index b771fe9..9e8f36f 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -615,6 +615,8 @@ static void add_alternate_refs(void)
 
 int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 {
+	int advertise_refs = 0;
+	int one_shot_rpc = 0;
 	int i;
 	char *dir = NULL;
 
@@ -623,7 +625,15 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 		const char *arg = *argv++;
 
 		if (*arg == '-') {
-			/* Do flag handling here */
+			if (!strcmp(arg, "--advertise-refs")) {
+				advertise_refs = 1;
+				continue;
+			}
+			if (!strcmp(arg, "--one-shot-rpc")) {
+				one_shot_rpc = 1;
+				continue;
+			}
+
 			usage(receive_pack_usage);
 		}
 		if (dir)
@@ -652,12 +662,16 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 		" report-status delete-refs ofs-delta " :
 		" report-status delete-refs ";
 
-	add_alternate_refs();
-	write_head_info();
-	clear_extra_refs();
+	if (advertise_refs || !one_shot_rpc) {
+		add_alternate_refs();
+		write_head_info();
+		clear_extra_refs();
 
-	/* EOF */
-	packet_flush(1);
+		/* EOF */
+		packet_flush(1);
+	}
+	if (advertise_refs)
+		return 0;
 
 	read_head_info();
 	if (commands) {
diff --git a/upload-pack.c b/upload-pack.c
index 5024b59..71318d7 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -39,6 +39,8 @@ static unsigned int timeout;
  */
 static int use_sideband;
 static int debug_fd;
+static int advertise_refs;
+static int one_shot_rpc;
 
 static void reset_timeout(void)
 {
@@ -509,6 +511,8 @@ static int get_common_commits(void)
 		if (!len) {
 			if (have_obj.nr == 0 || multi_ack)
 				packet_write(1, "NAK\n");
+			if (one_shot_rpc)
+				exit(0);
 			continue;
 		}
 		strip(line, len);
@@ -710,12 +714,32 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 	return 0;
 }
 
+static int mark_our_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+{
+	struct object *o = parse_object(sha1);
+	if (!o)
+		die("git upload-pack: cannot find object %s:", sha1_to_hex(sha1));
+	if (!(o->flags & OUR_REF)) {
+		o->flags |= OUR_REF;
+		nr_our_refs++;
+	}
+	return 0;
+}
+
 static void upload_pack(void)
 {
-	reset_timeout();
-	head_ref(send_ref, NULL);
-	for_each_ref(send_ref, NULL);
-	packet_flush(1);
+	if (advertise_refs || !one_shot_rpc) {
+		reset_timeout();
+		head_ref(send_ref, NULL);
+		for_each_ref(send_ref, NULL);
+		packet_flush(1);
+	} else {
+		head_ref(mark_our_ref, NULL);
+		for_each_ref(mark_our_ref, NULL);
+	}
+	if (advertise_refs)
+		return;
+
 	receive_needs();
 	if (want_obj.nr) {
 		get_common_commits();
@@ -737,6 +761,14 @@ int main(int argc, char **argv)
 
 		if (arg[0] != '-')
 			break;
+		if (!strcmp(arg, "--advertise-refs")) {
+			advertise_refs = 1;
+			continue;
+		}
+		if (!strcmp(arg, "--one-shot-rpc")) {
+			one_shot_rpc = 1;
+			continue;
+		}
 		if (!strcmp(arg, "--strict")) {
 			strict = 1;
 			continue;
-- 
1.6.5.52.g0ff2e

[RFC PATCH v2 12/16] Smart fetch and push over HTTP: server side

[Shawn O. Pearce]

Requests for $GIT_URL/git-receive-pack and $GIT_URL/git-upload-pack
are forwarded to the corresponding backend process by directly
executing it and leaving stdin and stdout connected to the invoking
web server.  Prior to starting the backend process the HTTP response
headers are sent, thereby freeing the backend from needing to know
about the HTTP protocol.

Requests that are encoded with Content-Encoding: gzip are
automatically inflated before being streamed into the backend.
This is primarily useful for the git-upload-pack backend, which
receives highly repetitive text data from clients that easily
compresses to 50% of its original size.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 http-backend.c |  192 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 191 insertions(+), 1 deletions(-)

diff --git a/http-backend.c b/http-backend.c
index 39cfd25..adb3256 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -77,6 +77,152 @@ static NORETURN void not_found(const char *err, ...)
 	exit(0);
 }
 
+static NORETURN void forbidden(const char *err, ...)
+{
+	va_list params;
+
+	write_status(403, "Forbidden");
+	write_nocache();
+	end_headers();
+
+	va_start(params, err);
+	if (err && *err) {
+		vsnprintf(buffer, sizeof(buffer), err, params);
+		fprintf(stderr, "%s\n", buffer);
+	}
+	va_end(params);
+	exit(0);
+}
+
+struct http_service {
+	const char *name;
+	const char *config_name;
+	int enabled;
+};
+static struct http_service *service;
+
+static struct http_service http_service[] = {
+	{ "upload-pack", "uploadpack", 1 },
+	{ "receive-pack", "receivepack", 0 },
+};
+
+static int http_config(const char *var, const char *value, void *cb)
+{
+	if (!prefixcmp(var, "http.") &&
+	    !strcmp(var + 5, service->config_name)) {
+		service->enabled = git_config_bool(var, value);
+		return 0;
+	}
+
+	/* we are not interested in parsing any other configuration here */
+	return 0;
+}
+
+static void select_service(const char *name)
+{
+	int i;
+
+	if (prefixcmp(name, "git-"))
+		forbidden("Unsupported service: '%s'", name);
+
+	for (i = 0; i < ARRAY_SIZE(http_service); i++) {
+		service = &http_service[i];
+		if (!strcmp(service->name, name + 4)) {
+			git_config(http_config, NULL);
+			if (!service->enabled)
+				forbidden("Service not enabled: '%s'", name);
+			return;
+		}
+	}
+	forbidden("Unsupported service: '%s'", name);
+}
+
+static void inflate_request(const char *prog_name, int out)
+{
+	z_stream stream;
+	unsigned char in_buf[8192];
+	unsigned char out_buf[8192];
+	unsigned long cnt = 0;
+	int ret;
+
+	memset(&stream, 0, sizeof(stream));
+	ret = inflateInit2(&stream, (15 + 16));
+	if (ret != Z_OK)
+		die("cannot start zlib inflater, zlib err %d", ret);
+
+	while (1) {
+		ssize_t n = xread(0, in_buf, sizeof(in_buf));
+		if (n <= 0)
+			die("request ended in the middle of the gzip stream");
+
+		stream.next_in = in_buf;
+		stream.avail_in = n;
+
+		while (0 < stream.avail_in) {
+			int ret;
+
+			stream.next_out = out_buf;
+			stream.avail_out = sizeof(out_buf);
+
+			ret = inflate(&stream, Z_NO_FLUSH);
+			if (ret != Z_OK && ret != Z_STREAM_END)
+				die("zlib error inflating request, result %d", ret);
+
+			n = stream.total_out - cnt;
+			if (write_in_full(out, out_buf, n) != n)
+				die("%s aborted reading request", prog_name);
+			cnt += n;
+
+			if (ret == Z_STREAM_END)
+				goto done;
+		}
+	}
+
+done:
+	inflateEnd(&stream);
+	close(out);
+}
+
+static void run_service(const char **argv)
+{
+	const char *encoding = getenv("HTTP_CONTENT_ENCODING");
+	int use_gzip = 0;
+	struct child_process cld;
+
+	if (encoding && !strcmp(encoding, "gzip"))
+		use_gzip = 1;
+	else if (encoding && !strcmp(encoding, "x-gzip"))
+		use_gzip = 1;
+
+	memset(&cld, 0, sizeof(cld));
+	cld.argv = argv;
+	if (use_gzip)
+		cld.in = -1;
+	cld.git_cmd = 1;
+	if (start_command(&cld))
+		die_errno("Cannot start %s", argv[0]);
+
+	close(1);
+	if (use_gzip)
+		inflate_request(argv[0], cld.in);
+	else
+		close(0);
+
+	if (finish_command(&cld))
+		die("%s terminated with error", argv[0]);
+}
+
+static void require_content_type(const char *need_type)
+{
+	const char *input_type = getenv("CONTENT_TYPE");
+	if (!input_type || strcmp(input_type, need_type)) {
+		write_status(415, "Unsupported Media Type");
+		write_nocache();
+		end_headers();
+		exit(0);
+	}
+}
+
 static void write_file(const char *the_type, const char *name)
 {
 	const char *p = git_path("%s", name);
@@ -151,6 +297,25 @@ static int show_text_ref(const char *name, const unsigned char *sha1,
 
 static void get_info_refs(char *arg)
 {
+	char *query = getenv("QUERY_STRING");
+
+	if (query && !prefixcmp(query, "service=")) {
+		const char *argv[] = {NULL /* service name */,
+			"--one-shot-rpc", "--advertise-refs",
+			".", NULL};
+
+		select_service(query + 8);
+
+		write_nocache();
+		format_write("%s: application/x-git-%s-advertisement\r\n",
+			content_type, service->name);
+		end_headers();
+		packet_write(1, "# service=git-%s\n", service->name);
+
+		argv[0] = service->name;
+		run_service(argv);
+	}
+
 	write_nocache();
 	write_header(content_type, "text/plain; charset=utf-8");
 	end_headers();
@@ -176,6 +341,28 @@ static void get_info_packs(char *arg)
 	safe_write(1, "\n", 1);
 }
 
+static void post_to_service(char *service_name)
+{
+	const char *argv[] = {NULL, "--one-shot-rpc", ".", NULL};
+	unsigned n;
+
+	select_service(service_name);
+
+	n = snprintf(buffer, sizeof(buffer),
+		"application/x-git-%s-request", service->name);
+	if (n >= sizeof(buffer))
+		die("impossibly long service name");
+	require_content_type(buffer);
+
+	write_nocache();
+	format_write("%s: application/x-git-%s-result\r\n",
+		content_type, service->name);
+	end_headers();
+
+	argv[0] = service->name;
+	run_service(argv);
+}
+
 static NORETURN void die_webcgi(const char *err, va_list params)
 {
 	write_status(500, "Internal Server Error");
@@ -198,7 +385,10 @@ static struct service_cmd {
 	{"GET", "/objects/info/[^/]*$", get_text_file},
 	{"GET", "/objects/[0-9a-f]{2}/[0-9a-f]{38}$", get_loose_object},
 	{"GET", "/objects/pack/pack-[0-9a-f]{40}\\.pack$", get_pack_file},
-	{"GET", "/objects/pack/pack-[0-9a-f]{40}\\.idx$", get_idx_file}
+	{"GET", "/objects/pack/pack-[0-9a-f]{40}\\.idx$", get_idx_file},
+
+	{"POST", "/git-upload-pack$", post_to_service},
+	{"POST", "/git-receive-pack$", post_to_service}
 };
 
 int main(int argc, char **argv)
-- 
1.6.5.52.g0ff2e

Re: [RFC PATCH v2 12/16] Smart fetch and push over HTTP: server side

[Johannes Sixt]

Shawn O. Pearce schrieb:
> diff --git a/http-backend.c b/http-backend.c
> index 39cfd25..adb3256 100644
> --- a/http-backend.c
> +++ b/http-backend.c

#include "run-command.h" is missing here because you added it already in
patch 10/16 unnecessarily.

> +	if (start_command(&cld))
> +		die_errno("Cannot start %s", argv[0]);
> ...
> +	if (finish_command(&cld))
> +		die("%s terminated with error", argv[0]);

start_command and finish_command already write an error message for you
that includes argv[0] and errno. You can just exit(1) here.

-- Hannes

Re: [RFC PATCH v2 12/16] Smart fetch and push over HTTP: server side

[Shawn O. Pearce]

Johannes Sixt <j.sixt@viscovery.net> wrote:
> Shawn O. Pearce schrieb:
> > diff --git a/http-backend.c b/http-backend.c
> 
> #include "run-command.h" is missing here because you added it already in
> patch 10/16 unnecessarily.
> 
> > +	if (start_command(&cld))
> > +		die_errno("Cannot start %s", argv[0]);
> > ...
> > +	if (finish_command(&cld))
> > +		die("%s terminated with error", argv[0]);
> 
> start_command and finish_command already write an error message for you
> that includes argv[0] and errno. You can just exit(1) here.

Whoops on both; thanks for the catch.

-- 
Shawn.

[RFC PATCH v2 13/16] Discover refs via smart HTTP server when available

[Shawn O. Pearce]

Instead of loading the cached info/refs, try to use the smart HTTP
version when the server supports it.  Since the smart variant is
actually the pkt-line stream from the start of either upload-pack
or receive-pack we need to parse these through get_remote_heads,
which requires a background thread to feed its pipe.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
CC: Daniel Barkalow <barkalow@iabervon.org>
---
 remote-curl.c |  135 +++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 116 insertions(+), 19 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 000bb52..42fd06c 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -4,6 +4,7 @@
 #include "strbuf.h"
 #include "walker.h"
 #include "http.h"
+#include "pkt-line.h"
 #include "run-command.h"
 
 
@@ -17,24 +18,41 @@ static void init_walker(void)
 		walker = get_http_walker(url, remote);
 }
 
-static struct ref *get_refs(void)
+struct discovery {
+	const char *service;
+	char *buf_alloc;
+	char *buf;
+	size_t len;
+	unsigned proto_git : 1;
+};
+static struct discovery *last_discovery;
+
+static void free_discovery(struct discovery *d)
 {
-	struct strbuf buffer = STRBUF_INIT;
-	char *data, *start, *mid;
-	char *ref_name;
+	if (d) {
+		if (d == last_discovery)
+			last_discovery = NULL;
+		free(d->buf_alloc);
+		free(d);
+	}
+}
+
+static struct discovery* discover_refs(const char *service)
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct discovery *last = last_discovery;
 	char *refs_url;
-	int i = 0;
 	int http_ret;
 
-	struct ref *refs = NULL;
-	struct ref *ref = NULL;
-	struct ref *last_ref = NULL;
+	if (last && !strcmp(service, last->service))
+		return last;
+	free_discovery(last);
 
-	refs_url = xmalloc(strlen(url) + 11);
-	sprintf(refs_url, "%s/info/refs", url);
+	strbuf_addf(&buf, "%s/info/refs?service=%s", url, service);
+	refs_url = strbuf_detach(&buf, NULL);
 
 	init_walker();
-	http_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE);
+	http_ret = http_get_strbuf(refs_url, &buf, HTTP_NO_CACHE);
 	switch (http_ret) {
 	case HTTP_OK:
 		break;
@@ -46,10 +64,78 @@ static struct ref *get_refs(void)
 		die("HTTP request failed");
 	}
 
-	data = buffer.buf;
+	last= xcalloc(1, sizeof(*last_discovery));
+	last->service = service;
+	last->buf_alloc = strbuf_detach(&buf, &last->len);
+	last->buf = last->buf_alloc;
+
+	if (5 <= last->len && last->buf[4] == '#') {
+		/* smart HTTP response; validate that the service
+		 * pkt-line matches our request.
+		 */
+		struct strbuf exp = STRBUF_INIT;
+
+		if (packet_get_line(&buf, &last->buf, &last->len) <= 0)
+			die("%s has invalid packet header", refs_url);
+		if (buf.len && buf.buf[buf.len - 1] == '\n')
+			strbuf_setlen(&buf, buf.len - 1);
+
+		strbuf_addf(&exp, "# service=%s", service);
+		if (strbuf_cmp(&exp, &buf))
+			die("invalid server response; got '%s'", buf.buf);
+		strbuf_release(&exp);
+
+		last->proto_git = 1;
+	}
+
+	free(refs_url);
+	strbuf_release(&buf);
+	last_discovery = last;
+	return last;
+}
+
+static int write_discovery(int fd, void *data)
+{
+	struct discovery *heads = data;
+	int err = 0;
+	if (write_in_full(fd, heads->buf, heads->len) != heads->len)
+		err = 1;
+	close(fd);
+	return err;
+}
+
+static struct ref *parse_git_refs(struct discovery *heads)
+{
+	struct ref *list = NULL;
+	struct async async;
+
+	memset(&async, 0, sizeof(async));
+	async.proc = write_discovery;
+	async.data = heads;
+
+	if (start_async(&async))
+		die("cannot start thread to parse advertised refs");
+	get_remote_heads(async.out, &list, 0, NULL, 0, NULL);
+	close(async.out);
+	if (finish_async(&async))
+		die("ref parsing thread failed");
+	return list;
+}
+
+static struct ref *parse_info_refs(struct discovery *heads)
+{
+	char *data, *start, *mid;
+	char *ref_name;
+	int i = 0;
+
+	struct ref *refs = NULL;
+	struct ref *ref = NULL;
+	struct ref *last_ref = NULL;
+
+	data = heads->buf;
 	start = NULL;
 	mid = data;
-	while (i < buffer.len) {
+	while (i < heads->len) {
 		if (!start) {
 			start = &data[i];
 		}
@@ -73,8 +159,7 @@ static struct ref *get_refs(void)
 		i++;
 	}
 
-	strbuf_release(&buffer);
-
+	init_walker();
 	ref = alloc_ref("HEAD");
 	if (!walker->fetch_ref(walker, ref) &&
 	    !resolve_remote_symref(ref, refs)) {
@@ -84,11 +169,23 @@ static struct ref *get_refs(void)
 		free(ref);
 	}
 
-	strbuf_release(&buffer);
-	free(refs_url);
 	return refs;
 }
 
+static struct ref *get_refs(int for_push)
+{
+	struct discovery *heads;
+
+	if (for_push)
+		heads = discover_refs("git-receive-pack");
+	else
+		heads = discover_refs("git-upload-pack");
+
+	if (heads->proto_git)
+		return parse_git_refs(heads);
+	return parse_info_refs(heads);
+}
+
 static void output_refs(struct ref *refs)
 {
 	struct ref *posn;
@@ -285,10 +382,10 @@ int main(int argc, const char **argv)
 			parse_fetch(&buf, 1);
 
 		} else if (!strcmp(buf.buf, "list")) {
-			output_refs(get_refs());
+			output_refs(get_refs(0));
 
 		} else if (!strcmp(buf.buf, "list for-push")) {
-			output_refs(get_refs());
+			output_refs(get_refs(1));
 
 		} else if (!prefixcmp(buf.buf, "push ")) {
 			parse_push(&buf);
-- 
1.6.5.52.g0ff2e

[RFC PATCH v2 14/16] Smart push over HTTP: client side

[Shawn O. Pearce]

The git-remote-curl backend detects if the remote server supports
the git-receive-pack service, and if so, runs git-send-pack in a
pipe to dump the command and pack data as a single POST request.

The advertisements from the server that were obtained during the
discovery are passed into git-send-pack before the POST request
starts.  This permits git-send-pack to operate largely unmodified.

For smaller packs (those under 1 MiB) a tranditional POST with a
Content-Length is used, permitting interaction with any HTTP/1.0
compliant server.  The 1 MiB limit is arbitrary, but is sufficent
to fit most deltas created by human authors against text sources
with the occasional small binary file (e.g. few KiB icon image).

For larger packs which cannot be spooled entirely into the
helper's memory space, the POST request requires HTTP/1.1 and
Transfer-Encoding: chunked.  This permits the client to upload an
unknown amount of data in one HTTP transaction without needing to
pregenerate the entire pack file locally.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
CC: Daniel Barkalow <barkalow@iabervon.org>
---
 Documentation/config.txt |    8 ++
 builtin-send-pack.c      |  116 ++++++++++++++++++++-
 remote-curl.c            |  258 +++++++++++++++++++++++++++++++++++++++++++++-
 send-pack.h              |    3 +-
 sideband.c               |   11 ++-
 transport.c              |    1 +
 6 files changed, 384 insertions(+), 13 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index cd17814..7130d07 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1089,6 +1089,14 @@ http.maxRequests::
 	How many HTTP requests to launch in parallel. Can be overridden
 	by the 'GIT_HTTP_MAX_REQUESTS' environment variable. Default is 5.
 
+http.postBuffer::
+	Maximum size in bytes of the buffer used by smart HTTP
+	transports when POSTing data to the remote system.
+	For requests larger than this buffer size, HTTP/1.1 and
+	Transfer-Encoding: chunked is used to avoid creating a
+	massive pack file locally.  Default is 1 MiB, which is
+	sufficient for most requests.
+
 http.lowSpeedLimit, http.lowSpeedTime::
 	If the HTTP transfer speed is less than 'http.lowSpeedLimit'
 	for longer than 'http.lowSpeedTime' seconds, the transfer is aborted.
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 37e528e..654c3d4 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -2,9 +2,11 @@
 #include "commit.h"
 #include "refs.h"
 #include "pkt-line.h"
+#include "sideband.h"
 #include "run-command.h"
 #include "remote.h"
 #include "send-pack.h"
+#include "quote.h"
 
 static const char send_pack_usage[] =
 "git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
@@ -59,7 +61,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 	memset(&po, 0, sizeof(po));
 	po.argv = argv;
 	po.in = -1;
-	po.out = fd;
+	po.out = args->one_shot_rpc ? -1 : fd;
 	po.git_cmd = 1;
 	if (start_command(&po))
 		die_errno("git pack-objects failed");
@@ -83,6 +85,20 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 	}
 
 	close(po.in);
+
+	if (args->one_shot_rpc) {
+		char *buf = xmalloc(LARGE_PACKET_MAX);
+		while (1) {
+			ssize_t n = xread(po.out, buf, LARGE_PACKET_MAX);
+			if (n <= 0)
+				break;
+			send_sideband(fd, -1, buf, n, LARGE_PACKET_MAX);
+		}
+		free(buf);
+		close(po.out);
+		po.out = -1;
+	}
+
 	if (finish_command(&po))
 		return error("pack-objects died with strange error");
 	return 0;
@@ -303,6 +319,59 @@ static int refs_pushed(struct ref *ref)
 	return 0;
 }
 
+static void print_helper_status(struct ref *ref)
+{
+	struct strbuf buf = STRBUF_INIT;
+
+	for (; ref; ref = ref->next) {
+		const char *msg = NULL;
+		const char *res;
+
+		switch(ref->status) {
+		case REF_STATUS_NONE:
+			res = "error";
+			msg = "no match";
+			break;
+
+		case REF_STATUS_OK:
+			res = "ok";
+			break;
+
+		case REF_STATUS_UPTODATE:
+			res = "ok";
+			msg = "up to date";
+			break;
+
+		case REF_STATUS_REJECT_NONFASTFORWARD:
+			res = "error";
+			msg = "non-fast forward";
+			break;
+
+		case REF_STATUS_REJECT_NODELETE:
+		case REF_STATUS_REMOTE_REJECT:
+			res = "error";
+			break;
+
+		case REF_STATUS_EXPECTING_REPORT:
+		default:
+			continue;
+		}
+
+		strbuf_reset(&buf);
+		strbuf_addf(&buf, "%s %s", res, ref->name);
+		if (ref->remote_status)
+			msg = ref->remote_status;
+		if (msg) {
+			strbuf_addch(&buf, ' ');
+			quote_two_c_style(&buf, "", msg, 0);
+		}
+		strbuf_addch(&buf, '\n');
+
+		safe_write(1, buf.buf, buf.len);
+	}
+	strbuf_release(&buf);
+}
+
 int send_pack(struct send_pack_args *args,
 	      int fd[], struct child_process *conn,
 	      struct ref *remote_refs,
@@ -310,6 +379,7 @@ int send_pack(struct send_pack_args *args,
 {
 	int in = fd[0];
 	int out = fd[1];
+	struct strbuf req_buf = STRBUF_INIT;
 	struct ref *ref;
 	int new_refs;
 	int ask_for_status_report = 0;
@@ -391,14 +461,14 @@ int send_pack(struct send_pack_args *args,
 			char *new_hex = sha1_to_hex(ref->new_sha1);
 
 			if (ask_for_status_report) {
-				packet_write(out, "%s %s %s%c%s",
+				packet_buf_write(&req_buf, "%s %s %s%c%s",
 					old_hex, new_hex, ref->name, 0,
 					"report-status");
 				ask_for_status_report = 0;
 				expect_status_report = 1;
 			}
 			else
-				packet_write(out, "%s %s %s",
+				packet_buf_write(&req_buf, "%s %s %s",
 					old_hex, new_hex, ref->name);
 		}
 		ref->status = expect_status_report ?
@@ -406,7 +476,17 @@ int send_pack(struct send_pack_args *args,
 			REF_STATUS_OK;
 	}
 
-	packet_flush(out);
+	if (args->one_shot_rpc) {
+		if (!args->dry_run) {
+			packet_buf_flush(&req_buf);
+			send_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);
+		}
+	} else {
+		safe_write(out, req_buf.buf, req_buf.len);
+		packet_flush(out);
+	}
+	strbuf_release(&req_buf);
+
 	if (new_refs && !args->dry_run) {
 		if (pack_objects(out, remote_refs, extra_have, args) < 0) {
 			for (ref = remote_refs; ref; ref = ref->next)
@@ -414,11 +494,15 @@ int send_pack(struct send_pack_args *args,
 			return -1;
 		}
 	}
+	if (args->one_shot_rpc && !args->dry_run)
+		packet_flush(out);
 
 	if (expect_status_report)
 		ret = receive_status(in, remote_refs);
 	else
 		ret = 0;
+	if (args->one_shot_rpc)
+		packet_flush(out);
 
 	if (ret < 0)
 		return ret;
@@ -478,6 +562,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 	struct extra_have_objects extra_have;
 	struct ref *remote_refs, *local_refs;
 	int ret;
+	int helper_status = 0;
 	int send_all = 0;
 	const char *receivepack = "git-receive-pack";
 	int flags;
@@ -523,6 +608,14 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 				args.use_thin_pack = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--one-shot-rpc")) {
+				args.one_shot_rpc = 1;
+				continue;
+			}
+			if (!strcmp(arg, "--helper-status")) {
+				helper_status = 1;
+				continue;
+			}
 			usage(send_pack_usage);
 		}
 		if (!dest) {
@@ -551,7 +644,14 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	conn = git_connect(fd, dest, receivepack, args.verbose ? CONNECT_VERBOSE : 0);
+	if (args.one_shot_rpc) {
+		conn = NULL;
+		fd[0] = 0;
+		fd[1] = 1;
+	} else {
+		conn = git_connect(fd, dest, receivepack,
+			args.verbose ? CONNECT_VERBOSE : 0);
+	}
 
 	memset(&extra_have, 0, sizeof(extra_have));
 
@@ -575,12 +675,16 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 
 	ret = send_pack(&args, fd, conn, remote_refs, &extra_have);
 
+	if (helper_status)
+		print_helper_status(remote_refs);
+
 	close(fd[1]);
 	close(fd[0]);
 
 	ret |= finish_connect(conn);
 
-	print_push_status(dest, remote_refs);
+	if (!helper_status)
+		print_push_status(dest, remote_refs);
 
 	if (!args.dry_run && remote) {
 		struct ref *ref;
diff --git a/remote-curl.c b/remote-curl.c
index 42fd06c..529df42 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -5,17 +5,33 @@
 #include "walker.h"
 #include "http.h"
 #include "pkt-line.h"
+#include "sideband.h"
 #include "run-command.h"
 
 
+static size_t post_buffer_size = 16 * LARGE_PACKET_MAX;
 static struct remote *remote;
 static const char *url;
 static struct walker *walker;
 
+static int http_options(const char *var, const char *value, void *cb)
+{
+	if (!strcmp("http.postbuffer", var)) {
+		post_buffer_size = git_config_int(var, value);
+		if (post_buffer_size < LARGE_PACKET_MAX)
+			post_buffer_size = LARGE_PACKET_MAX;
+		return 0;
+	}
+
+	return 0;
+}
+
 static void init_walker(void)
 {
-	if (!walker)
+	if (!walker) {
 		walker = get_http_walker(url, remote);
+		git_config(http_options, NULL);
+	}
 }
 
 struct discovery {
@@ -200,6 +216,193 @@ static void output_refs(struct ref *refs)
 	free_refs(refs);
 }
 
+struct rpc_state {
+	const char *service_name;
+	char *service_url;
+	char *hdr_content_type;
+	char *hdr_accept;
+	char *buf;
+	size_t alloc;
+	size_t len;
+	size_t pos;
+	int in;
+	int out;
+	unsigned verbose : 1;
+};
+
+static size_t rpc_out(void *ptr, size_t eltsize,
+		size_t nmemb, void *buffer_)
+{
+	size_t max = eltsize * nmemb;
+	struct rpc_state *state = buffer_;
+	size_t avail = state->len - state->pos;
+
+	if (!avail) {
+		avail = packet_read_line(state->out, state->buf, state->alloc);
+		if (!avail)
+			return 0;
+		state->pos = 0;
+		state->len = avail;
+	}
+
+	if (max < avail);
+		avail = max;
+	memcpy(ptr, state->buf + state->pos, avail);
+	state->pos += avail;
+	return avail;
+}
+
+static size_t rpc_in(const void *ptr, size_t eltsize,
+		size_t nmemb, void *buffer_)
+{
+	size_t size = eltsize * nmemb;
+	struct rpc_state *state = buffer_;
+	write_or_die(state->in, ptr, size);
+	return size;
+}
+
+static int post_rpc(struct rpc_state *state)
+{
+	struct active_request_slot *slot;
+	struct slot_results results;
+	struct curl_slist *headers = NULL;
+	int err = 0, large_request = 0;
+
+	/* Try to load the entire request, if we can fit it into the
+	 * allocated buffer space we can use HTTP/1.0 and avoid the
+	 * chunked encoding mess.
+	 */
+	while (1) {
+		size_t left = state->alloc - state->len;
+		char *buf = state->buf + state->len;
+		int n;
+
+		if (left < LARGE_PACKET_MAX) {
+			large_request = 1;
+			break;
+		}
+
+		n = packet_read_line(state->out, buf, left);
+		if (!n)
+			break;
+		state->len += n;
+	}
+
+	slot = get_active_slot();
+	slot->results = &results;
+
+	curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
+	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
+	curl_easy_setopt(slot->curl, CURLOPT_URL, state->service_url);
+
+	headers = curl_slist_append(headers, state->hdr_content_type);
+	headers = curl_slist_append(headers, state->hdr_accept);
+
+	if (large_request) {
+		/* The request body is large and the size cannot be predicted.
+		 * We must use chunked encoding to send it.
+		 */
+		headers = curl_slist_append(headers, "Expect: 100-continue");
+		headers = curl_slist_append(headers, "Transfer-Encoding: chunked");
+		curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
+		curl_easy_setopt(slot->curl, CURLOPT_INFILE, state);
+		if (state->verbose) {
+			fprintf(stderr, "POST %s (chunked)\n", state->service_name);
+			fflush(stderr);
+		}
+
+	} else {
+		/* We know the complete request size in advance, use the
+		 * more normal Content-Length approach.
+		 */
+		curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, state->buf);
+		curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, state->len);
+		if (state->verbose) {
+			fprintf(stderr, "POST %s (%lu bytes)\n",
+				state->service_name, (unsigned long)state->len);
+			fflush(stderr);
+		}
+	}
+
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
+	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
+	curl_easy_setopt(slot->curl, CURLOPT_FILE, state);
+
+	if (start_active_slot(slot)) {
+		run_active_slot(slot);
+		if (results.curl_result != CURLE_OK) {
+			err |= error("RPC failed; result=%d, HTTP code = %ld",
+				results.curl_result, results.http_code);
+		}
+	}
+	curl_slist_free_all(headers);
+	return err;
+}
+
+static int one_shot_rpc_service(const char *service,
+	int verbose,
+	const char **client_argv,
+	struct discovery *heads,
+	struct strbuf *result)
+{
+	struct child_process client;
+	struct rpc_state state;
+	struct strbuf buf = STRBUF_INIT;
+	int err = 0;
+
+	init_walker();
+	memset(&client, 0, sizeof(client));
+	client.in = -1;
+	client.out = -1;
+	client.git_cmd = 1;
+	client.argv = client_argv;
+	if (start_command(&client))
+		die("%s failed to execute", client_argv[0]);
+	if (heads)
+		write_or_die(client.in, heads->buf, heads->len);
+
+	memset(&state, 0, sizeof(state));
+	state.alloc = post_buffer_size;
+	state.buf = xmalloc(state.alloc);
+	state.in = client.in;
+	state.out = client.out;
+	state.service_name = service;
+	state.verbose = !!verbose;
+
+	strbuf_addf(&buf, "%s/%s", url, service);
+	state.service_url = strbuf_detach(&buf, NULL);
+
+	strbuf_addf(&buf, "Content-Type: application/x-%s-request", service);
+	state.hdr_content_type = strbuf_detach(&buf, NULL);
+
+	strbuf_addf(&buf, "Accept: application/x-%s-response", service);
+	state.hdr_accept = strbuf_detach(&buf, NULL);
+
+	while (!err) {
+		int n = packet_read_line(state.out, state.buf, state.alloc);
+		if (!n)
+			break;
+		state.pos = 0;
+		state.len = n;
+		err |= post_rpc(&state);
+	}
+	if (result)
+		strbuf_read(result, client.out, 0);
+
+	close(client.in);
+	close(client.out);
+	client.in = -1;
+	client.out = -1;
+
+	err |= finish_command(&client);
+	free(state.service_url);
+	free(state.hdr_content_type);
+	free(state.hdr_accept);
+	free(state.buf);
+	strbuf_release(&buf);
+	return err;
+}
+
 struct fetch_args {
 	unsigned verbose : 1;
 };
@@ -289,7 +492,8 @@ static void parse_fetch(struct strbuf *buf, int multiple)
 
 struct push_args {
 	unsigned dry_run : 1,
-		verbose : 1;
+		verbose : 1,
+		thin : 1;
 };
 
 static int push_dav(struct push_args *args, int nr_spec, char **specs)
@@ -314,6 +518,51 @@ static int push_dav(struct push_args *args, int nr_spec, char **specs)
 	return 0;
 }
 
+static int push_git(struct discovery *heads,
+	struct push_args *args, int nr_spec, char **specs)
+{
+	struct strbuf res = STRBUF_INIT;
+	const char **argv;
+	int argc = 0, i, err;
+
+	argv = xmalloc((10 + nr_spec) * sizeof(char*));
+	argv[argc++] = "send-pack";
+	argv[argc++] = "--one-shot-rpc";
+	argv[argc++] = "--helper-status";
+	if (args->thin)
+		argv[argc++] = "--thin";
+	if (args->dry_run)
+		argv[argc++] = "--dry-run";
+	if (args->verbose)
+		argv[argc++] = "--verbose";
+	argv[argc++] = url;
+	for (i = 0; i < nr_spec; i++)
+		argv[argc++] = specs[i];
+	argv[argc++] = NULL;
+
+	err = one_shot_rpc_service("git-receive-pack",
+		args->verbose,
+		argv, heads, &res);
+	if (res.len)
+		safe_write(1, res.buf, res.len);
+	strbuf_release(&res);
+	free(argv);
+	return err;
+}
+
+static int push(struct push_args *args, int nr_spec, char **specs)
+{
+	struct discovery *heads = discover_refs("git-receive-pack");
+	int ret;
+
+	if (heads->proto_git)
+		ret = push_git(heads, args, nr_spec, specs);
+	else
+		ret = push_dav(args, nr_spec, specs);
+	free_discovery(heads);
+	return ret;
+}
+
 static void parse_push(struct strbuf *buf)
 {
 	char **specs = NULL;
@@ -330,6 +579,8 @@ static void parse_push(struct strbuf *buf)
 			args.dry_run = 1;
 		else if (!strcmp(buf->buf, "option verbose"))
 			args.verbose = 1;
+		else if (!strcmp(buf->buf, "option thin"))
+			args.thin = 1;
 		else
 			die("http transport does not support %s", buf->buf);
 
@@ -340,7 +591,7 @@ static void parse_push(struct strbuf *buf)
 			break;
 	} while (1);
 
-	if (push_dav(&args, nr_spec, specs))
+	if (push(&args, nr_spec, specs))
 		exit(128); /* error already reported */
 	for (i = 0; i < nr_spec; i++)
 		free(specs[i]);
@@ -395,6 +646,7 @@ int main(int argc, const char **argv)
 			printf("fetch-multiple\n");
 			printf("push\n");
 			printf("option dry-run\n");
+			printf("option thin\n");
 			printf("option verbose\n");
 			printf("\n");
 			fflush(stdout);
diff --git a/send-pack.h b/send-pack.h
index 8b3cf02..a7f4abf 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -8,7 +8,8 @@ struct send_pack_args {
 		force_update:1,
 		use_thin_pack:1,
 		use_ofs_delta:1,
-		dry_run:1;
+		dry_run:1,
+		one_shot_rpc:1;
 };
 
 int send_pack(struct send_pack_args *args,
diff --git a/sideband.c b/sideband.c
index 899b1ff..d5ffa1c 100644
--- a/sideband.c
+++ b/sideband.c
@@ -135,9 +135,14 @@ ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int packet
 		n = sz;
 		if (packet_max - 5 < n)
 			n = packet_max - 5;
-		sprintf(hdr, "%04x", n + 5);
-		hdr[4] = band;
-		safe_write(fd, hdr, 5);
+		if (0 <= band) {
+			sprintf(hdr, "%04x", n + 5);
+			hdr[4] = band;
+			safe_write(fd, hdr, 5);
+		} else {
+			sprintf(hdr, "%04x", n + 4);
+			safe_write(fd, hdr, 4);
+		}
 		safe_write(fd, p, n);
 		p += n;
 		sz -= n;
diff --git a/transport.c b/transport.c
index 6d9652d..2ff1650 100644
--- a/transport.c
+++ b/transport.c
@@ -731,6 +731,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
 				 NULL);
 	}
 
+	memset(&args, 0, sizeof(args));
 	args.send_mirror = !!(flags & TRANSPORT_PUSH_MIRROR);
 	args.force_update = !!(flags & TRANSPORT_PUSH_FORCE);
 	args.use_thin_pack = data->thin;
-- 
1.6.5.52.g0ff2e

Re: [RFC PATCH v2 14/16] Smart push over HTTP: client side

[Felipe Contreras]

On Tue, Oct 13, 2009 at 5:25 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> The git-remote-curl backend detects if the remote server supports
> the git-receive-pack service, and if so, runs git-send-pack in a
> pipe to dump the command and pack data as a single POST request.
>
> The advertisements from the server that were obtained during the
> discovery are passed into git-send-pack before the POST request
> starts.  This permits git-send-pack to operate largely unmodified.
>
> For smaller packs (those under 1 MiB) a tranditional POST with a

Traditional?

> Content-Length is used, permitting interaction with any HTTP/1.0
> compliant server.  The 1 MiB limit is arbitrary, but is sufficent
> to fit most deltas created by human authors against text sources
> with the occasional small binary file (e.g. few KiB icon image).
>
> For larger packs which cannot be spooled entirely into the
> helper's memory space, the POST request requires HTTP/1.1 and
> Transfer-Encoding: chunked.  This permits the client to upload an
> unknown amount of data in one HTTP transaction without needing to
> pregenerate the entire pack file locally.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> CC: Daniel Barkalow <barkalow@iabervon.org>
> ---
>  Documentation/config.txt |    8 ++
>  builtin-send-pack.c      |  116 ++++++++++++++++++++-
>  remote-curl.c            |  258 +++++++++++++++++++++++++++++++++++++++++++++-
>  send-pack.h              |    3 +-
>  sideband.c               |   11 ++-
>  transport.c              |    1 +
>  6 files changed, 384 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index cd17814..7130d07 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1089,6 +1089,14 @@ http.maxRequests::
>        How many HTTP requests to launch in parallel. Can be overridden
>        by the 'GIT_HTTP_MAX_REQUESTS' environment variable. Default is 5.
>
> +http.postBuffer::
> +       Maximum size in bytes of the buffer used by smart HTTP
> +       transports when POSTing data to the remote system.
> +       For requests larger than this buffer size, HTTP/1.1 and
> +       Transfer-Encoding: chunked is used to avoid creating a
> +       massive pack file locally.  Default is 1 MiB, which is
> +       sufficient for most requests.
> +
>  http.lowSpeedLimit, http.lowSpeedTime::
>        If the HTTP transfer speed is less than 'http.lowSpeedLimit'
>        for longer than 'http.lowSpeedTime' seconds, the transfer is aborted.
> diff --git a/builtin-send-pack.c b/builtin-send-pack.c
> index 37e528e..654c3d4 100644
> --- a/builtin-send-pack.c
> +++ b/builtin-send-pack.c
> @@ -2,9 +2,11 @@
>  #include "commit.h"
>  #include "refs.h"
>  #include "pkt-line.h"
> +#include "sideband.h"
>  #include "run-command.h"
>  #include "remote.h"
>  #include "send-pack.h"
> +#include "quote.h"
>
>  static const char send_pack_usage[] =
>  "git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
> @@ -59,7 +61,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
>        memset(&po, 0, sizeof(po));
>        po.argv = argv;
>        po.in = -1;
> -       po.out = fd;
> +       po.out = args->one_shot_rpc ? -1 : fd;
>        po.git_cmd = 1;
>        if (start_command(&po))
>                die_errno("git pack-objects failed");
> @@ -83,6 +85,20 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
>        }
>
>        close(po.in);
> +
> +       if (args->one_shot_rpc) {
> +               char *buf = xmalloc(LARGE_PACKET_MAX);
> +               while (1) {
> +                       ssize_t n = xread(po.out, buf, LARGE_PACKET_MAX);
> +                       if (n <= 0)
> +                               break;
> +                       send_sideband(fd, -1, buf, n, LARGE_PACKET_MAX);
> +               }
> +               free(buf);
> +               close(po.out);
> +               po.out = -1;
> +       }
> +
>        if (finish_command(&po))
>                return error("pack-objects died with strange error");
>        return 0;
> @@ -303,6 +319,59 @@ static int refs_pushed(struct ref *ref)
>        return 0;
>  }
>
> +static void print_helper_status(struct ref *ref)
> +{
> +       struct strbuf buf = STRBUF_INIT;
> +
> +       for (; ref; ref = ref->next) {
> +               const char *msg = NULL;
> +               const char *res;
> +
> +               switch(ref->status) {
> +               case REF_STATUS_NONE:
> +                       res = "error";
> +                       msg = "no match";
> +                       break;
> +
> +               case REF_STATUS_OK:
> +                       res = "ok";
> +                       break;
> +
> +               case REF_STATUS_UPTODATE:
> +                       res = "ok";
> +                       msg = "up to date";
> +                       break;
> +
> +               case REF_STATUS_REJECT_NONFASTFORWARD:
> +                       res = "error";
> +                       msg = "non-fast forward";
> +                       break;
> +
> +               case REF_STATUS_REJECT_NODELETE:
> +               case REF_STATUS_REMOTE_REJECT:
> +                       res = "error";
> +                       break;
> +
> +               case REF_STATUS_EXPECTING_REPORT:
> +               default:
> +                       continue;
> +               }
> +
> +               strbuf_reset(&buf);
> +               strbuf_addf(&buf, "%s %s", res, ref->name);
> +               if (ref->remote_status)
> +                       msg = ref->remote_status;
> +               if (msg) {
> +                       strbuf_addch(&buf, ' ');
> +                       quote_two_c_style(&buf, "", msg, 0);
> +               }
> +               strbuf_addch(&buf, '\n');
> +
> +               safe_write(1, buf.buf, buf.len);
> +       }
> +       strbuf_release(&buf);
> +}
> +
>  int send_pack(struct send_pack_args *args,
>              int fd[], struct child_process *conn,
>              struct ref *remote_refs,
> @@ -310,6 +379,7 @@ int send_pack(struct send_pack_args *args,
>  {
>        int in = fd[0];
>        int out = fd[1];
> +       struct strbuf req_buf = STRBUF_INIT;
>        struct ref *ref;
>        int new_refs;
>        int ask_for_status_report = 0;
> @@ -391,14 +461,14 @@ int send_pack(struct send_pack_args *args,
>                        char *new_hex = sha1_to_hex(ref->new_sha1);
>
>                        if (ask_for_status_report) {
> -                               packet_write(out, "%s %s %s%c%s",
> +                               packet_buf_write(&req_buf, "%s %s %s%c%s",
>                                        old_hex, new_hex, ref->name, 0,
>                                        "report-status");
>                                ask_for_status_report = 0;
>                                expect_status_report = 1;
>                        }
>                        else
> -                               packet_write(out, "%s %s %s",
> +                               packet_buf_write(&req_buf, "%s %s %s",
>                                        old_hex, new_hex, ref->name);
>                }
>                ref->status = expect_status_report ?
> @@ -406,7 +476,17 @@ int send_pack(struct send_pack_args *args,
>                        REF_STATUS_OK;
>        }
>
> -       packet_flush(out);
> +       if (args->one_shot_rpc) {
> +               if (!args->dry_run) {
> +                       packet_buf_flush(&req_buf);
> +                       send_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);
> +               }
> +       } else {
> +               safe_write(out, req_buf.buf, req_buf.len);
> +               packet_flush(out);
> +       }
> +       strbuf_release(&req_buf);
> +
>        if (new_refs && !args->dry_run) {
>                if (pack_objects(out, remote_refs, extra_have, args) < 0) {
>                        for (ref = remote_refs; ref; ref = ref->next)
> @@ -414,11 +494,15 @@ int send_pack(struct send_pack_args *args,
>                        return -1;
>                }
>        }
> +       if (args->one_shot_rpc && !args->dry_run)
> +               packet_flush(out);
>
>        if (expect_status_report)
>                ret = receive_status(in, remote_refs);
>        else
>                ret = 0;
> +       if (args->one_shot_rpc)
> +               packet_flush(out);
>
>        if (ret < 0)
>                return ret;
> @@ -478,6 +562,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
>        struct extra_have_objects extra_have;
>        struct ref *remote_refs, *local_refs;
>        int ret;
> +       int helper_status = 0;
>        int send_all = 0;
>        const char *receivepack = "git-receive-pack";
>        int flags;
> @@ -523,6 +608,14 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
>                                args.use_thin_pack = 1;
>                                continue;
>                        }
> +                       if (!strcmp(arg, "--one-shot-rpc")) {
> +                               args.one_shot_rpc = 1;
> +                               continue;
> +                       }
> +                       if (!strcmp(arg, "--helper-status")) {
> +                               helper_status = 1;
> +                               continue;
> +                       }
>                        usage(send_pack_usage);
>                }
>                if (!dest) {
> @@ -551,7 +644,14 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
>                }
>        }
>
> -       conn = git_connect(fd, dest, receivepack, args.verbose ? CONNECT_VERBOSE : 0);
> +       if (args.one_shot_rpc) {
> +               conn = NULL;
> +               fd[0] = 0;
> +               fd[1] = 1;
> +       } else {
> +               conn = git_connect(fd, dest, receivepack,
> +                       args.verbose ? CONNECT_VERBOSE : 0);
> +       }
>
>        memset(&extra_have, 0, sizeof(extra_have));
>
> @@ -575,12 +675,16 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
>
>        ret = send_pack(&args, fd, conn, remote_refs, &extra_have);
>
> +       if (helper_status)
> +               print_helper_status(remote_refs);
> +
>        close(fd[1]);
>        close(fd[0]);
>
>        ret |= finish_connect(conn);
>
> -       print_push_status(dest, remote_refs);
> +       if (!helper_status)
> +               print_push_status(dest, remote_refs);
>
>        if (!args.dry_run && remote) {
>                struct ref *ref;
> diff --git a/remote-curl.c b/remote-curl.c
> index 42fd06c..529df42 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -5,17 +5,33 @@
>  #include "walker.h"
>  #include "http.h"
>  #include "pkt-line.h"
> +#include "sideband.h"
>  #include "run-command.h"
>
>
> +static size_t post_buffer_size = 16 * LARGE_PACKET_MAX;
>  static struct remote *remote;
>  static const char *url;
>  static struct walker *walker;
>
> +static int http_options(const char *var, const char *value, void *cb)
> +{
> +       if (!strcmp("http.postbuffer", var)) {
> +               post_buffer_size = git_config_int(var, value);
> +               if (post_buffer_size < LARGE_PACKET_MAX)
> +                       post_buffer_size = LARGE_PACKET_MAX;
> +               return 0;
> +       }
> +
> +       return 0;
> +}
> +
>  static void init_walker(void)
>  {
> -       if (!walker)
> +       if (!walker) {
>                walker = get_http_walker(url, remote);
> +               git_config(http_options, NULL);
> +       }
>  }
>
>  struct discovery {
> @@ -200,6 +216,193 @@ static void output_refs(struct ref *refs)
>        free_refs(refs);
>  }
>
> +struct rpc_state {
> +       const char *service_name;
> +       char *service_url;
> +       char *hdr_content_type;
> +       char *hdr_accept;
> +       char *buf;
> +       size_t alloc;
> +       size_t len;
> +       size_t pos;
> +       int in;
> +       int out;
> +       unsigned verbose : 1;
> +};
> +
> +static size_t rpc_out(void *ptr, size_t eltsize,
> +               size_t nmemb, void *buffer_)
> +{
> +       size_t max = eltsize * nmemb;
> +       struct rpc_state *state = buffer_;
> +       size_t avail = state->len - state->pos;
> +
> +       if (!avail) {
> +               avail = packet_read_line(state->out, state->buf, state->alloc);
> +               if (!avail)
> +                       return 0;
> +               state->pos = 0;
> +               state->len = avail;
> +       }
> +
> +       if (max < avail);
> +               avail = max;
> +       memcpy(ptr, state->buf + state->pos, avail);
> +       state->pos += avail;
> +       return avail;
> +}
> +
> +static size_t rpc_in(const void *ptr, size_t eltsize,
> +               size_t nmemb, void *buffer_)
> +{
> +       size_t size = eltsize * nmemb;
> +       struct rpc_state *state = buffer_;
> +       write_or_die(state->in, ptr, size);
> +       return size;
> +}
> +
> +static int post_rpc(struct rpc_state *state)
> +{
> +       struct active_request_slot *slot;
> +       struct slot_results results;
> +       struct curl_slist *headers = NULL;
> +       int err = 0, large_request = 0;
> +
> +       /* Try to load the entire request, if we can fit it into the
> +        * allocated buffer space we can use HTTP/1.0 and avoid the
> +        * chunked encoding mess.
> +        */
> +       while (1) {
> +               size_t left = state->alloc - state->len;
> +               char *buf = state->buf + state->len;
> +               int n;
> +
> +               if (left < LARGE_PACKET_MAX) {
> +                       large_request = 1;
> +                       break;
> +               }
> +
> +               n = packet_read_line(state->out, buf, left);
> +               if (!n)
> +                       break;
> +               state->len += n;
> +       }
> +
> +       slot = get_active_slot();
> +       slot->results = &results;
> +
> +       curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
> +       curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
> +       curl_easy_setopt(slot->curl, CURLOPT_URL, state->service_url);
> +
> +       headers = curl_slist_append(headers, state->hdr_content_type);
> +       headers = curl_slist_append(headers, state->hdr_accept);
> +
> +       if (large_request) {
> +               /* The request body is large and the size cannot be predicted.
> +                * We must use chunked encoding to send it.
> +                */
> +               headers = curl_slist_append(headers, "Expect: 100-continue");
> +               headers = curl_slist_append(headers, "Transfer-Encoding: chunked");
> +               curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
> +               curl_easy_setopt(slot->curl, CURLOPT_INFILE, state);
> +               if (state->verbose) {
> +                       fprintf(stderr, "POST %s (chunked)\n", state->service_name);
> +                       fflush(stderr);
> +               }
> +
> +       } else {
> +               /* We know the complete request size in advance, use the
> +                * more normal Content-Length approach.
> +                */
> +               curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, state->buf);
> +               curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, state->len);
> +               if (state->verbose) {
> +                       fprintf(stderr, "POST %s (%lu bytes)\n",
> +                               state->service_name, (unsigned long)state->len);
> +                       fflush(stderr);
> +               }
> +       }
> +
> +       curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
> +       curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
> +       curl_easy_setopt(slot->curl, CURLOPT_FILE, state);
> +
> +       if (start_active_slot(slot)) {
> +               run_active_slot(slot);
> +               if (results.curl_result != CURLE_OK) {
> +                       err |= error("RPC failed; result=%d, HTTP code = %ld",
> +                               results.curl_result, results.http_code);
> +               }
> +       }
> +       curl_slist_free_all(headers);
> +       return err;
> +}
> +
> +static int one_shot_rpc_service(const char *service,
> +       int verbose,
> +       const char **client_argv,
> +       struct discovery *heads,
> +       struct strbuf *result)
> +{
> +       struct child_process client;
> +       struct rpc_state state;
> +       struct strbuf buf = STRBUF_INIT;
> +       int err = 0;
> +
> +       init_walker();
> +       memset(&client, 0, sizeof(client));
> +       client.in = -1;
> +       client.out = -1;
> +       client.git_cmd = 1;
> +       client.argv = client_argv;
> +       if (start_command(&client))
> +               die("%s failed to execute", client_argv[0]);
> +       if (heads)
> +               write_or_die(client.in, heads->buf, heads->len);
> +
> +       memset(&state, 0, sizeof(state));
> +       state.alloc = post_buffer_size;
> +       state.buf = xmalloc(state.alloc);
> +       state.in = client.in;
> +       state.out = client.out;
> +       state.service_name = service;
> +       state.verbose = !!verbose;
> +
> +       strbuf_addf(&buf, "%s/%s", url, service);
> +       state.service_url = strbuf_detach(&buf, NULL);
> +
> +       strbuf_addf(&buf, "Content-Type: application/x-%s-request", service);
> +       state.hdr_content_type = strbuf_detach(&buf, NULL);
> +
> +       strbuf_addf(&buf, "Accept: application/x-%s-response", service);
> +       state.hdr_accept = strbuf_detach(&buf, NULL);
> +
> +       while (!err) {
> +               int n = packet_read_line(state.out, state.buf, state.alloc);
> +               if (!n)
> +                       break;
> +               state.pos = 0;
> +               state.len = n;
> +               err |= post_rpc(&state);
> +       }
> +       if (result)
> +               strbuf_read(result, client.out, 0);
> +
> +       close(client.in);
> +       close(client.out);
> +       client.in = -1;
> +       client.out = -1;
> +
> +       err |= finish_command(&client);
> +       free(state.service_url);
> +       free(state.hdr_content_type);
> +       free(state.hdr_accept);
> +       free(state.buf);
> +       strbuf_release(&buf);
> +       return err;
> +}
> +
>  struct fetch_args {
>        unsigned verbose : 1;
>  };
> @@ -289,7 +492,8 @@ static void parse_fetch(struct strbuf *buf, int multiple)
>
>  struct push_args {
>        unsigned dry_run : 1,
> -               verbose : 1;
> +               verbose : 1,
> +               thin : 1;
>  };
>
>  static int push_dav(struct push_args *args, int nr_spec, char **specs)
> @@ -314,6 +518,51 @@ static int push_dav(struct push_args *args, int nr_spec, char **specs)
>        return 0;
>  }
>
> +static int push_git(struct discovery *heads,
> +       struct push_args *args, int nr_spec, char **specs)
> +{
> +       struct strbuf res = STRBUF_INIT;
> +       const char **argv;
> +       int argc = 0, i, err;
> +
> +       argv = xmalloc((10 + nr_spec) * sizeof(char*));
> +       argv[argc++] = "send-pack";
> +       argv[argc++] = "--one-shot-rpc";
> +       argv[argc++] = "--helper-status";
> +       if (args->thin)
> +               argv[argc++] = "--thin";
> +       if (args->dry_run)
> +               argv[argc++] = "--dry-run";
> +       if (args->verbose)
> +               argv[argc++] = "--verbose";
> +       argv[argc++] = url;
> +       for (i = 0; i < nr_spec; i++)
> +               argv[argc++] = specs[i];
> +       argv[argc++] = NULL;
> +
> +       err = one_shot_rpc_service("git-receive-pack",
> +               args->verbose,
> +               argv, heads, &res);
> +       if (res.len)
> +               safe_write(1, res.buf, res.len);
> +       strbuf_release(&res);
> +       free(argv);
> +       return err;
> +}
> +
> +static int push(struct push_args *args, int nr_spec, char **specs)
> +{
> +       struct discovery *heads = discover_refs("git-receive-pack");
> +       int ret;
> +
> +       if (heads->proto_git)
> +               ret = push_git(heads, args, nr_spec, specs);
> +       else
> +               ret = push_dav(args, nr_spec, specs);
> +       free_discovery(heads);
> +       return ret;
> +}
> +
>  static void parse_push(struct strbuf *buf)
>  {
>        char **specs = NULL;
> @@ -330,6 +579,8 @@ static void parse_push(struct strbuf *buf)
>                        args.dry_run = 1;
>                else if (!strcmp(buf->buf, "option verbose"))
>                        args.verbose = 1;
> +               else if (!strcmp(buf->buf, "option thin"))
> +                       args.thin = 1;
>                else
>                        die("http transport does not support %s", buf->buf);
>
> @@ -340,7 +591,7 @@ static void parse_push(struct strbuf *buf)
>                        break;
>        } while (1);
>
> -       if (push_dav(&args, nr_spec, specs))
> +       if (push(&args, nr_spec, specs))
>                exit(128); /* error already reported */
>        for (i = 0; i < nr_spec; i++)
>                free(specs[i]);
> @@ -395,6 +646,7 @@ int main(int argc, const char **argv)
>                        printf("fetch-multiple\n");
>                        printf("push\n");
>                        printf("option dry-run\n");
> +                       printf("option thin\n");
>                        printf("option verbose\n");
>                        printf("\n");
>                        fflush(stdout);
> diff --git a/send-pack.h b/send-pack.h
> index 8b3cf02..a7f4abf 100644
> --- a/send-pack.h
> +++ b/send-pack.h
> @@ -8,7 +8,8 @@ struct send_pack_args {
>                force_update:1,
>                use_thin_pack:1,
>                use_ofs_delta:1,
> -               dry_run:1;
> +               dry_run:1,
> +               one_shot_rpc:1;
>  };
>
>  int send_pack(struct send_pack_args *args,
> diff --git a/sideband.c b/sideband.c
> index 899b1ff..d5ffa1c 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -135,9 +135,14 @@ ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int packet
>                n = sz;
>                if (packet_max - 5 < n)
>                        n = packet_max - 5;
> -               sprintf(hdr, "%04x", n + 5);
> -               hdr[4] = band;
> -               safe_write(fd, hdr, 5);
> +               if (0 <= band) {
> +                       sprintf(hdr, "%04x", n + 5);
> +                       hdr[4] = band;
> +                       safe_write(fd, hdr, 5);
> +               } else {
> +                       sprintf(hdr, "%04x", n + 4);
> +                       safe_write(fd, hdr, 4);
> +               }
>                safe_write(fd, p, n);
>                p += n;
>                sz -= n;
> diff --git a/transport.c b/transport.c
> index 6d9652d..2ff1650 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -731,6 +731,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
>                                 NULL);
>        }
>
> +       memset(&args, 0, sizeof(args));
>        args.send_mirror = !!(flags & TRANSPORT_PUSH_MIRROR);
>        args.force_update = !!(flags & TRANSPORT_PUSH_FORCE);
>        args.use_thin_pack = data->thin;
> --
> 1.6.5.52.g0ff2e
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Felipe Contreras

[RFC PATCH v2 15/16] Smart fetch over HTTP: client side

[Shawn O. Pearce]

The git-remote-curl backend detects if the remote server supports
the git-upload-pack service, and if so, runs git-fetch-pack locally
in a pipe to generate the want/have commands.

The advertisements from the server that were obtained during the
discovery are passed into git-fetch-pack before the POST request
starts, permitting server capability discovery and enablement.

Common objects that are discovered are appended onto the request as
have lines and are sent again on the next request.  This allows the
remote side to reinitialize its in-memory list of common objects
during the next request.

Because all requests are relatively short, below git-remote-curl's
1 MiB buffer limit, requests will use the standard Content-Length
header and be valid HTTP/1.0 POST requests.  This makes the fetch
client more tolerant of proxy servers which don't support HTTP/1.1
or the chunked transfer encoding.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
CC: Daniel Barkalow <barkalow@iabervon.org>
---
 builtin-fetch-pack.c |  110 ++++++++++++++++++++++++++++++++++++++++++--------
 fetch-pack.h         |    3 +-
 remote-curl.c        |   75 +++++++++++++++++++++++++++++++++-
 3 files changed, 168 insertions(+), 20 deletions(-)

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index b68b3eb..fba0c74 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -165,6 +165,24 @@ enum ack_type {
 	ACK_ready
 };
 
+static void consume_shallow_list(int fd)
+{
+	if (args.one_shot_rpc && args.depth > 0) {
+		/* If we sent a depth we will get back "duplicate"
+		 * shallow and unshallow commands every time there
+		 * is a block of have lines exchanged.
+		 */
+		char line[1000];
+		while (packet_read_line(fd, line, sizeof(line))) {
+			if (!prefixcmp(line, "shallow "))
+				continue;
+		 	if (!prefixcmp(line, "unshallow "))
+				continue;
+			die("git fetch-pack: expected shallow list");
+		}
+	}
+}
+
 static enum ack_type get_ack(int fd, unsigned char *result_sha1)
 {
 	static char line[1000];
@@ -190,6 +208,15 @@ static enum ack_type get_ack(int fd, unsigned char *result_sha1)
 	die("git fetch_pack: expected ACK/NAK, got '%s'", line);
 }
 
+static void send_request(int fd, struct strbuf *buf)
+{
+	if (args.one_shot_rpc) {
+		send_sideband(fd, -1, buf->buf, buf->len, LARGE_PACKET_MAX);
+		packet_flush(fd);
+	} else
+		safe_write(fd, buf->buf, buf->len);
+}
+
 static int find_common(int fd[2], unsigned char *result_sha1,
 		       struct ref *refs)
 {
@@ -199,7 +226,10 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 	unsigned in_vain = 0;
 	int got_continue = 0;
 	struct strbuf req_buf = STRBUF_INIT;
+	size_t state_len = 0;
 
+	if (args.one_shot_rpc && multi_ack == 1)
+		die("--one-shot-rpc requires multi_ack_2");
 	if (marked)
 		for_each_ref(clear_marks, NULL);
 	marked = 1;
@@ -256,13 +286,13 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 	if (args.depth > 0)
 		packet_buf_write(&req_buf, "deepen %d", args.depth);
 	packet_buf_flush(&req_buf);
-
-	safe_write(fd[1], req_buf.buf, req_buf.len);
+	state_len = req_buf.len;
 
 	if (args.depth > 0) {
 		char line[1024];
 		unsigned char sha1[20];
 
+		send_request(fd[1], &req_buf);
 		while (packet_read_line(fd[0], line, sizeof(line))) {
 			if (!prefixcmp(line, "shallow ")) {
 				if (get_sha1_hex(line + 8, sha1))
@@ -284,28 +314,40 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 			}
 			die("expected shallow/unshallow, got %s", line);
 		}
+	} else if (!args.one_shot_rpc)
+		send_request(fd[1], &req_buf);
+
+	if (!args.one_shot_rpc) {
+		/* If we aren't using the stateless one-shot-rpc
+		 * interface we don't need to retain the headers.
+		 */
+		strbuf_setlen(&req_buf, 0);
+		state_len = 0;
 	}
 
 	flushes = 0;
 	retval = -1;
 	while ((sha1 = get_rev())) {
-		packet_write(fd[1], "have %s\n", sha1_to_hex(sha1));
+		packet_buf_write(&req_buf, "have %s\n", sha1_to_hex(sha1));
 		if (args.verbose)
 			fprintf(stderr, "have %s\n", sha1_to_hex(sha1));
 		in_vain++;
 		if (!(31 & ++count)) {
 			int ack;
 
-			packet_flush(fd[1]);
+			packet_buf_flush(&req_buf);
+			send_request(fd[1], &req_buf);
+			strbuf_setlen(&req_buf, state_len);
 			flushes++;
 
 			/*
 			 * We keep one window "ahead" of the other side, and
 			 * will wait for an ACK only on the next one
 			 */
-			if (count == 32)
+			if (!args.one_shot_rpc && count == 32)
 				continue;
 
+			consume_shallow_list(fd[0]);
 			do {
 				ack = get_ack(fd[0], result_sha1);
 				if (args.verbose && ack)
@@ -322,6 +364,17 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 				case ACK_continue: {
 					struct commit *commit =
 						lookup_commit(result_sha1);
+					if (args.one_shot_rpc
+					 && ack == ACK_common
+					 && !(commit->object.flags & COMMON)) {
+						/* We need to replay the have for this object
+						 * on the next RPC request so the peer knows
+						 * it is in common with us.
+						 */
+						const char *hex = sha1_to_hex(result_sha1);
+						packet_buf_write(&req_buf, "have %s\n", hex);
+						state_len = req_buf.len;
+					}
 					mark_common(commit, 0, 1);
 					retval = 0;
 					in_vain = 0;
@@ -339,7 +392,8 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 		}
 	}
 done:
-	packet_write(fd[1], "done\n");
+	packet_buf_write(&req_buf, "done\n");
+	send_request(fd[1], &req_buf);
 	if (args.verbose)
 		fprintf(stderr, "done\n");
 	if (retval != 0) {
@@ -348,6 +402,7 @@ done:
 	}
 	strbuf_release(&req_buf);
 
+	consume_shallow_list(fd[0]);
 	while (flushes || multi_ack) {
 		int ack = get_ack(fd[0], result_sha1);
 		if (ack) {
@@ -672,6 +727,8 @@ static struct ref *do_fetch_pack(int fd[2],
 			 */
 			warning("no common commits");
 
+	if (args.one_shot_rpc)
+		packet_flush(fd[1]);
 	if (get_pack(fd, pack_lockfile))
 		die("git fetch-pack: fetch failed.");
 
@@ -742,6 +799,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 	struct ref *ref = NULL;
 	char *dest = NULL, **heads;
 	int fd[2];
+	char *pack_lockfile = NULL;
+	char **pack_lockfile_ptr = NULL;
 	struct child_process *conn;
 
 	nr_heads = 0;
@@ -791,6 +850,15 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 				args.no_progress = 1;
 				continue;
 			}
+			if (!strcmp("--one-shot-rpc", arg)) {
+				args.one_shot_rpc = 1;
+				continue;
+			}
+			if (!strcmp("--lock-pack", arg)) {
+				args.lock_pack = 1;
+				pack_lockfile_ptr = &pack_lockfile;
+				continue;
+			}
 			usage(fetch_pack_usage);
 		}
 		dest = (char *)arg;
@@ -801,19 +869,27 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 	if (!dest)
 		usage(fetch_pack_usage);
 
-	conn = git_connect(fd, (char *)dest, args.uploadpack,
-			   args.verbose ? CONNECT_VERBOSE : 0);
-	if (conn) {
-		get_remote_heads(fd[0], &ref, 0, NULL, 0, NULL);
-
-		ref = fetch_pack(&args, fd, conn, ref, dest, nr_heads, heads, NULL);
-		close(fd[0]);
-		close(fd[1]);
-		if (finish_connect(conn))
-			ref = NULL;
+	if (args.one_shot_rpc) {
+		conn = NULL;
+		fd[0] = 0;
+		fd[1] = 1;
 	} else {
-		ref = NULL;
+		conn = git_connect(fd, (char *)dest, args.uploadpack,
+				   args.verbose ? CONNECT_VERBOSE : 0);
+	}
+
+	get_remote_heads(fd[0], &ref, 0, NULL, 0, NULL);
+
+	ref = fetch_pack(&args, fd, conn, ref, dest,
+		nr_heads, heads, pack_lockfile_ptr);
+	if (pack_lockfile) {
+		printf("lock %s\n", pack_lockfile);
+		fflush(stdout);
 	}
+	close(fd[0]);
+	close(fd[1]);
+	if (finish_connect(conn))
+		ref = NULL;
 	ret = !ref;
 
 	if (!ret && nr_heads) {
diff --git a/fetch-pack.h b/fetch-pack.h
index 8bd9c32..ee6b17f 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -13,7 +13,8 @@ struct fetch_pack_args
 		fetch_all:1,
 		verbose:1,
 		no_progress:1,
-		include_tag:1;
+		include_tag:1,
+		one_shot_rpc:1;
 };
 
 struct ref *fetch_pack(struct fetch_pack_args *args,
diff --git a/remote-curl.c b/remote-curl.c
index 529df42..31d1d34 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -404,7 +404,10 @@ static int one_shot_rpc_service(const char *service,
 }
 
 struct fetch_args {
-	unsigned verbose : 1;
+	int depth;
+	unsigned verbose : 1,
+		thin : 1,
+		followtags : 1;
 };
 
 static int fetch_dumb(struct fetch_args *args,
@@ -413,6 +416,8 @@ static int fetch_dumb(struct fetch_args *args,
 	char **targets = xmalloc(nr_heads * sizeof(char*));
 	int ret, i;
 
+	if (args->depth)
+		die("dumb http transport does not support --depth");
 	for (i = 0; i < nr_heads; i++)
 		targets[i] = xstrdup(sha1_to_hex(to_fetch[i]->old_sha1));
 
@@ -431,6 +436,64 @@ static int fetch_dumb(struct fetch_args *args,
 	return ret ? error("Fetch failed.") : 0;
 }
 
+static int fetch_git(struct fetch_args *args,
+	struct discovery *heads,
+	int nr_heads, struct ref **to_fetch)
+{
+	struct strbuf res = STRBUF_INIT;
+	char *depth_arg = NULL;
+	const char **argv;
+	int argc = 0, i, err;
+
+	argv = xmalloc((15 + nr_heads) * sizeof(char*));
+	argv[argc++] = "fetch-pack";
+	argv[argc++] = "--one-shot-rpc";
+	argv[argc++] = "--lock-pack";
+	if (args->followtags)
+		argv[argc++] = "--include-tag";
+	if (args->thin)
+		argv[argc++] = "--thin";
+	if (args->verbose) {
+		argv[argc++] = "-v";
+		argv[argc++] = "-v";
+	}
+	if (args->depth) {
+		struct strbuf buf = STRBUF_INIT;
+		strbuf_addf(&buf, "--depth=%d", args->depth);
+		depth_arg = strbuf_detach(&buf, NULL);
+		argv[argc++] = depth_arg;
+	}
+	argv[argc++] = url;
+	for (i = 0; i < nr_heads; i++) {
+		struct ref *ref = to_fetch[i];
+		if (!ref->name || !*ref->name)
+			die("cannot fetch by sha1 over smart http");
+		argv[argc++] = ref->name;
+	}
+	argv[argc++] = NULL;
+
+	err = one_shot_rpc_service("git-upload-pack",
+		args->verbose,
+		argv, heads, &res);
+	if (res.len)
+		safe_write(1, res.buf, res.len);
+	strbuf_release(&res);
+
+	free(argv);
+	free(depth_arg);
+	return err;
+}
+
+static int fetch(struct fetch_args *args,
+	int nr_heads, struct ref **to_fetch)
+{
+	struct discovery *d = discover_refs("git-upload-pack");
+	if (d->proto_git)
+		return fetch_git(args, d, nr_heads, to_fetch);
+	else 
+		return fetch_dumb(args, nr_heads, to_fetch);
+}
+
 static void parse_fetch(struct strbuf *buf, int multiple)
 {
 	struct ref **to_fetch = NULL;
@@ -470,6 +533,12 @@ static void parse_fetch(struct strbuf *buf, int multiple)
 		}
 		else if (!strcmp(buf->buf, "option verbose"))
 			args.verbose = 1;
+		else if (!strcmp(buf->buf, "option followtags"))
+			args.followtags = 1;
+		else if (!strcmp(buf->buf, "option thin"))
+			args.thin = 1;
+		else if (!prefixcmp(buf->buf, "option depth "))
+			args.depth = atoi(buf->buf + strlen("option depth "));
 		else
 			die("http transport does not support %s", buf->buf);
 
@@ -480,7 +549,7 @@ static void parse_fetch(struct strbuf *buf, int multiple)
 			break;
 	} while (1);
 
-	if (fetch_dumb(&args, nr_heads, to_fetch))
+	if (fetch(&args, nr_heads, to_fetch))
 		exit(128); /* error already reported */
 	free_refs(list_head);
 	free(to_fetch);
@@ -645,7 +714,9 @@ int main(int argc, const char **argv)
 			printf("fetch\n");
 			printf("fetch-multiple\n");
 			printf("push\n");
+			printf("option depth\n");
 			printf("option dry-run\n");
+			printf("option followtags\n");
 			printf("option thin\n");
 			printf("option verbose\n");
 			printf("\n");
-- 
1.6.5.52.g0ff2e

[RFC PATCH v2 16/16] Smart HTTP fetch: gzip requests

[Shawn O. Pearce]

The upload-pack requests are mostly plain text and they compress
rather well.  Deflating them with Content-Encoding: gzip can easily
drop the size of the request by 50%, reducing the amount of data
to transfer as we negotiate the common commits.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
CC: Daniel Barkalow <barkalow@iabervon.org>
---
 remote-curl.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 31d1d34..d53215d 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -261,11 +261,12 @@ static size_t rpc_in(const void *ptr, size_t eltsize,
 	return size;
 }
 
-static int post_rpc(struct rpc_state *state)
+static int post_rpc(struct rpc_state *state, int use_gzip)
 {
 	struct active_request_slot *slot;
 	struct slot_results results;
 	struct curl_slist *headers = NULL;
+	unsigned char *gzip_body = NULL;
 	int err = 0, large_request = 0;
 
 	/* Try to load the entire request, if we can fit it into the
@@ -279,6 +280,7 @@ static int post_rpc(struct rpc_state *state)
 
 		if (left < LARGE_PACKET_MAX) {
 			large_request = 1;
+			use_gzip = 0;
 			break;
 		}
 
@@ -311,6 +313,48 @@ static int post_rpc(struct rpc_state *state)
 			fflush(stderr);
 		}
 
+	} else if (use_gzip && 1024 < state->len) {
+		/* The client backend isn't giving us compressed data so
+		 * we can try to deflate it ourselves, this may save on.
+		 * the transfer time.
+		 */
+		size_t size;
+		z_stream stream;
+		int ret;
+
+		memset(&stream, 0, sizeof(stream));
+		ret = deflateInit2(&stream, Z_DEFAULT_COMPRESSION,
+				Z_DEFLATED, (15 + 16),
+				8, Z_DEFAULT_STRATEGY);
+		if (ret != Z_OK)
+			die("cannot deflate request; zlib init error %d", ret);
+		size = deflateBound(&stream, state->len);
+		gzip_body = xmalloc(size);
+
+		stream.next_in = (unsigned char *)state->buf;
+		stream.avail_in = state->len;
+		stream.next_out = gzip_body;
+		stream.avail_out = size;
+
+		ret = deflate(&stream, Z_FINISH);
+		if (ret != Z_STREAM_END)
+			die("cannot deflate request; zlib deflate error %d", ret);
+
+		ret = deflateEnd(&stream);
+		if (ret != Z_OK)
+			die("cannot deflate request; zlib end error %d", ret);
+
+		size = stream.total_out;
+
+		headers = curl_slist_append(headers, "Content-Encoding: gzip");
+		curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);
+		curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, size);
+
+		if (state->verbose) {
+			fprintf(stderr, "POST %s (gzip %lu to %lu bytes)\n",
+				state->service_name, state->len, size);
+			fflush(stderr);
+		}
 	} else {
 		/* We know the complete request size in advance, use the
 		 * more normal Content-Length approach.
@@ -336,11 +380,13 @@ static int post_rpc(struct rpc_state *state)
 		}
 	}
 	curl_slist_free_all(headers);
+	free(gzip_body);
 	return err;
 }
 
 static int one_shot_rpc_service(const char *service,
 	int verbose,
+	int use_gzip,
 	const char **client_argv,
 	struct discovery *heads,
 	struct strbuf *result)
@@ -384,7 +430,7 @@ static int one_shot_rpc_service(const char *service,
 			break;
 		state.pos = 0;
 		state.len = n;
-		err |= post_rpc(&state);
+		err |= post_rpc(&state, use_gzip);
 	}
 	if (result)
 		strbuf_read(result, client.out, 0);
@@ -474,6 +520,7 @@ static int fetch_git(struct fetch_args *args,
 
 	err = one_shot_rpc_service("git-upload-pack",
 		args->verbose,
+		1 /* gzip request */,
 		argv, heads, &res);
 	if (res.len)
 		safe_write(1, res.buf, res.len);
@@ -611,6 +658,7 @@ static int push_git(struct discovery *heads,
 
 	err = one_shot_rpc_service("git-receive-pack",
 		args->verbose,
+		0 /* no gzip */,
 		argv, heads, &res);
 	if (res.len)
 		safe_write(1, res.buf, res.len);
-- 
1.6.5.52.g0ff2e

Re: [RFC PATCH v2 16/16] Smart HTTP fetch: gzip requests

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> diff --git a/remote-curl.c b/remote-curl.c
> index 31d1d34..d53215d 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -261,11 +261,12 @@ static size_t rpc_in(const void *ptr, size_t eltsize,
>  	return size;
>  }
>  
> -static int post_rpc(struct rpc_state *state)
> +static int post_rpc(struct rpc_state *state, int use_gzip)
>  {
>  	struct active_request_slot *slot;
>  	struct slot_results results;
>  	struct curl_slist *headers = NULL;
> +	unsigned char *gzip_body = NULL;
>  	int err = 0, large_request = 0;
>  
>  	/* Try to load the entire request, if we can fit it into the
> @@ -311,6 +313,48 @@ static int post_rpc(struct rpc_state *state)
> ...
> +		curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);

cc1: warnings being treated as errors
In function 'post_rpc',
    inlined from 'one_shot_rpc_service' at remote-curl.c:433:
remote-curl.c:350: error: call to '_curl_easy_setopt_err_postfields' declared with attribute warning: curl_easy_setopt expects a void* or char* argument for this option
make: *** [remote-curl.o] Error 1

Re: [RFC PATCH v2 00/16] Return of smart HTTP

[eduard stefan]

Shawn O. Pearce wrote:
> The series has gotten a lot larger since my last posting, but I have
> what appears to be a fully working client *AND* server implementation
> for both fetch and push, and the client should be supporting deeping
> shallow repositories over the smart variant of HTTP.
> 
> I've dropped the documentation patch from the series for now as
> I have quite a few edits queued up from folks in the last round
> (thanks for those!) that I have not yet applied.  So there is no
> point in sending that particular patch again.

I have apllied you patches on top of Git 1.6.5 release,
and they solved my http cloning crashes on windows
(msysGit environment).

Now git http cloning works just fine for local http servers,
but on GitHub I get an 502 error:
"error: Unable to get pack file
http://github.com/loudej/spark.git/objects/pack/
pack-1bc121e71e2847622f814603ddb34444bfc6a16c.pack
The requested URL returned error: 502"
which seems to be more like a GitHub problem,
since that URL returns an error when accessing it with a browser.

Have a nice day,
  Eduard

Re: [RFC PATCH v2 00/16] Return of smart HTTP

[Junio C Hamano]

eduard stefan <eduard.stefan@gmail.com> writes:

> I have apllied you patches on top of Git 1.6.5 release,
> and they solved my http cloning crashes on windows
> (msysGit environment).

Unfortunately that is not a very good news.

As there is no chance Shawn's new series will be applied to 'maint' for
updating 1.6.5.X series, somebody needs to address the breakage without
introducing the whole new HTTP code.  Is the breakage you are seeing
limited only to msysgit environment, or does the same breakage appear on
other platforms?