merge-recursive: Fix D/F conflicts

The D/F conflicts that can be automatically resolved (file or directory
unmodified on one side of history), have the nice property that
process_entry() can correctly handle all subpaths of the D/F conflict.  In
the case of D->F conversions, it will correctly delete all non-conflicting
files below the relevant directory and the directory itself (note that both
untracked and conflicting files below the directory will prevent its
removal).  So if we handle D/F conflicts after all other conflicts, they
become fairly simple to handle -- we just need to check for whether or not
a path (file/directory) is in the way of creating the new content.  We do
this by having process_entry() defer handling such entries to a subsequent
process_df_entry() step.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/merge-recursive.c b/merge-recursive.c
index 917397ca7a0c3a702681e1c3e93eb385bb6f28fb..c8d5362191151bf6b2fb418277b761b5cc6dbeee 100644 (file)
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1072,6 +1072,7 @@ static int process_entry(struct merge_options *o,
        unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);
        unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);
 
+       entry->processed = 1;
        if (o_sha && (!a_sha || !b_sha)) {
                /* Case A: Deleted in one */
                if ((!a_sha && !b_sha) ||
@@ -1104,33 +1105,28 @@ static int process_entry(struct merge_options *o,
        } else if ((!o_sha && a_sha && !b_sha) ||
                   (!o_sha && !a_sha && b_sha)) {
                /* Case B: Added in one. */
-               const char *add_branch;
-               const char *other_branch;
                unsigned mode;
                const unsigned char *sha;
-               const char *conf;
 
                if (a_sha) {
-                       add_branch = o->branch1;
-                       other_branch = o->branch2;
                        mode = a_mode;
                        sha = a_sha;
-                       conf = "file/directory";
                } else {
-                       add_branch = o->branch2;
-                       other_branch = o->branch1;
                        mode = b_mode;
                        sha = b_sha;
-                       conf = "directory/file";
                }
                if (string_list_has_string(&o->current_directory_set, path)) {
-                       const char *new_path = unique_path(o, path, add_branch);
-                       clean_merge = 0;
-                       output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
-                              "Adding %s as %s",
-                              conf, path, other_branch, path, new_path);
-                       remove_file(o, 0, path, 0);
-                       update_file(o, 0, sha, mode, new_path);
+                       /* Handle D->F conflicts after all subfiles */
+                       entry->processed = 0;
+                       /* But get any file out of the way now, so conflicted
+                        * entries below the directory of the same name can
+                        * be put in the working directory.
+                        */
+                       if (a_sha)
+                               output(o, 2, "Removing %s", path);
+                       /* do not touch working file if it did not exist */
+                       remove_file(o, 0, path, !a_sha);
+                       return 1; /* Assume clean till processed */
                } else {
                        output(o, 2, "Adding %s", path);
                        update_file(o, 1, sha, mode, path);
@@ -1178,6 +1174,64 @@ static int process_entry(struct merge_options *o,
        return clean_merge;
 }
 
+/*
+ * Per entry merge function for D/F conflicts, to be called only after
+ * all files below dir have been processed.  We do this because in the
+ * cases we can cleanly resolve D/F conflicts, process_entry() can clean
+ * out all the files below the directory for us.
+ */
+static int process_df_entry(struct merge_options *o,
+                        const char *path, struct stage_data *entry)
+{
+       int clean_merge = 1;
+       unsigned o_mode = entry->stages[1].mode;
+       unsigned a_mode = entry->stages[2].mode;
+       unsigned b_mode = entry->stages[3].mode;
+       unsigned char *o_sha = stage_sha(entry->stages[1].sha, o_mode);
+       unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);
+       unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);
+       const char *add_branch;
+       const char *other_branch;
+       unsigned mode;
+       const unsigned char *sha;
+       const char *conf;
+       struct stat st;
+
+       /* We currently only handle D->F cases */
+       assert((!o_sha && a_sha && !b_sha) ||
+              (!o_sha && !a_sha && b_sha));
+
+       entry->processed = 1;
+
+       if (a_sha) {
+               add_branch = o->branch1;
+               other_branch = o->branch2;
+               mode = a_mode;
+               sha = a_sha;
+               conf = "file/directory";
+       } else {
+               add_branch = o->branch2;
+               other_branch = o->branch1;
+               mode = b_mode;
+               sha = b_sha;
+               conf = "directory/file";
+       }
+       if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) {
+               const char *new_path = unique_path(o, path, add_branch);
+               clean_merge = 0;
+               output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
+                      "Adding %s as %s",
+                      conf, path, other_branch, path, new_path);
+               remove_file(o, 0, path, 0);
+               update_file(o, 0, sha, mode, new_path);
+       } else {
+               output(o, 2, "Adding %s", path);
+               update_file(o, 1, sha, mode, path);
+       }
+
+       return clean_merge;
+}
+
 struct unpack_trees_error_msgs get_porcelain_error_msgs(void)
 {
        struct unpack_trees_error_msgs msgs = {
@@ -1249,6 +1303,13 @@ int merge_trees(struct merge_options *o,
                                && !process_entry(o, path, e))
                                clean = 0;
                }
+               for (i = 0; i < entries->nr; i++) {
+                       const char *path = entries->items[i].string;
+                       struct stage_data *e = entries->items[i].util;
+                       if (!e->processed
+                               && !process_df_entry(o, path, e))
+                               clean = 0;
+               }
 
                string_list_clear(re_merge, 0);
                string_list_clear(re_head, 0);

diff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh
index 761ad9d1548ab0bd7538e228cc9a30db5f7924ac..f6972bed06d088e8346e79658abe3f66d048e590 100755 (executable)
--- a/t/t6035-merge-dir-to-symlink.sh
+++ b/t/t6035-merge-dir-to-symlink.sh
@@ -56,7 +56,7 @@ test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (resolv
        test -f a/b-2/c/d
 '
 
-test_expect_failure 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recursive)' '
+test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recursive)' '
        git reset --hard &&
        git checkout baseline^0 &&
        git merge -s recursive master &&
@@ -130,7 +130,7 @@ test_expect_success 'merge should not have D/F conflicts (resolve)' '
        test -f a/b/c/d
 '
 
-test_expect_failure 'merge should not have D/F conflicts (recursive)' '
+test_expect_success 'merge should not have D/F conflicts (recursive)' '
        git reset --hard &&
        git checkout baseline^0 &&
        git merge -s recursive test2 &&
@@ -138,7 +138,7 @@ test_expect_failure 'merge should not have D/F conflicts (recursive)' '
        test -f a/b/c/d
 '
 
-test_expect_failure 'merge should not have F/D conflicts (recursive)' '
+test_expect_success 'merge should not have F/D conflicts (recursive)' '
        git reset --hard &&
        git checkout -b foo test2 &&
        git merge -s recursive baseline^0 &&