Re: [PATCHv2 0/10] pkt-line and remote-curl cleanups server

On Mon, Feb 18, 2013 at 1:12 AM, Jeff King [off-list ref] wrote:

On Sun, Feb 17, 2013 at 05:41:13PM -0800, Jonathan Nieder wrote:

quoted

quoted

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. :)

quoted

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.