Re: What's cooking in git.git (Mar 2017, #02; Fri, 3)

[Junio C Hamano <gitster@pobox.com>]

Lars Schneider <larsxschneider@gmail.com> writes:

>> On 04 Mar 2017, at 00:26, Junio C Hamano <gitster@pobox.com> wrote:
>> 
>> 
>> * ls/filter-process-delayed (2017-01-08) 1 commit
>> . convert: add "status=delayed" to filter process protocol
>> 
>> Ejected, as does not build when merged to 'pu'.
>
> I send v2 [1] where I tried to address the points in your feedback [2].

Ah, I took a look at it back then and then forgot about it.  I'll
try to see if I can replace the stale one I have and merge it to
'pu'.

> v2 not the final roll. My goal for v2 is to get the interface
> to convert.h right.

You sound like you are trying to make the interface to the callers
finalized before doing further work, but after re-reading the
exchange between you and Peff in that thread [*1*], I am not sure
that is feasible to begin with.  

For example, your async_convert_to_working_tree() returns Success or
Delayed [*2*] and the callers of write_entry() cannot tell which the
paths on the filesystem needs a call to checkout_delayed_entries()
to finish them before they can safely tell the outside world that
these paths are safe to use.  

It seems to me that any caller that calls checkout_entry() needs to
essentially do:

	- compute which entries in the index need to be checked out
          to the filesystem;

	- for each of them:
		- call checkout_entry()

	- call checkout_delayed_entries(), because among the
          checkout_entry() calls we did in the above loop, some of
          them may have "delayed", but we do not know which one(s).

Output from "git grep -e checkout_delayed_entries -e checkout_entry"
seems to tell me that at least builtin/apply.c and
builtin/checkout-index.c forget the last step.

I'd understand the design better if the delayed-item list were a
part of the "struct checkout" state structure, and write_entry(),
when delaying the write, returned enough information (not just "this
has been delayed") that can be used to later instruct the
long-running filter process things like "you gave me this 'delayed'
token earlier; I want the result for it now!", "are you finished
processing my earlier request, to which you gave me this 'delayed'
token?", etc.  One of these instructions could be "here is the
path. write the result out for the earlier request of mine you gave
me this 'delayed' token for.  I do not need the result in-core.  And
take as much time as you need--I do not mind blocking here at this
point."  In a future, a new instruction may be added to ask "please
give me the result in-core, as if you returned the result to my
earlier original async_convert_to_working_tree() call without
delaying the request."

Within such a framework, your checkout_delayed_entries() would be a
special case for finalizing a "struct checkout" that has been in
use.  By enforcing that any "struct checkout" begins its life by a
"state = CHECKOUT_INIT" initialization and finishes its life by a
"finish_checkout(&state)" call, we will reduce risks to forget
making necessary call to checkout_delayed_entries(), I would think.


[References and Footnotes]

*1* http://public-inbox.org/git/20170226184816.30010-1-larsxschneider@gmail.com/

*2* By the way, the code in write_entry() should have a final else
clause that diagnoses an error return from
async_convert_to_working_tree() and act on it---an unexpected return
will fall through to the code that opens output fd and