> On 06 Mar 2017, at 22:08, Junio C Hamano <gits...@pobox.com> wrote: > > Lars Schneider <larsxschnei...@gmail.com> writes: > >>> On 04 Mar 2017, at 00:26, Junio C Hamano <gits...@pobox.com> wrote: >>> >>> >>> * ls/filter-process-delayed (2017-01-08) 1 commit >>> . convert: add "status=delayed" to filter process protocol
Sorry for the delayed response. I am still pursuing this topic but unfortunately I wasn't able to spend time on it recently. > 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). Agreed. Would it be OK to store the "delayed" bit in the cache entry itself? The extended ce_flags are stored on disk which is not necessary I think. Would a new member in the cache_entry struct be an acceptable option? > 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. For all these "single" checkout entry places the delayed checkout makes no sense I think. Would you prefer something likes this: checkout_entry(); finish_checkout(); or this: checkout_entry(..., DISABLE_DELAYED_CHECKOUT); in all these places? > I'd understand the design better if the delayed-item list were a > part of the "struct checkout" state structure, and write_entry(), This works [1], however I had to remove "const" from "const struct checkout *state" in a few places. OK? > 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. Agreed. How would you want to enforce "finish_checkout(&state)", though? By convention or by different means? > *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 It is ok for async_convert_to_working_tree() to fail if the filter is not required. That's how it is currently implemented upstream. I will post a new round to the mailing list soon. Here is a preview if you want to look at the const changes etc: https://github.com/larsxschneider/git/commit/a0132e564faaf5bca76af0ccc4ec6fe900be739a?w=1 Thanks, Lars
