Re: [PATCH] merge-tree: fix "same file added in subdir"
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 27, 2013, 22:42 UTC
- Message-ID
- <7vk3osjwgv.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <0a6a0c978569906b8c8d9209a85338554e503236.1364419952.git.john@keeping.me.uk>
John Keeping <john@keeping.me.uk> writes:
Show 9 quoted lines
> When the same file is added with identical content at the top level, > git-merge-tree prints "added in both" with the details. But if the file > is added in an existing subdirectory, threeway_callback() bails out early > because the two trees have been modified identically. > > In order to detect this, we need to fall through and recurse into the > subtree in this case. > > Signed-off-by: John Keeping <john@keeping.me.uk>
The rationale the above description gives is internally consistent, but it is rather sad to see this optimization go. The primary motivation behind this program, which does not use the usual unpack-trees machinery, is to allow us to cull the identical result at a shallow level of the traversal when the both sides changed (not added) a file deep in a subdirectory hierarchy.
The patch makes me wonder if we should go the other way around, resolving the "both added identically" case at the top cleanly without complaint.
Show 54 quoted lines
> builtin/merge-tree.c | 9 +++++++--
> t/t4300-merge-tree.sh | 17 +++++++++++++++++
> 2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
> index e0d0b7d..ca97fbd 100644
> --- a/builtin/merge-tree.c
> +++ b/builtin/merge-tree.c
> @@ -298,12 +298,17 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s
> {
> /* Same in both? */
> if (same_entry(entry+1, entry+2)) {
> - if (entry[0].sha1) {
> + if (entry[0].sha1 && !S_ISDIR(entry[0].mode)) {
> /* Modified identically */
> resolve(info, NULL, entry+1);
> return mask;
> }
> - /* "Both added the same" is left unresolved */
> + /*
> + * "Both added the same" is left unresolved. We also leave
> + * "Both directories modified identically" unresolved in
> + * order to catch changes where the same file (with the same
> + * content) has been added to both directories.
> + */
> }
>
> if (same_entry(entry+0, entry+1)) {
> diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
> index d0b2a45..be0737e 100755
> --- a/t/t4300-merge-tree.sh
> +++ b/t/t4300-merge-tree.sh
> @@ -298,4 +298,21 @@ test_expect_success 'turn tree to file' '
> test_cmp expect actual
> '
>
> +test_expect_success 'add identical files to subdir' '
> + cat >expected <<\EXPECTED &&
> +added in both
> + our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/ONE
> + their 100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/ONE
> +EXPECTED
> +
> + git reset --hard initial &&
> + mkdir sub &&
> + test_commit "sub-initial" "sub/initial" "initial" &&
> + test_commit "sub-add-a-b-same-A" "sub/ONE" "AAA" &&
> + git reset --hard sub-initial &&
> + test_commit "sub-add-a-b-same-B" "sub/ONE" "AAA" &&
> + git merge-tree sub-initial sub-add-a-b-same-A sub-add-a-b-same-B >actual &&
> + test_cmp expected actual
> +'
> +
> test_done