From: Jeff King <peff@peff.net>
Date: Sat, 06 Aug 2016 12:14:21 GMT
Subject: Re: [PATCH v4 11/12] convert: add filter.<driver>.process option
Message-ID: <20160806121421.bs7n4lhed7phdshb@sigill.intra.peff.net>
In-Reply-To: <87D4BF17-67BB-4AFA-9B27-40DBB44C0456@gmail.com>

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.

-Peff