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>

David Turner committed Nov 7, 2016 at 13:31 UTC 5423d2e7005eca89481d3137569b2b96b4d133ff
3 files changed +17 -11
merge-recursive.c
+15 -6
@@ -664,7 +664,13 @@ static char *unique_path(struct merge_options *o, const char *path, const char *
664 return strbuf_detach(&newpath, NULL);
665 }
666
667 -static int dir_in_way(const char *path, int check_working_copy)
667 +/**
668 + * Check whether a directory in the index is in the way of an incoming
669 + * file. Return 1 if so. If check_working_copy is non-zero, also
670 + * check the working directory. If empty_ok is non-zero, also return
671 + * 0 in the case where the working-tree dir exists but is empty.
672 + */
673 +static int dir_in_way(const char *path, int check_working_copy, int empty_ok)
674 {
675 int pos;
676 struct strbuf dirpath = STRBUF_INIT;
@@ -684,7 +690,8 @@ static int dir_in_way(const char *path, int check_working_copy)
690 }
691
692 strbuf_release(&dirpath);
687 - return check_working_copy && !lstat(path, &st) && S_ISDIR(st.st_mode);
693 + return check_working_copy && !lstat(path, &st) && S_ISDIR(st.st_mode) &&
694 + !(empty_ok && is_empty_dir(path));
695 }
696
697 static int was_tracked(const char *path)
@@ -1062,7 +1069,7 @@ static int handle_change_delete(struct merge_options *o,
1069 {
1070 char *renamed = NULL;
1071 int ret = 0;
1065 - if (dir_in_way(path, !o->call_depth)) {
1072 + if (dir_in_way(path, !o->call_depth, 0)) {
1073 renamed = unique_path(o, path, a_oid ? o->branch1 : o->branch2);
1074 }
1075
@@ -1195,7 +1202,7 @@ static int handle_file(struct merge_options *o,
1202 remove_file(o, 0, rename->path, 0);
1203 dst_name = unique_path(o, rename->path, cur_branch);
1204 } else {
1198 - if (dir_in_way(rename->path, !o->call_depth)) {
1205 + if (dir_in_way(rename->path, !o->call_depth, 0)) {
1206 dst_name = unique_path(o, rename->path, cur_branch);
1207 output(o, 1, _("%s is a directory in %s adding as %s instead"),
1208 rename->path, other_branch, dst_name);
@@ -1704,7 +1711,8 @@ static int merge_content(struct merge_options *o,
1711 o->branch2 == rename_conflict_info->branch1) ?
1712 pair1->two->path : pair1->one->path;
1713
1707 - if (dir_in_way(path, !o->call_depth))
1714 + if (dir_in_way(path, !o->call_depth,
1715 + S_ISGITLINK(pair1->two->mode)))
1716 df_conflict_remains = 1;
1717 }
1718 if (merge_file_special_markers(o, &one, &a, &b,
@@ -1862,7 +1870,8 @@ static int process_entry(struct merge_options *o,
1870 oid = b_oid;
1871 conf = _("directory/file");
1872 }
1865 - if (dir_in_way(path, !o->call_depth)) {
1873 + if (dir_in_way(path, !o->call_depth,
1874 + S_ISGITLINK(a_mode))) {
1875 char *new_path = unique_path(o, path, add_branch);
1876 clean_merge = 0;
1877 output(o, 1, _("CONFLICT (%s): There is a directory with name %s in %s. "
t/t3030-merge-recursive.sh
+2 -2
@@ -575,13 +575,13 @@ test_expect_success 'merge removes empty directories' '
575 test_must_fail test -d d
576 '
577
578 -test_expect_failure 'merge-recursive simple w/submodule' '
578 +test_expect_success 'merge-recursive simple w/submodule' '
579
580 git checkout submod &&
581 git merge remove
582 '
583
584 -test_expect_failure 'merge-recursive simple w/submodule result' '
584 +test_expect_success 'merge-recursive simple w/submodule result' '
585
586 git ls-files -s >actual &&
587 (
t/t3426-rebase-submodule.sh
-3
@@ -38,9 +38,6 @@ git_rebase_interactive () {
38 git rebase -i "$1"
39 }
40
41 -KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
42 -# The real reason "replace directory with submodule" fails is because a
43 -# directory "sub1" exists, but we reuse the suppression added for merge here
41 test_submodule_switch "git_rebase_interactive"
42
43 test_done