John Keeping <john@xxxxxxxxxxxxx> writes:
> 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@xxxxxxxxxxxxx>
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.
> 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
--
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