From: Shawn Pearce <spearce@spearce.org>
Date: Wed, 07 Sep 2011 23:23:13 GMT
Subject: Re: [PATCH 2/2] push -s: skeleton
Message-ID: <CAJo=hJsLx1Q9ZDoxGn=dww5J-rO9GitH47rEme_1L8Lg0RmAqw@mail.gmail.com>
In-Reply-To: <7vpqjc0zaf.fsf@alter.siamese.dyndns.org>

On Wed, Sep 7, 2011 at 15:21, Junio C Hamano <gitster@pobox.com> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
>
>> Yes. Above we flushed the req_buf and send that in an HTTP request.
>> You need to hoist this block above the "if (args->stateless_rpc)"
>> segment.
>
> What do you mean by "hoist"? For the req advertisement, it seems that you
> are not hoisting anything but duplicating the code, turning safe_write()
> followed by flush into packet-buf-flush and sending the result over the
> sideband. Shouldn't this new data be sent over the sideband-to-http the
> same way?
>
> Unless you do not want signed push over http, that is...

We do.

> diff --git a/builtin/send-pack.c b/builtin/send-pack.c
> index 3193f34..37e0313 100644
> --- a/builtin/send-pack.c
> +++ b/builtin/send-pack.c
> @@ -379,9 +379,13 @@ int send_pack(struct send_pack_args *args,
>                        packet_buf_write(&req_buf, "%.*s",
>                                         (int)(ep - cp), cp);
>                }
> -               /* Do we need anything funky for stateless rpc? */
> -               safe_write(out, req_buf.buf, req_buf.len);
> -               packet_flush(out);
> +               if (args->stateless_rpc) {
> +                       packet_buf_flush(&req_buf);
> +                       send_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);
> +               } else {
> +                       safe_write(out, req_buf.buf, req_buf.len);
> +                       packet_flush(out);
> +               }

This sounds too late to me.  I think you just caused 2 HTTP POSTs, one
a partial one with the commands and no pack data, and another with the
push certificate and the pack. Neither is useful.

-- 
Shawn.