> On 27 Jun 2017, at 23:30, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Lars Schneider <larsxschneider@xxxxxxxxx> 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. I posted it here: http://public-inbox.org/git/7FF53F59-1240-4BED-999F-FE9C9AD7E6AC@xxxxxxxxx/ If you are OK with it, then I will post a v8 with all changes combined. >> 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. Yes, I think this should be equivalent. >> 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. I'll remove it. Thanks, Lars
