On Thu, 21 Feb 2008, Junio C Hamano wrote: > > but I am wondering if we should instead really _remove_ entries > from the index instead, just like the attached patch. Absolutely. I think that your patch is "ObviouslyCorrect(tm)", and that code should never have used CE_REMOVE in the first place. The fact that the old code worked was perhaps intentional, but still very subtle and lucky. The whole (and only) point of CE_REMOVE was and is to do the "-u" handling when resolving a merge, and keep the thing in the index in order to later still do work with it (ie remove it) when we have verified that the index is complete. But that's not what "read_cache_unmerged()" is about: it's very explicitly documented to just ignore the unmerged entries. So your patch looks very good to me. Basically, the merge code absolutely does not want to be called with some entries already marked as CE_REMOVE (it's supposed to *add* those markers as part of resolving the merge, but it is not able to handle them in the source). So ack, ack, ack. Linus - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
