On Wed, 28 Feb 2018 15:22:37 -0800
Brandon Williams <bmw...@google.com> wrote:

> +    output = *section
> +    section = (acknowledgments | packfile)
> +           (flush-pkt | delim-pkt)
> +
> +    acknowledgments = PKT-LINE("acknowledgments" LF)
> +                   (nak | *ack)
> +                   (ready)
> +    ready = PKT-LINE("ready" LF)
> +    nak = PKT-LINE("NAK" LF)
> +    ack = PKT-LINE("ACK" SP obj-id LF)
> +
> +    packfile = PKT-LINE("packfile" LF)
> +            [PACKFILE]

I should have noticed this earlier, but "PACKFILE" is not defined anywhere -
it's probably better written as:

    *PKT-LINE(%x01-03 *%x00-ff)"

or something like that.

> +    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 MUST be
> +       omitted from the server's response.
> +
> +     * 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 the
> +       section header
> +
> +     * The data transfer of the packfile is always multiplexed, using
> +       the same semantics of the 'side-band-64k' capability from
> +       protocol version 1.  This means that each packet, during the
> +       packfile data stream, is made up of a leading 4-byte pkt-line
> +       length (typical of the pkt-line format), followed by a 1-byte
> +       stream code, followed by the actual data.
> +
> +       The stream code can be one of:
> +             1 - pack data
> +             2 - progress messages
> +             3 - fatal error message just before stream aborts
> +
> +     * This section is only included if the client has sent 'want'
> +       lines in its request and either requested that no more
> +       negotiation be done by sending 'done' or if the server has
> +       decided it has found a sufficient cut point to produce a
> +       packfile.

For both the sections, I think that the conditions for
inclusion/non-inclusion ("This section is only included if...") should
be the first point.

> +static void upload_pack_data_init(struct upload_pack_data *data)
> +{
> +     struct object_array wants = OBJECT_ARRAY_INIT;
> +     struct oid_array haves = OID_ARRAY_INIT;
> +
> +     memset(data, 0, sizeof(*data));
> +     data->wants = wants;
> +     data->haves = haves;
> +}

Any reason to use a initializer function instead of a static literal?

Reply via email to