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

From

Junio C Hamano <gitster@pobox.com>

Date

Dec 15, 2008, 03:34 UTC

Message-ID

<7vmyeyyuuh.fsf@gitster.siamese.dyndns.org>

In-Reply-To

<7v63lm1c76.fsf@gitster.siamese.dyndns.org>

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

> Clemens Buchacher <drizzd@aon.at> writes:
>
>> On Wed, Dec 10, 2008 at 09:12:59PM +0100, Clemens Buchacher wrote:
>>> If a file was removed in HEAD, but modified in MERGE_HEAD, recursive merge
>>> will result in a "CONFLICT (delete/modify)". If the (now untracked) file
>>> already exists and was not added to the index, it is overwritten with the
>>> conflict resolution contents.
>>
>> The following patch fixes the problem described above, but it also breaks
>> t6023-merge-rename-nocruft.sh, which tries to merge "A" renamed to "B" in
>> HEAD and "A" modified in MERGE_HEAD, while ignoring an untracked file "A" in
>> the working tree. If we want to be able to do this, we have to handle the
>> other case after rename detection.
>
> If the breakage is in merge-recursive but not in merge-resolve, my gut
> feeling is that we should not be touching unpack-trees at all.  With luck
> I may be able to find some time to take a look at this myself but right
> now we are entertaining a guest, so....

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 merge-recursive.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git c/merge-recursive.c w/merge-recursive.c
index a0c804c..2da4333 100644
--- c/merge-recursive.c
+++ w/merge-recursive.c
@@ -447,6 +447,30 @@ static void flush_buffer(int fd, const char *buf, unsigned long size)
 	}
 }
 
+static int would_lose_untracked(const char *path)
+{
+	int pos = cache_name_pos(path, strlen(path));
+
+	if (pos < 0)
+		pos = -1 - pos;
+	while (pos < active_nr &&
+	       !strcmp(path, active_cache[pos]->name)) {
+		/*
+		 * If stage #0, it is definitely tracked.
+		 * If it has stage #2 then it was tracked
+		 * before this merge started.  All other
+		 * cases the path was not tracked.
+		 */
+		switch (ce_stage(active_cache[pos])) {
+		case 0:
+		case 2:
+			return 0;
+		}
+		pos++;
+	}
+	return file_exists(path);
+}
+
 static int make_room_for_path(const char *path)
 {
 	int status;
@@ -462,6 +486,14 @@ static int make_room_for_path(const char *path)
 		die(msg, path, "");
 	}
 
+	/*
+	 * Do not unlink a file in the work tree if we are not
+	 * tracking it.
+	 */
+	if (would_lose_untracked(path))
+		return error("refusing to lose untracked file at '%s'",
+			     path);
+
 	/* Successful unlink is good.. */
 	if (!unlink(path))
 		return 0;