Re: [PATCHv2 0/10] pkt-line and remote-curl cleanups server
- From
Shawn Pearce <spearce@spearce.org>
- Date
- Feb 20, 2013, 07:05 UTC
- Message-ID
- <CAJo=hJvrc3EUfzr2BDZ3i68rwZVeHFGHmKwrvhhMMV45DLaWzw@mail.gmail.com>
- In-Reply-To
- <20130218091203.GB17003@sigill.intra.peff.net>
On Mon, Feb 18, 2013 at 1:12 AM, Jeff King <peff@peff.net> wrote:
Show 22 quoted lines
> On Sun, Feb 17, 2013 at 05:41:13PM -0800, Jonathan Nieder wrote: > >> > I don't think so. Don't ERR lines appear inside their own packets? >> >> Yes, I misread get_remote_heads for some reason. Thanks for checking. > > Thanks for bringing it up. I had not even thought about ERR at all. So > it was luck rather than skill that I was right. :) > >> I'm not sure whether servers are expected to send a flush after an >> ERR packet. The only codepath I know of in git itself that sends >> such packets is git-daemon, which does not flush after the error (but >> is not used in the stateless-rpc case). http-backend uses HTTP error >> codes for its errors. > > I just checked, and GitHub also does not send flush packets after ERR. > Which makes sense; ERR is supposed to end the conversation. I can change > GitHub, of course, but who knows what other implementations exist (e.g., > I do not know off-hand whether gitolite has custom ERR responses). So it > seems pretty clear that just checking for a flush packet is not the > right thing, and we need to actually parse the packet contents (at least > to some degree).
JGit (and by extension Gerrit Code Review, android.googlesource.com) sends ERR with no flush-pkt. I would like to sort of keep the protocol this way, given how many servers in the wild are running Gerrit and currently use ERR with no flush-pkt. IMHO its a little late to be closing that door and stuffing a flush-pkt after the ERR that ends the conversation.