Re: [PATCH 5/5] t3030: merge-recursive backend test.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <junkio@xxxxxxx> writes:

> +test_expect_success 'merge-recursive d/f conflict result' '
> +
> +	git ls-files -s >actual &&
> +	(
> +		echo "100644 $o0 0	A"
> +		echo "100644 $o0 1	a"
> +		echo "100644 $o1 2	a"
> +		echo "100644 $o4 0	a/c"
> +		echo "100644 $o0 0	b"
> +		echo "100644 $o1 0	d/e"
> +	) >expected &&
> +	git diff -u expected actual
> +
> +'

The expected result of this test is what I find very iffy.  The
input trees are:

 * Ancestor has a file "a"
 * Our tree has a file "a", which was modified from the
   ancestor's
 * The other tree has a file "a/c"

merge-recursive.c::git_merge_trees() calls unpack_trees() using
the threeway_merge function and what it gets back are stage 1
and 2 entries for "a", and stage 3 entry for "a/c" (you can see
it by running "git-read-tree -m $c0 $c1 $c4" instead).  But the
output from merge-recursive has "a/c" resolved down to stage 0.

The problem is that process_entry() finds that there is only
stage 3 entry for that path and decides to resolve it.  I think
it should do the D/F check and leave "a/c" in stage 3.

Looking at the D/F conflict detection code in merge-recursive.c
makes my head hurt, and I do not quite follow the logic there,
but it seems to be doing the following to me.

 * current_directory_set is initialized to be union of all
   directories that appear in our tree and the other tree (I am
   not sure if that set is updated and if so how);

 * when a path is about to be resolved to a blob, this set is
   consulted, and if there is a directory there, declare D/F
   conflict and blob is added under a name that does not
   conflict with the directory;

 * otherwise the path is resolved down to stage 0.

However, in this case, there is no such directory "a/c" but the
code forgets that adding a blob at "a/c" requires that there is
no file "a" at all.  I am not sure if this is deliberate or just
a simple oversight.

Also I suspect this can be done more easily with just checking
the index.  We know blob "a" is in the index, unresolved, when
we try to resolve path "a/c".
 





-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]