Re: [PATCH v3 20/35] upload-pack: introduce fetch server command
From: Jonathan Tan <hidden>
Date: 2018-02-21 23:46:33
On Tue, 6 Feb 2018 17:12:57 -0800 Brandon Williams [off-list ref] wrote:
quoted hunk ↗ jump to hunk
+ want <oid> + 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).
quoted hunk ↗ jump to hunk
+ 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.
quoted hunk ↗ jump to hunk
+ 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.)
quoted hunk ↗ jump to hunk
+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?
quoted hunk ↗ jump to hunk
+ 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; +}