[PATCH v2 0/4] Add proto v2 archive command with HTTP support

This is the second version of my series to add a new protocol v2 command for archiving, with support for HTTP(S).

NEEDSWORK: a server built with this series is not backwards-compatible
with clients that set GIT_PROTOCOL=version=2 or configure
protocol.version=2. The old client will unconditionally send "argument
..." packet lines, which breaks the server's expectations of a
"command=archive" request, while the server's capability advertisement
in turn breaks the clients expectation of either an ACK or NACK.

I've been discussing workarounds for this with Jonathan Nieder, but please let me know if you have any suggestions for v3 of this series.

Josh Steadmon (4):
  archive: follow test standards around assertions
  archive: use packet_reader for communications
  archive: implement protocol v2 archive command
  archive: allow archive over HTTP(S) with proto v2

 Documentation/technical/protocol-v2.txt | 21 ++++++++-
 builtin/archive.c                       | 58 +++++++++++++++++++------
 builtin/upload-archive.c                | 27 ++++++++++--
 http-backend.c                          | 13 +++++-
 serve.c                                 |  7 +++
 t/t5000-tar-tree.sh                     | 33 +++++++-------
 t/t5701-git-serve.sh                    |  1 +
 transport-helper.c                      |  7 +--
 8 files changed, 130 insertions(+), 37 deletions(-)

Range-diff against v1:
-:  ---------- > 1:  c2e371ad24 archive: follow test standards around assertions
1:  b514184273 ! 2:  a65f73f627 archive: use packet_reader for communications
    @@ -6,7 +6,10 @@
         handling, which will make implementation of protocol v2 support in
         git-archive easier.
     
    +    This refactoring does not change the behavior of "git archive".
    +
         Signed-off-by: Josh Steadmon <steadmon@google.com>
    +    Reviewed-by: Stefan Beller <sbeller@google.com>
     
      diff --git a/builtin/archive.c b/builtin/archive.c
    @@ -42,24 +45,24 @@
     -	if (!buf)
     +	status = packet_reader_read(&reader);
     +
    -+	if (status == PACKET_READ_FLUSH)
    ++	if (status != PACKET_READ_NORMAL || reader.pktlen <= 0)
      		die(_("git archive: expected ACK/NAK, got a flush packet"));
     -	if (strcmp(buf, "ACK")) {
     -		if (starts_with(buf, "NACK "))
     -			die(_("git archive: NACK %s"), buf + 5);
     -		if (starts_with(buf, "ERR "))
     -			die(_("remote error: %s"), buf + 4);
    -+	if (strcmp(reader.buffer, "ACK")) {
    -+		if (starts_with(reader.buffer, "NACK "))
    -+			die(_("git archive: NACK %s"), reader.buffer + 5);
    -+		if (starts_with(reader.buffer, "ERR "))
    -+			die(_("remote error: %s"), reader.buffer + 4);
    ++	if (strcmp(reader.line, "ACK")) {
    ++		if (starts_with(reader.line, "NACK "))
    ++			die(_("git archive: NACK %s"), reader.line + 5);
    ++		if (starts_with(reader.line, "ERR "))
    ++			die(_("remote error: %s"), reader.line + 4);
      		die(_("git archive: protocol error"));
      	}
      
     -	if (packet_read_line(fd[0], NULL))
     +	status = packet_reader_read(&reader);
    -+	if (status != PACKET_READ_FLUSH)
    ++	if (status == PACKET_READ_NORMAL && reader.pktlen > 0)
      		die(_("git archive: expected a flush"));
      
      	/* Now, start reading from fd[0] and spit it out to stdout */
2:  1518c15dc1 < -:  ---------- archive: implement protocol v2 archive command
-:  ---------- > 3:  0a8cc5e331 archive: implement protocol v2 archive command
3:  1b7ad8d8f6 ! 4:  97a1424f32 archive: allow archive over HTTP(S) with proto v2
    @@ -10,16 +10,20 @@
      +++ b/builtin/archive.c
     @@
      		status = packet_reader_read(&reader);
    - 		if (status != PACKET_READ_FLUSH)
    + 		if (status == PACKET_READ_NORMAL && reader.pktlen > 0)
      			die(_("git archive: expected a flush"));
     -	}
     +	} else if (version == protocol_v2 &&
    -+		   starts_with(transport->url, "http"))
    ++		   (starts_with(transport->url, "http://") ||
    ++		    starts_with(transport->url, "https://")))
     +		/*
     +		 * Commands over HTTP require two requests, so there's an
    -+		 * additional server response to parse.
    ++		 * additional server response to parse. We do only basic sanity
    ++		 * checking here that the versions presented match across
    ++		 * requests.
     +		 */
    -+		discover_version(&reader);
    ++		if (version != discover_version(&reader))
    ++			die(_("git archive: received different protocol versions in subsequent requests"));
      
      	/* Now, start reading from fd[0] and spit it out to stdout */
      	rv = recv_sideband("archive", fd[0], 1);
    @@ -40,7 +44,10 @@
      	struct strbuf buf = STRBUF_INIT;
      
     +	if (!strcmp(service_name, "git-upload-archive")) {
    -+		/* git-upload-archive doesn't need --stateless-rpc */
    ++		/*
    ++		 * git-upload-archive doesn't need --stateless-rpc, because it
    ++		 * always handles only a single request.
    ++		 */
     +		argv[1] = ".";
     +		argv[2] = NULL;
     +	}
    @@ -63,10 +70,12 @@
      --- a/transport-helper.c
      +++ b/transport-helper.c
     @@
    + 		strbuf_addf(&cmdbuf, "connect %s\n", name);
      		ret = run_connect(transport, &cmdbuf);
      	} else if (data->stateless_connect &&
    - 		   (get_protocol_version_config() == protocol_v2) &&
    +-		   (get_protocol_version_config() == protocol_v2) &&
     -		   !strcmp("git-upload-pack", name)) {
    ++		   get_protocol_version_config() == protocol_v2 &&
     +		   (!strcmp("git-upload-pack", name) ||
     +		    !strcmp("git-upload-archive", name))) {
      		strbuf_addf(&cmdbuf, "stateless-connect %s\n", name);