Junio, I noticed that this did not make it into your "What's cooking"
e-mail [1] - is there anything more that you would like to see in this
patch?
Jonathan Nieder has reviewed an earlier version, and seems to be OK with
it. He recommended a change in the error message, which I have changed
in this new version. I have also done one of his follow-up ideas
(updating the documentation) in this patch.
[1]
http://public-inbox.org/git/xmqqmvbfij92.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxx/
On 04/12/2017 11:06 AM, Jonathan Tan wrote:
Currently, fetch-pack prints a confusing error message ("expected
ACK/NAK") when the server it's communicating with sends a pkt-line
starting with "ERR". Replace it with a less confusing error message.
Also update the documentation describing the fetch-pack/upload-pack
protocol (pack-protocol.txt) to indicate that "ERR" can be sent in the
place of "ACK" or "NAK". In practice, this has been done for quite some
time by other Git implementations (e.g. JGit sends "want $id not valid")
and by Git itself (since commit bdb31ea: "upload-pack: report "not our
ref" to client", 2017-02-23) whenever a "want" line references an object
that it does not have. (This is uncommon, but can happen if a repository
is garbage-collected during a negotiation.)
Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
---
Changes from v1:
- used jrneider's suggested change to error message
- added documentation about "ERR" in protocol
- updated commit message to explain documentation change
Documentation/technical/pack-protocol.txt | 7 ++++++-
fetch-pack.c | 2 ++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
index c59ac9936..5b0ba3ef2 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -351,14 +351,19 @@ ACK after 'done' if there is at least one common base and multi_ack or
multi_ack_detailed is enabled. The server always sends NAK after 'done'
if there is no common base found.
+Instead of 'ACK' or 'NAK', the server may send an error message (for
+example, if it does not recognize an object in a 'want' line received
+from the client).
+
Then the server will start sending its packfile data.
----
- server-response = *ack_multi ack / nak
+ server-response = *ack_multi ack / nak / error-line
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/fetch-pack.c b/fetch-pack.c
index d07d85ce3..4bed270c5 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -276,6 +276,8 @@ static enum ack_type get_ack(int fd, unsigned char *result_sha1)
return ACK;
}
}
+ if (skip_prefix(line, "ERR ", &arg))
+ die(_("remote error: %s"), arg);
die(_("git fetch-pack: expected ACK/NAK, got '%s'"), line);
}