On Tue Aug 4, 2020 at 3:30 PM EDT, Junio C Hamano wrote:
> How often do we pass ok_if_exists, I have to wonder. If it is often
> enough, then we can check that first way before we even check to see
> if a cache entry for the path even exists or what its i-t-a flag
> says. Something along the lines of this untested code:
>
> if (state->check_index && !ok_if_exists) {
> int pos = index_name_pos(state->repo->index, new_name,
> strlen(new_name));
> if (pos >= 0 &&
> !(state->repo->index->cache[pos]->ce_flags & CE_INTENT_TO_ADD))
> return EXISTS_IN_INDEX;
> }
>
> That is, only if we are told to make sure the path does not already
> exist,
> we see if the path is in the index, and if the cache entry for the
> path in the index is a real entry (as opposed to i-t-a aka "not
> added yet"), we complain. Otherwise we'd happily take the patch.
>
> Whether ok_if_exists is frequently used or not, the resulting code
> may be easier to understand, but I am of course biased, as I just
> wrote it ;-)
ok_if_exists gets passed in cases where a real entry does exist but
we're okay with a new file diff anyway due to other patches in the set
being applied making it valid (type-change diffs and rename diffs) - for
this reason, I didn't pass ok_if_exists, but instead checked here. I
think we're in agreement on this and your logic makes sense in that
light. I'll send an updated patch.
