Masaya Suzuki <masayasuzuki@xxxxxxxxxx> writes:
> In the Git pack protocol definition, an error packet may appear only in
> a certain context. However, servers can face a runtime error (e.g. I/O
> error) at an arbitrary timing. This patch changes the protocol to allow
> an error packet to be sent instead of any packet.
Makes perfect sense.
> Following this protocol spec change, the error packet handling code is
> moved to pkt-line.c.
>
> Signed-off-by: Masaya Suzuki <masayasuzuki@xxxxxxxxxx>
> ---
> Documentation/technical/pack-protocol.txt | 20 +++++++++++---------
> builtin/archive.c | 2 --
> connect.c | 2 --
> fetch-pack.c | 2 --
> pkt-line.c | 4 ++++
> 5 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
> index 6ac774d5f..7a2375a55 100644
> --- a/Documentation/technical/pack-protocol.txt
> +++ b/Documentation/technical/pack-protocol.txt
> @@ -22,6 +22,16 @@ protocol-common.txt. When the grammar indicate `PKT-LINE(...)`, unless
> otherwise noted the usual pkt-line LF rules apply: the sender SHOULD
> include a LF, but the receiver MUST NOT complain if it is not present.
>
> +An error packet is a special pkt-line that contains an error string.
> +
> +----
> + error-line = PKT-LINE("ERR" SP explanation-text)
> +----
> +
> +Throughout the protocol, where `PKT-LINE(...)` is expected, an error packet MAY
> +be sent. Once this packet is sent by a client or a server, the data transfer
> +process defined in this protocol is terminated.
> +
> Transports
> ----------
> There are three transports over which the packfile protocol is
> @@ -89,13 +99,6 @@ process on the server side over the Git protocol is this:
> "0039git-upload-pack /schacon/gitbook.git\0host=example.com\0" |
> nc -v example.com 9418
>
> -If the server refuses the request for some reasons, it could abort
> -gracefully with an error message.
> -
> -----
> - error-line = PKT-LINE("ERR" SP explanation-text)
> -----
> -
>
> SSH Transport
> -------------
> @@ -398,12 +401,11 @@ from the client).
> Then the server will start sending its packfile data.
>
> ----
> - server-response = *ack_multi ack / nak / error-line
> + server-response = *ack_multi ack / nak
> ack_multi = PKT-LINE("ACK" SP obj-id ack_status)
> ack_status = "continue" / "common" / "ready"
> ack = PKT-LINE("ACK" SP obj-id)
> nak = PKT-LINE("NAK")
> - error-line = PKT-LINE("ERR" SP explanation-text)
> ----
>
> A simple clone may look like this (with no 'have' lines):
> diff --git a/builtin/archive.c b/builtin/archive.c
> index d2455237c..5d179bbd1 100644
> --- a/builtin/archive.c
> +++ b/builtin/archive.c
> @@ -59,8 +59,6 @@ static int run_remote_archiver(int argc, const char **argv,
> 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);
> die(_("git archive: protocol error"));
> }
>
> diff --git a/connect.c b/connect.c
> index 24281b608..458906e60 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -306,8 +306,6 @@ struct ref **get_remote_heads(struct packet_reader *reader,
> die_initial_contact(1);
> case PACKET_READ_NORMAL:
> len = reader->pktlen;
> - if (len > 4 && skip_prefix(reader->line, "ERR ", &arg))
> - die(_("remote error: %s"), arg);
> break;
> case PACKET_READ_FLUSH:
> state = EXPECTING_DONE;
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 9691046e6..e66cd7b71 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -178,8 +178,6 @@ static enum ack_type get_ack(int fd, struct object_id *result_oid)
> return ACK;
> }
> }
> - if (skip_prefix(line, "ERR ", &arg))
> - die(_("remote error: %s"), arg);
> die(_("git fetch-pack: expected ACK/NAK, got '%s'"), line);
> }
>
> diff --git a/pkt-line.c b/pkt-line.c
> index 04d10bbd0..ce9e42d10 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -346,6 +346,10 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
> return PACKET_READ_EOF;
> }
>
> + if (starts_with(buffer, "ERR ")) {
> + die(_("remote error: %s"), buffer + 4);
> + }
> +
> if ((options & PACKET_READ_CHOMP_NEWLINE) &&
> len && buffer[len-1] == '\n')
> len--;
