Subject: Re: [PATCH v1] convert: add "status=delayed" to filter process protocol

From: Jakub Narębski <[email-masked]>
Date: Tue, 10 Jan 2017 23:11:01 +0100

W dniu 09.01.2017 o 00:42, Junio C Hamano pisze:
> [email-masked] writes:
>> From: Lars Schneider <[email-masked]>
>>
>> Some `clean` / `smudge` filters might require a significant amount of
>> time to process a single blob. During this process the Git checkout
>> operation is blocked and Git needs to wait until the filter is done to
>> continue with the checkout.

Lars, what is expected use case for this feature; that is when do you
think this problem may happen?  Is it something that happened IRL?

>>
>> Teach the filter process protocol (introduced in edcc858) to accept the
>> status "delayed" as response to a filter request. Upon this response Git
>> continues with the checkout operation and asks the filter to process the
>> blob again after all other blobs have been processed.
> 
> Hmm, I would have expected that the basic flow would become
> 
> 	for each paths to be processed:
> 		convert-to-worktree to buf
> 		if not delayed:
> 			do the caller's thing to use buf
> 		else:
> 			remember path
> 
> 	for each delayed paths:
> 		ensure filter process finished processing for path
> 		fetch the thing to buf from the process
> 		do the caller's thing to use buf

I would expect here to have a kind of event loop, namely

        while there are delayed paths:
                get path that is ready from filter
                fetch the thing to buf (supporting "delayed")
                if path done
                        do the caller's thing to use buf 
                        (e.g. finish checkout path, eof convert, etc.)

We can either trust filter process to tell us when it finished sending
delayed paths, or keep list of paths that are being delayed in Git.

> 
> and that would make quite a lot of sense.  However, what is actually
> implemented is a bit disappointing from that point of view.  While
> its first part is the same as above, the latter part instead does:
> 
> 	for each delayed paths:
> 		checkout the path
> 
> Presumably, checkout_entry() does the "ensure that the process is
> done converting" (otherwise the result is simply buggy), but what
> disappoints me is that this does not allow callers that call
> "convert-to-working-tree", whose interface is obtain the bytestream 
> in-core in the working tree representation, given an object in the
> object-db representation in an in-core buffer, to _use_ the result
> of the conversion.  The caller does not have a chance to even see
> the result as it is written straight to the filesystem, once it
> calls checkout_delayed_entries().
>