Re: [PATCH v4 11/12] convert: add filter.<driver>.process option

On 08 Aug 2016, at 17:02, Jeff King [off-list ref] wrote:

On Sat, Aug 06, 2016 at 08:19:28PM +0200, Lars Schneider wrote:

quoted

quoted

I dunno. It's not _that_ big a deal to code around. I was just surprised
not to see an up-front status when responding to a request. It seems
like the normal thing in just about every protocol I've ever used.

Alright. The fact that it "surprised" you is a bad sign. 
How about this:

Happy answer:
------------------------
packet:          git< status=accept\n
packet:          git< SMUDGED_CONTENT
packet:          git< 0000
packet:          git< status=success\n
------------------------

I notice that the status pkt-lines are by themselves. I had assumed we'd
be sending other data, too (presumably before, but I guess possibly
after, too). Something like:

 git< status=accept
 git< 0000
 git< SMUDGED_CONTENT
 git< 0000
 git< status=success
 git< 0000

I don't have any particular meta-information in mind, but I thought
stuff like the tentative "size" field would be here.

I had imagined it at the front, but I guess it could go in either place.
I wonder if keys at the end could simply replace ones from the beginning
(so if you say "foo=bar" at the front, that is tentative, but if you
then say "foo=revised" at the end, that takes precedence).

And so the happy answer is really:

 git< status=success
 git< 0000
 git< SMUDGED_CONTENT
 git< 0000
 git< 0000  # empty list!

i.e., no second status. The original "success" still holds.

OK, that sounds sensible to me.

And then:

quoted

Happy answer with no content:
------------------------
packet:          git< status=success\n
------------------------

This can just be spelled:

 git< status=success
 git< 0000
 git< 0000   # empty content!
 git< 0000   # empty list!

Is the first flush packet one too many?
If there is nothing then I think we shouldn't
send any packets?!

I agree with the remaining two flush packets.

quoted

Rejected content:
------------------------
packet:          git< status=reject\n
------------------------

I'd assume that an error status would end the output for that file
immediately, no empty lists necessary (so what you have here). I'd
probably just call this "error" (see below).

OK!

quoted

Error during content response:
------------------------
packet:          git< status=accept\n
packet:          git< HALF_WRITTEN_ERRONEOUS_CONTENT
packet:          git< 0000
packet:          git< status=error\n
------------------------

And then this would be:

 git< status=success
 git< 0000
 git< HALF_OF_CONTENT
 git< 0000
 git< status=error
 git< 0000

And then you have only two status codes: success and error. Which keeps
things simple.

There's one other case, which is when the filter dies halfway through
the conversation, like:

 git< status=success
 git< 0000
 git< CONTENT
 git< 0000
 ... EOF on pipe ...

Any time git does not get the conversation all the way to the final
flush after the trailers, it should be considered an error (because we
can never know if the filter was about to say "whoops, status=error").

Right. I agree with the protocol above and I will implement it
that way.

There is one more thing: I introduced a return value "status=error-all".
Using this the filter can signal Git that it does not want to process
any other file using the particular command.

Jakub came up with this idea here:

"Another response, which I think should be standarized, or at
least described in the documentation, is filter driver refusing
to filter further (e.g. git-LFS and network is down), to be not
restarted by Git."

http://public-inbox.org/git/607c07fe-5b6f-fd67-13e1-705020c267ee%40gmail.com/

I think it is a good idea. Do you see arguments against it?

Thanks,
Lars