Re: [RFH] CE_REMOVE conversion

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Feb 21, 2008, 16:05 UTC
Message-ID: <alpine.LFD.1.00.0802210746410.7833@woody.linux-foundation.org>
In-Reply-To: <7v7igywvnj.fsf@gitster.siamese.dyndns.org>

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