Re: [PATCH 1/3] Fix sparse checkout not removing files from index

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 30, 2010, 19:50 UTC
Message-ID: <20100730195022.GB2448@burratino>
In-Reply-To: <AANLkTi=3pX=k=Pf5SnWt8s=hbvwAXcZdmcqP_+kgCyE5@mail.gmail.com>

Nguyen Thai Ngoc Duy wrote:

> For unpack_trees() only, yes.

Ah, that is what I was missing.

So would the following be correct?

	Subject: unpack-trees: let read-tree -u remove index entries outside sparse area

	To avoid touching the worktree outside a sparse checkout,
	when the update flag is enabled unpack_trees() clears the
	CE_UPDATE and CE_REMOVE flags on entries that do not match the
	sparse pattern before actually committing any updates to the
	index file or worktree.

	The effect on the index was unintentional; sparse checkout was
	never meant to prevent index updates outside the area checked
	out.  And the result is very confusing: for example, after a
	failed merge, currently "git reset --hard" does not reset the
	state completely but an additional "git reset --mixed" will.

	So stop clearing the CE_REMOVE flag.  Instead, maintain a
	CE_WT_REMOVE flag to separately track whether a particular
	file removal should apply to the worktree in addition to the
	index or not.

	The CE_WT_REMOVE flag is used already to mark files that
	should be removed because of a narrowing checkout area.  That
	usage will still apply; do not clear the CE_WT_REMOVE flag
	in that case (detectable because the CE_REMOVE flag is not
	set).

	This bug masked some other bugs illustrated by the test
	suite, which will be addressed by later patches.

	Reported-by: Frédéric Brière <fbriere@fbriere.net>
	Fixes: http://bugs.debian.org/583699

> It'd better be done in one place.

I think I understand better now, and now I agree. (This patch is not a change in the semantics of CE_REMOVE, but just some new unpack-trees bookkeeping.)

> 2010/7/30 Jonathan Nieder <jrnieder@gmail.com>:

Show 7 quoted lines

>> This is a little tricky: the CE_WT_REMOVE case (without CE_REMOVE)
>> represents a narrowing of the checkout and should be preserved,
>> while CE_WT_REMOVE|CE_REMOVE represents a removed index entry and
>> should be changed to just CE_REMOVE.
>
> Yeah. I did wonder if it's worth a comment to explain. I forget why I
> did not add that comment now.

What happens if an index entry is removed at the same time as the checkout is narrowed?