Re: [PATCH v2 0/5] Git filter protocol

On 29 Jul 2016, at 17:57, Jeff King [off-list ref] wrote:

On Fri, Jul 29, 2016 at 10:14:17AM +0200, Lars Schneider wrote:

quoted

My current implementation supports only two cases. Either the filter
knows the size and sends it back. Or the filter doesn't know the size
and Git reads until the flush packet (your "unknown" case). "Approx" is 
probably hard to do and fail shouldn't be part of the size, no?

Ah, OK, I missed that you could handle both cases. I think that is a
reasonable approach. It means the filter has to bother with pkt-lines,
but beyond that, it can choose the simple or streaming approach as
appropriate.

Right.

quoted

That being said a "fail" response is a very good idea! This allows
the filter to communicate to git that a non required filter process
failed. I will add that to the protocol. Thanks :) 

Maybe just send "ok <size>", "ok -1" (for streaming), or "fail <reason>"
followed by the content? That is similar to other Git protocols, though
I am not sure they are good models for sanity or extensibility. :)

I don't know if you would want to leave room for other "headers" in the
response, but you could also do something more HTTP-like, with a status
code, and arbitrary headers. And presumably git would just ignore
headers it doesn't know about. I think that's what Jakub's example was
leaning towards. I'm just not sure what other headers are really useful,
but it does leave room for extensibility.

Well, "ok <size>" wouldn't make much sense as we already transmitted
the size upfront I think. Right now I have implemented the following options:

"success\n" --> everything was alright
"reject\n" --> the filter rejected the operation but this is no error 
               if "filter.<driver>.required = false"
<anything else> --> failure that stops/restarts the filter process

I don't think sending any failure reason makes sense because if a failure
happens then we are likely in a bad state already (that's why I restart the
filter process. I think the filter can report trouble on its own via stdout,
no? I think this is what Git-LFS already does.

I am working on the docs right now and afterwards I will send a v3 :-)

- Lars