submodules: allow empty working-tree dirs in merge/cherry-pick

When a submodule is being merged or cherry-picked into a working
tree that already contains a corresponding empty directory, do not
record a conflict.

One situation where this bug appears is:

- Commit 1 adds a submodule
- Commit 2 removes that submodule and re-adds it into a subdirectory
(sub1 to sub1/sub1).
- Commit 3 adds an unrelated file.

Now the user checks out commit 1 (first deinitializing the submodule),
and attempts to cherry-pick commit 3.  Previously, this would fail,
because the incoming submodule sub1/sub1 would falsely conflict with
the empty sub1 directory.

This patch ignores the empty sub1 directory, fixing the bug.  We only
ignore the empty directory if the object being emplaced is a
submodule, which expects an empty directory.

Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/merge-recursive.c b/merge-recursive.c
index 9041c2f149c01134ce02119354455894533e713c..e64b48b25da2a53851aa40cf149e510641c23a8d 100644 (file)
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -664,7 +664,13 @@ static char *unique_path(struct merge_options *o, const char *path, const char *
        return strbuf_detach(&newpath, NULL);
 }
 
        return strbuf_detach(&newpath, NULL);
 }
 

-static int dir_in_way(const char *path, int check_working_copy)
+/**
+ * Check whether a directory in the index is in the way of an incoming
+ * file.  Return 1 if so.  If check_working_copy is non-zero, also
+ * check the working directory.  If empty_ok is non-zero, also return
+ * 0 in the case where the working-tree dir exists but is empty.
+ */
+static int dir_in_way(const char *path, int check_working_copy, int empty_ok)

 {
        int pos;
        struct strbuf dirpath = STRBUF_INIT;
 {
        int pos;
        struct strbuf dirpath = STRBUF_INIT;


@@ -684,7 +690,8 @@ static int dir_in_way(const char *path, int check_working_copy)
        }
 
        strbuf_release(&dirpath);
        }
 
        strbuf_release(&dirpath);

-       return check_working_copy && !lstat(path, &st) && S_ISDIR(st.st_mode);
+       return check_working_copy && !lstat(path, &st) && S_ISDIR(st.st_mode) &&
+               !(empty_ok && is_empty_dir(path));

 }
 
 static int was_tracked(const char *path)
 }
 
 static int was_tracked(const char *path)


@@ -1062,7 +1069,7 @@ static int handle_change_delete(struct merge_options *o,
 {
        char *renamed = NULL;
        int ret = 0;
 {
        char *renamed = NULL;
        int ret = 0;

-       if (dir_in_way(path, !o->call_depth)) {
+       if (dir_in_way(path, !o->call_depth, 0)) {

                renamed = unique_path(o, path, a_oid ? o->branch1 : o->branch2);
        }
 
                renamed = unique_path(o, path, a_oid ? o->branch1 : o->branch2);
        }
 


@@ -1195,7 +1202,7 @@ static int handle_file(struct merge_options *o,
                remove_file(o, 0, rename->path, 0);
                dst_name = unique_path(o, rename->path, cur_branch);
        } else {
                remove_file(o, 0, rename->path, 0);
                dst_name = unique_path(o, rename->path, cur_branch);
        } else {

-               if (dir_in_way(rename->path, !o->call_depth)) {
+               if (dir_in_way(rename->path, !o->call_depth, 0)) {

                        dst_name = unique_path(o, rename->path, cur_branch);
                        output(o, 1, _("%s is a directory in %s adding as %s instead"),
                               rename->path, other_branch, dst_name);
                        dst_name = unique_path(o, rename->path, cur_branch);
                        output(o, 1, _("%s is a directory in %s adding as %s instead"),
                               rename->path, other_branch, dst_name);


@@ -1704,7 +1711,8 @@ static int merge_content(struct merge_options *o,
                         o->branch2 == rename_conflict_info->branch1) ?
                        pair1->two->path : pair1->one->path;
 
                         o->branch2 == rename_conflict_info->branch1) ?
                        pair1->two->path : pair1->one->path;
 

-               if (dir_in_way(path, !o->call_depth))
+               if (dir_in_way(path, !o->call_depth,
+                              S_ISGITLINK(pair1->two->mode)))

                        df_conflict_remains = 1;
        }
        if (merge_file_special_markers(o, &one, &a, &b,
                        df_conflict_remains = 1;
        }
        if (merge_file_special_markers(o, &one, &a, &b,


@@ -1862,7 +1870,8 @@ static int process_entry(struct merge_options *o,
                        oid = b_oid;
                        conf = _("directory/file");
                }
                        oid = b_oid;
                        conf = _("directory/file");
                }

-               if (dir_in_way(path, !o->call_depth)) {
+               if (dir_in_way(path, !o->call_depth,
+                              S_ISGITLINK(a_mode))) {

                        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. "
                        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. "

diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 470f33466ca561484da080a522add72bed51ad6e..9a893b5fe746f03521454972199ecb2d243d79ab 100755 (executable)
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -575,13 +575,13 @@ test_expect_success 'merge removes empty directories' '
        test_must_fail test -d d
 '
 
        test_must_fail test -d d
 '
 

-test_expect_failure 'merge-recursive simple w/submodule' '
+test_expect_success 'merge-recursive simple w/submodule' '

 
        git checkout submod &&
        git merge remove
 '
 
 
        git checkout submod &&
        git merge remove
 '
 

-test_expect_failure 'merge-recursive simple w/submodule result' '
+test_expect_success 'merge-recursive simple w/submodule result' '

 
        git ls-files -s >actual &&
        (
 
        git ls-files -s >actual &&
        (

diff --git a/t/t3426-rebase-submodule.sh b/t/t3426-rebase-submodule.sh
index d5b896d4459e82cdc7eb363af77d8eef28871229..ebf4f5e4b2c1c1cc20a49888df14cedd14b49a8b 100755 (executable)
--- a/t/t3426-rebase-submodule.sh
+++ b/t/t3426-rebase-submodule.sh
@@ -38,9 +38,6 @@ git_rebase_interactive () {
        git rebase -i "$1"
 }
 
        git rebase -i "$1"
 }
 

-KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
-# The real reason "replace directory with submodule" fails is because a
-# directory "sub1" exists, but we reuse the suppression added for merge here

 test_submodule_switch "git_rebase_interactive"
 
 test_done
 test_submodule_switch "git_rebase_interactive"
 
 test_done