Re: [PATCH v7 6/6] convert: add "status=delayed" to filter process protocol

> On 27 Jun 2017, at 23:30, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Lars Schneider <larsxschneider@gmail.com> writes:
> 
>> I treated that as low-prio as I got the impression that you are generally OK with
>> the current implementation. I promise to send a patch with this change. 
>> Either as part of this series or as a follow up patch.
> 
> Sure. I was just being curious what happened; I agree that it is a
> good idea to treat this as a lower priority clean-up item and leave
> it to later.

>> Yes. I guess that was a premature optimization on my end.
>> This is how "write_entry()" in entry.c would change:
>> 
>> -                               dco->is_delayed = 0;
>>                                ret = async_convert_to_working_tree(
>>                                        ce->name, new, size, &buf, dco);
>> -                               if (ret && dco->is_delayed) {
>> +                               if (ret && string_list_has_string(&dco->paths, ce->name)) {
>>                                        free(new);
>>                                        goto finish;
>>                                }
>> 
>> OK?
> 
> Actually I should be asking the "OK?" question---the above was what
> I had in mind when I commented, but you are a lot more familiar with
> the way how this code is supposed to work, and I didn't know if the
> emptyness of the list can be a 100% substitute for that bit.  If you
> think they are equivalent, that is a great news.

>> At this point I want to ensure that checkout_entry() is called
>> with the CE_RETRY state. Because checkout_entry() needs to pass 
>> that flag to the convert machinery.
>> 
>> This assert was useful when checkout_entry() changed dco->state
>> between CAN_DELAY and DELAYED. In the current implementation the
>> state should not be changed anymore.
>> 
>> Would you prefer if I remove it? (with or without is both fine with me)
> 
> If that is the reason behind the assert, I am OK with either (1)
> moving it _after_ checkout_entry() returns (to ensure that the
> function no longer mucks with the state, or (2) removing it.
> 
> Leaving it at the current position does not make much sense to me.