Re: [PATCH v3 20/35] upload-pack: introduce fetch server command
On 02/21, Jonathan Tan wrote:
> On Tue, 6 Feb 2018 17:12:57 -0800
> Brandon Williams wrote:
>
> > +want
> > + Indicates to the server an object which the client wants to
> > + retrieve.
>
> Mention that the client can "want" anything even if not advertised by
> the server (like uploadpack.allowanysha1inwant).
Will do.
>
> > +output = *section
> > +section = (acknowledgments | packfile)
> > + (flush-pkt | delim-pkt)
> > +
> > +acknowledgments = PKT-LINE("acknowledgments" LF)
> > + *(ready | nak | ack)
>
> Can this part be described more precisely in the BNF section? I see that
> you describe later that there can be multiple ACKs or one NAK (but not
> both), and "ready" can be sent regardless of whether ACKs or a NAK is
> sent.
Yep I'll fix that.
>
> > +ready = PKT-LINE("ready" LF)
> > +nak = PKT-LINE("NAK" LF)
> > +ack = PKT-LINE("ACK" SP obj-id LF)
> > +
> > +packfile = PKT-LINE("packfile" LF)
> > + [PACKFILE]
> > +
> > +
> > +acknowledgments section
> > + * Always begins with the section header "acknowledgments"
> > +
> > + * The server will respond with "NAK" if none of the object ids sent
> > + as have lines were common.
> > +
> > + * The server will respond with "ACK obj-id" for all of the
> > + object ids sent as have lines which are common.
> > +
> > + * A response cannot have both "ACK" lines as well as a "NAK"
> > + line.
> > +
> > + * The server will respond with a "ready" line indicating that
> > + the server has found an acceptable common base and is ready to
> > + make and send a packfile (which will be found in the packfile
> > + section of the same response)
> > +
> > + * If the client determines that it is finished with negotiations
> > + by sending a "done" line, the acknowledgments sections can be
> > + omitted from the server's response as an optimization.
>
> Should this be changed to "must"? The current implementation does not
> support it (on the client side).
This is actually a great question and one which may need to be thought
about in terms of its application to future extensions to the fetch
command. Since fetch's response is now broken up into sections we may
want the client to cope with sections being in any order and maybe even
skipping sections it doesn't know about. Not sure if its necessary but
its an idea.
>
> > +#define UPLOAD_PACK_DATA_INIT { OBJECT_ARRAY_INIT, OID_ARRAY_INIT, 0, 0,
> > 0, 0, 0, 0 }
>
> Optional: the trailing zeroes can be omitted. (That's shorter, and also
> easier to maintain when we add new fields.)
>
> > +int upload_pack_v2(struct repository *r, struct argv_array *keys,
> > + struct argv_array *args)
> > +{
> > + enum fetch_state state = FETCH_PROCESS_ARGS;
> > + struct upload_pack_data data = UPLOAD_PACK_DATA_INIT;
> > + use_sideband = LARGE_PACKET_MAX;
> > +
> > + while (state != FETCH_DONE) {
> > + switch (state) {
> > + case FETCH_PROCESS_ARGS:
> > + process_args(args, &data);
> > +
> > + if (!want_obj.nr) {
> > + /*
> > +* Request didn't contain any 'want' lines,
> > +* guess they didn't want anything.
> > +*/
> > + state = FETCH_DONE;
> > + } else if (data.haves.nr) {
> > + /*
> > +* Request had 'have' lines, so lets ACK them.
> > +*/
> > + state = FETCH_SEND_ACKS;
> > + } else {
> > + /*
> > +* Request had 'want's but no 'have's so we can
> > +* immedietly go to construct and send a pack.
> > +*/
> > + state = FETCH_SEND_PACK;
> > + }
> > + break;
> > + case FETCH_READ_HAVES:
> > + read_haves(&data);
> > + state = FETCH_SEND_ACKS;
> > + break;
>
> This branch seems to never be taken?
Must be left over from another version, I'll remove it.
>
> > + case FETCH_SEND_ACKS:
> > + if (process_haves_and_send_acks(&data))
> > + state = FETCH_SEND_PACK;
> > + else
> > + state = FETCH_DONE;
> > + break;
> > + case FETCH_SEND_PACK:
> > + packet_write_fmt(1, "packfile\n");
> > + create_pack_file();
> > + state = FETCH_DONE;
> > + break;
> > + case FETCH_DONE:
> > + continue;
> > + }
> > + }
> > +
> > + upload_pack_data_clear(&data);
> > + return 0;
> > +}
--
Brandon Williams
Re: [PATCH v3 20/35] upload-pack: introduce fetch server command
On Tue, 6 Feb 2018 17:12:57 -0800
Brandon Williams wrote:
> +want
> + Indicates to the server an object which the client wants to
> + retrieve.
Mention that the client can "want" anything even if not advertised by
the server (like uploadpack.allowanysha1inwant).
> +output = *section
> +section = (acknowledgments | packfile)
> + (flush-pkt | delim-pkt)
> +
> +acknowledgments = PKT-LINE("acknowledgments" LF)
> + *(ready | nak | ack)
Can this part be described more precisely in the BNF section? I see that
you describe later that there can be multiple ACKs or one NAK (but not
both), and "ready" can be sent regardless of whether ACKs or a NAK is
sent.
> +ready = PKT-LINE("ready" LF)
> +nak = PKT-LINE("NAK" LF)
> +ack = PKT-LINE("ACK" SP obj-id LF)
> +
> +packfile = PKT-LINE("packfile" LF)
> +[PACKFILE]
> +
> +
> +acknowledgments section
> + * Always begins with the section header "acknowledgments"
> +
> + * The server will respond with "NAK" if none of the object ids sent
> + as have lines were common.
> +
> + * The server will respond with "ACK obj-id" for all of the
> + object ids sent as have lines which are common.
> +
> + * A response cannot have both "ACK" lines as well as a "NAK"
> + line.
> +
> + * The server will respond with a "ready" line indicating that
> + the server has found an acceptable common base and is ready to
> + make and send a packfile (which will be found in the packfile
> + section of the same response)
> +
> + * If the client determines that it is finished with negotiations
> + by sending a "done" line, the acknowledgments sections can be
> + omitted from the server's response as an optimization.
Should this be changed to "must"? The current implementation does not
support it (on the client side).
> +#define UPLOAD_PACK_DATA_INIT { OBJECT_ARRAY_INIT, OID_ARRAY_INIT, 0, 0, 0,
> 0, 0, 0 }
Optional: the trailing zeroes can be omitted. (That's shorter, and also
easier to maintain when we add new fields.)
> +int upload_pack_v2(struct repository *r, struct argv_array *keys,
> +struct argv_array *args)
> +{
> + enum fetch_state state = FETCH_PROCESS_ARGS;
> + struct upload_pack_data data = UPLOAD_PACK_DATA_INIT;
> + use_sideband = LARGE_PACKET_MAX;
> +
> + while (state != FETCH_DONE) {
> + switch (state) {
> + case FETCH_PROCESS_ARGS:
> + process_args(args, &data);
> +
> + if (!want_obj.nr) {
> + /*
> + * Request didn't contain any 'want' lines,
> + * guess they didn't want anything.
> + */
> + state = FETCH_DONE;
> + } else if (data.haves.nr) {
> + /*
> + * Request had 'have' lines, so lets ACK them.
> + */
> + state = FETCH_SEND_ACKS;
> + } else {
> + /*
> + * Request had 'want's but no 'have's so we can
> + * immedietly go to construct and send a pack.
> + */
> + state = FETCH_SEND_PACK;
> + }
> + break;
> + case FETCH_READ_HAVES:
> + read_haves(&data);
> + state = FETCH_SEND_ACKS;
> + break;
This branch seems to never be taken?
> + case FETCH_SEND_ACKS:
> + if (process_haves_and_send_acks(&data))
> + state = FETCH_SEND_PACK;
> + else
> + state = FETCH_DONE;
> + break;
> + case FETCH_SEND_PACK:
> + packet_write_fmt(1, "packfile\n");
> + create_pack_file();
> + state = FETCH_DONE;
> + break;
> + case FETCH_DONE:
> + continue;
> + }
> + }
> +
> + upload_pack_data_clear(&data);
> + return 0;
> +}
[PATCH v3 20/35] upload-pack: introduce fetch server command
Introduce the 'fetch' server command.
Signed-off-by: Brandon Williams
---
Documentation/technical/protocol-v2.txt | 127 +++
serve.c | 2 +
t/t5701-git-serve.sh| 1 +
upload-pack.c | 281
upload-pack.h | 5 +
5 files changed, 416 insertions(+)
diff --git a/Documentation/technical/protocol-v2.txt
b/Documentation/technical/protocol-v2.txt
index ef81df868..4d5096dae 100644
--- a/Documentation/technical/protocol-v2.txt
+++ b/Documentation/technical/protocol-v2.txt
@@ -144,3 +144,130 @@ The output of ls-refs is as follows:
ref-attribute = (symref | peeled)
symref = "symref-target:" symref-target
peeled = "peeled:" obj-id
+
+ fetch
+---
+
+`fetch` is the command used to fetch a packfile in v2. It can be looked
+at as a modified version of the v1 fetch where the ref-advertisement is
+stripped out (since the `ls-refs` command fills that role) and the
+message format is tweaked to eliminate redundancies and permit easy
+addition of future extensions.
+
+Additional features not supported in the base command will be advertised
+as the value of the command in the capability advertisement in the form
+of a space separated list of features, e.g. "=
+".
+
+A `fetch` request can take the following parameters wrapped in
+packet-lines:
+
+want
+ Indicates to the server an object which the client wants to
+ retrieve.
+
+have
+ Indicates to the server an object which the client has locally.
+ This allows the server to make a packfile which only contains
+ the objects that the client needs. Multiple 'have' lines can be
+ supplied.
+
+done
+ Indicates to the server that negotiation should terminate (or
+ not even begin if performing a clone) and that the server should
+ use the information supplied in the request to construct the
+ packfile.
+
+thin-pack
+ Request that a thin pack be sent, which is a pack with deltas
+ which reference base objects not contained within the pack (but
+ are known to exist at the receiving end). This can reduce the
+ network traffic significantly, but it requires the receiving end
+ to know how to "thicken" these packs by adding the missing bases
+ to the pack.
+
+no-progress
+ Request that progress information that would normally be sent on
+ side-band channel 2, during the packfile transfer, should not be
+ sent. However, the side-band channel 3 is still used for error
+ responses.
+
+include-tag
+ Request that annotated tags should be sent if the objects they
+ point to are being sent.
+
+ofs-delta
+ Indicate that the client understands PACKv2 with delta referring
+ to its base by position in pack rather than by an oid. That is,
+ they can read OBJ_OFS_DELTA (ake type 6) in a packfile.
+
+The response of `fetch` is broken into a number of sections separated by
+delimiter packets (0001), with each section beginning with its section
+header.
+
+output = *section
+section = (acknowledgments | packfile)
+ (flush-pkt | delim-pkt)
+
+acknowledgments = PKT-LINE("acknowledgments" LF)
+ *(ready | nak | ack)
+ready = PKT-LINE("ready" LF)
+nak = PKT-LINE("NAK" LF)
+ack = PKT-LINE("ACK" SP obj-id LF)
+
+packfile = PKT-LINE("packfile" LF)
+ [PACKFILE]
+
+
+acknowledgments section
+ * Always begins with the section header "acknowledgments"
+
+ * The server will respond with "NAK" if none of the object ids sent
+ as have lines were common.
+
+ * The server will respond with "ACK obj-id" for all of the
+ object ids sent as have lines which are common.
+
+ * A response cannot have both "ACK" lines as well as a "NAK"
+ line.
+
+ * The server will respond with a "ready" line indicating that
+ the server has found an acceptable common base and is ready to
+ make and send a packfile (which will be found in the packfile
+ section of the same response)
+
+ * If the client determines that it is finished with negotiations
+ by sending a "done" line, the acknowledgments sections can be
+ omitted from the server's response as an optimization.
+
+ * If the server has found a suitable cut point and has decided
+ to send a "ready" line, then the server can decide to (as an
+ optimization) omit any "ACK" lines it would have sent during
+ its response. This is because the server will have already
+ determined the objects it plans to send to the client and no
+ further negotiation is needed.
+
+
+packfile section
+ * Always begins with the section header "packfile"
+
+ * The transmission of the packfile begins immediately after
