git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Josh Steadmon <steadmon@google.com>
To: git@vger.kernel.org
Cc: Josh Steadmon <steadmon@google.com>
Subject: [PATCH v2 0/4] Add proto v2 archive command with HTTP support
Date: Wed, 26 Sep 2018 18:24:51 -0700	[thread overview]
Message-ID: <20180927012455.234876-1-steadmon@google.com> (raw)
In-Reply-To: <20180912053519.31085-1-steadmon@google.com>

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);
-- 
2.19.0.605.g01d371f741-goog


  parent reply	other threads:[~2018-09-27  1:25 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-12  5:35 Add proto v2 archive command with HTTP support Josh Steadmon
2018-09-12  5:35 ` [PATCH 1/3] archive: use packet_reader for communications Josh Steadmon
2018-09-12 22:01   ` Stefan Beller
2018-09-13 14:58   ` Junio C Hamano
2018-09-13 15:34     ` Junio C Hamano
2018-09-12  5:35 ` [PATCH 2/3] archive: implement protocol v2 archive command Josh Steadmon
2018-09-12 22:28   ` Stefan Beller
2018-09-13 18:45     ` Ævar Arnfjörð Bjarmason
2018-09-14  6:05       ` Jonathan Nieder
2018-09-14 14:31         ` Ævar Arnfjörð Bjarmason
2018-09-14 16:14         ` Junio C Hamano
2018-09-14 16:19           ` Jonathan Nieder
2018-09-13 16:31   ` Junio C Hamano
2018-09-14  5:39   ` Jonathan Nieder
2018-09-12  5:35 ` [PATCH 3/3] archive: allow archive over HTTP(S) with proto v2 Josh Steadmon
2018-09-12 22:38   ` Stefan Beller
2018-09-13 16:47   ` Junio C Hamano
2018-09-27 20:28     ` Josh Steadmon
2018-09-14  5:57   ` Jonathan Nieder
2018-09-14  5:36 ` Add proto v2 archive command with HTTP support Jonathan Nieder
2018-09-27  1:24 ` Josh Steadmon [this message]
2018-09-27  1:24   ` [PATCH v2 1/4] archive: follow test standards around assertions Josh Steadmon
2018-09-27 18:38     ` Stefan Beller
2018-09-27  1:24   ` [PATCH v2 2/4] archive: use packet_reader for communications Josh Steadmon
2018-09-27 18:42     ` Stefan Beller
2018-09-27  1:24   ` [PATCH v2 3/4] archive: implement protocol v2 archive command Josh Steadmon
2018-09-27  1:24   ` [PATCH v2 4/4] archive: allow archive over HTTP(S) with proto v2 Josh Steadmon
2018-09-27 18:20   ` [PATCH v2 0/4] Add proto v2 archive command with HTTP support Stefan Beller
2018-09-27 18:30     ` Jonathan Nieder
2018-09-27 22:20       ` Junio C Hamano
2018-09-27 22:33         ` Josh Steadmon
2018-09-28  1:25           ` Junio C Hamano
2018-09-27 18:30     ` Josh Steadmon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180927012455.234876-1-steadmon@google.com \
    --to=steadmon@google.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).