Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Sat, 7 Jul 2007, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > The last time I looked at merge-recursive's D/F check, I found that it 
> > was not quite doing things right.  I may be able to dig up what I 
> > posted to the list...
> 
> It was from around April 7th-10th this year.

Unfortunately, this is way over my time budget.  As well as over my 
intelligence budget, since I did not even succeed in understanding the 
code in threeway_merge _at all_.

Besides, IMHO there is a deeper issue. Since merge-recursive started out 
as a Python script, and grew there until it was usable, and grew the 
rename detection therein, too, until it was finally converted to C, it 
accumulated a lot of features that would have been nice to have 
independently.

Almost the same goes for unpack-trees, which (its name to the contrary) 
does quite a few things to merge entries, too.  And it tries to detect d/f 
conflicts, too.

So there we are, with two really big and unwieldy chunks of code, each 
deserving an own GSoC project to clean them up.  Or maybe not even a GSoC 
project, but a longer project.

What I would _like_ to see is something as clean as merge-tree.  Which is 
clearly separated (code and file wise, too) into these stages:

- reading the trees

- determining renames

- determining true d/f conflicts

- threeway merge

- writing the tree object

- writing the work tree

- recursive

Ideally, merge-recursive would really have been as simple as

	case "$1" in
	--index_only)
		index_only=$1
		shift
	esac
	a="$1"
	b="$2"
	set $(git merge-base --all $a $b)
	temp=$1
	shift
	while case $# in 0) break;; esac
	do
		temp=$(git merge-recursive --index-only $temp $1)
		shift
	done
	git merge-non-recursive $index_only $temp -- "$a" "$b"

because _read-tree -m_ should have learnt about renames, _not_ 
merge-recursive.

As it is, both unpack_trees() and merge-recursive have a certain degree of 
not-quite duplicated yet wants-to-do-largely-the-same functionality.  
Which of course leads to much finger pointing: "it's unpack_trees() fault. 
no. it's merge-recursive's fault. no, vice versa."

Maybe the proper way out is really to start from merge-tree.c and do 
something which is easy to understand, and concise, and thus has a much 
lesser chance of being buggy.

Ciao,
Dscho