Re: [PATCH] modify/delete conflict resolution overwrites untracked file

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 15, 2008, 22:13 UTC
Message-ID: <7vej09w0hy.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <7vskopwxej.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 21 quoted lines

> Clemens Buchacher <drizzd@aon.at> writes:
>
>> On Sun, Dec 14, 2008 at 07:34:46PM -0800, Junio C Hamano wrote:
>>> merge-recursive: do not clobber untracked working tree garbage
>>> 
>>> When merge-recursive wanted to create a new file in the work tree (either
>>> as the final result, or a hint for reference purposes while delete/modify
>>> conflicts), it unconditionally overwrote an untracked file in the working
>>> tree.  Be careful not to lose whatever the user has that is not tracked.
>>
>> This leaves the index in an unmerged state, however, so that a subsequent
>> git reset --hard still kills the file. And I just realized that the same
>> goes for merge-resolve. So I'd prefer to abort the merge, leave everything
>> unchanged and tell the user to clean up first.
>
> That is unfortunately asking for a moon, I am afraid.
>
> It needs a major restructuring of the code so that the recursive works
> more like the way resolve works, namely, changing the final "writeout"
> into two phase thing (the first phase making sure nothing is clobbered in
> the work tree, and then the second phase actually touching the work tree).

Actually, the more I think about it, I do not think this is not something we would even want to do.

By this, I do not mean the restructuring to bring some sanity to merge-recursive. That is necessary. What I do not think is unnecessary is the issue you raise about "git reset --hard".

You can do a merge inside a dirty work tree, and the merge will fail without clobbering your work tree files that are dirty when it needs to be able to overwrite to do its job. The set of "dirty files" in this sentence of course includes paths that are modified since HEAD, but it also includes also paths that do not exist in HEAD (i.e. "new files").

But we already caution users that you need to know what you are doing when working in such a dirty work tree. Namely, after a failed merge, your next "git reset --hard" will blow away your local modifications. And local modifications in this context includes the files you could have added to the index but you haven't.

By the way, I think the patch I sent earlier is too complex and suboptimal for an entirely different reason.

The only reason the codepath for delete/modify in process_entry() wants to leave the modified side in the result is because the internal merge done when the algorithm is coming up with a merged merge bases _must_ be fully resolved. There is no such requirement for the final round of the merge whose result is written out to the work tree. Whether the path that was involved in delete/modify conflict was originally in the index or not, we should just leave it alone in the work tree. The logic I implemented as the would_lose_untracked() function is just overkill.