Re: [PATCH 3/3] read-tree -m 3-way: handle more trivial merges internally

On Thu, 9 Jun 2005, Junio C Hamano wrote:

No, I think this is quite possibly wrong for several reasons.

For one, it makes the rest of the system unaware of the deleted files, so nothing ever deletes them from the working directory.

And that's not entirely trivial to fix either, since the obvious fix (which is to just do a

	if (update)
		unlink(b->name);

or something like that) is wrong. It's wrong because we must not do the update until the very end, when we've either merged all entries or we've failed on an entry that couldn't be merged (that's why I did the extra CE_UPDATE flag, instead of updating as we go along).

Now, you could fix that by creating a separate list of files to be deleted (so this is not fundamental, it's just more complicated than the trivial case), but that doesn't help, because there's _another_ reason why read-tree shouldn't handle these cases.

Namely that read-tree doesn't have a frigging clue about renames, and shouldn't have.

But a real merge program _could_ have a frigging clue, and might notice patterns like

 - file got modified in one branch, removed in the other
 - a file got added in the other branch
 - "Hey, that added file looks like the removed one!"
 - Let's merge the modifications from the first branch into the move of 
   the second branch!

See? Now, git-read-tree won't handle the first case anyway, but your change _does_ make it handle the "file got added" case, which means that now the added file is invisible the the "smart merger", and the smart merger can't really tell that it was a rename any more.

So our current stupid file-by-file "git-merge-cache" will never do this, but that's a limitation of me being less than the intellectual giant I wish I was. So I just do the stupid merges. But I _know_ they are stupid, and I would like to leave the door open for somebody else to fix up the cases I don't handle.

You're basically closing that door.

Now, you can (validly) argue that you could still just look at the original trees ("git-diff-tree -C $O $M") and grep for copies/movement and do it by hand _there_ instead of looking at the result of the read-tree, and you may well be right. So again, this is not a _fundamental_ problem, although it's a bit more fundamental than the first one.

So if you want to convince me that it's better to do the rename detection outside of the index file, go wild. Alternatively, you can argue that we can always undo this later, when once we _do_ have rename and copy detection and can try to merge things automatically (what _do_ you do if a file is copied in one branch and modified in the other? Just warn the poor user, I guess).

So I just need a little convincing that this is a good idea.

		Linus