Re: [PATCH v4 11/12] convert: add filter.<driver>.process option
- From
Lars Schneider <larsxschneider@gmail.com>
- Date
- Aug 6, 2016, 18:19 UTC
- Message-ID
- <A07BE78B-5A5D-41F1-A51B-5C71F3E86CCF@gmail.com>
- In-Reply-To
- <20160806121421.bs7n4lhed7phdshb@sigill.intra.peff.net>
Show 53 quoted lines
> On 06 Aug 2016, at 14:14, Jeff King <peff@peff.net> wrote:
>
> On Sat, Aug 06, 2016 at 01:55:23PM +0200, Lars Schneider wrote:
>
>>> And I expect it makes the lives of the client
>>> easier to get a code up front, before it starts taking steps to handle
>>> what it _thinks_ is probably a valid response.
>>
>> I am not sure I can follow you here. Which actor are you referring to when
>> you write "client" -- Git, right? If the response is rejected right away
>> then Git just needs to read a single flush. If the response experiences
>> an error only later, then the filter wouldn't know about the error when
>> it starts sending. Therefore I don't see how an error code up front could
>> make it easier for Git.
>
> Yes, I mean git (I see it as the "client" side of the connection in that
> it is making requests of the filter, which will then provide responses).
>
> What I mean is that the git code could look something like:
>
> status == send_filter_request();
> if (status == OK) {
> prepare_storage();
> read_response_into_storage();
> } else {
> complain();
> }
>
> But if there's no status up front, then you probably have:
>
> send_filter_request();
> prepare_storage();
> status = read_response_into_storage();
> if (status != OK) {
> rollback_storage();
> complain();
> }
>
> In the first case, we could easily avoid preparing the storage if our
> request wasn't going to be filled, whereas in the second we have to do
> it unconditionally. That's not a big deal if preparing the storage is
> initializing a strbuf. It's more so if you're opening a temporary object
> file to stream into.
>
> You _do_ still have to deal with rollback in the first one (for the case
> that the stream ends prematurely for whatever reason). So it's really a
> question of where and how often we expect the failures to come, and
> whether it is worth git knowing up front that the request is not going
> to be fulfilled.
>
> 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 ------------------------
Happy answer with no content: ------------------------ packet: git< status=success\n ------------------------
Rejected content: ------------------------ packet: git< status=reject\n ------------------------
Error during content response: ------------------------ packet: git< status=accept\n packet: git< HALF_WRITTEN_ERRONEOUS_CONTENT packet: git< 0000 packet: git< status=error\n ------------------------
Cheers, Lars