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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:20

Junio C Hamano [off-list ref] writes:
Johannes Schindelin [off-list ref] writes:
...
quoted
Okay, so you're saying that merge-recursive should use the aggressive 
strategy?
I do not think so.  Isn't the whole "see if there are renames" thing
depend on threeway_merge() not resolving "one side removes other
side leaves intact" case itself?  Aggressive resolves it saying
"Ok that is a remove", which risks it to miss the case in which
that the side that apparently "removed" the path in fact moved
it somewhere else.

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.

    http://thread.gmane.org/gmane.comp.version-control.git/43970/focus=44158
    http://thread.gmane.org/gmane.comp.version-control.git/43971/focus=43997

I think the case described in the latter message is almost the
opposite case of what your patch tries to deal with.

In the web interface of

    http://news.gmane.org/gmane.comp.version-control.git

the patch series that led to my complaints are at around page 76
for me.

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

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:20

Hi,

On Sat, 7 Jul 2007, Junio C Hamano wrote:
Junio C Hamano [off-list ref] writes:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help