On Tue, 6 Feb 2018 17:12:57 -0800
Brandon Williams <bmw...@google.com> wrote:
> + 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).
> + 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;
> +}
